Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that graceful start-up is informed of unused SSH listener (#20877) #20888

Merged
merged 1 commit into from
Aug 21, 2022
Merged

Conversation

eeyrjmr
Copy link
Contributor

@eeyrjmr eeyrjmr commented Aug 21, 2022

Backport #20877

The graceful manager waits for 4 listeners to be created or to be told that they are not needed. If it is not told about them it will indefinitely and timeout.

This leads to SVC hosts not being told of being in the readyState but on Unix would lead to the termination of the process.

There was an unfortunate regression in #20299 which missed this subtly and in the case whereby SSH is disabled the builtinUnused() is not called.

This PR adds a call to builtinUnused() when not using the builtin ssh to allow createServerWaitGroup.Done() to be called.

In addition it was noted that the if/else clauses for timeout informing of the SVC host were in the wrong order. These have been swapped.

Fix #20609

This is to ensure any ServerWaitGroups complete

svc.Status called with the StartupTimeout if the setting is nonzero

correct gofmt
@Gusted Gusted added this to the 1.17.2 milestone Aug 21, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 21, 2022
@eeyrjmr eeyrjmr changed the title call builtinUnused() if internal SSH is disabled call builtinUnused() if internal SSH is disabled (#20877) Aug 21, 2022
@wxiaoguang wxiaoguang changed the title call builtinUnused() if internal SSH is disabled (#20877) Ensure that graceful start-up is informed of unused SSH listener (#20877) Aug 21, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 21, 2022
@wxiaoguang wxiaoguang merged commit eee51d8 into go-gitea:release/v1.17 Aug 21, 2022
@eeyrjmr eeyrjmr deleted the backport-20877 branch August 21, 2022 12:37
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants