Stop the API server even when it cannot say what it is running - #2173
Closed
MayCXC wants to merge 2 commits into
Closed
Stop the API server even when it cannot say what it is running#2173MayCXC wants to merge 2 commits into
MayCXC wants to merge 2 commits into
Conversation
Stopping the services waits for the containers to exit first, and asks the API server which of them are still running to do it. Both the wait and the bootout of that server sat in one do block, so a server that could not answer took the bootout with it into the catch: the command reported that it failed to wait for containers, said nothing about the server, and left it running. That is the case where stopping matters most. A server that cannot list its containers is the one process that needs to go, and the command that exists to stop it is the one that gives up. Waiting is best effort now, and the services are stopped either way. Verified: with the API server running, stopping the system leaves no apiserver process, and starting it brings one back.
`register` bootstraps into the domain of whoever called it, so the domain a service sits in belongs to the session that started it. A system brought up from a terminal that no window server owns lands in `user/<uid>`, and the same command from a login session lands in `gui/<uid>`. Stop derived its domain the same way, from its own session, and the note above the sweep held that the two would agree, since a domain that had changed would have taken XPC with it. XPC reaches across the difference: a login session drives the containers of a system started over ssh perfectly well, because finding a service is a matter of the Mach namespace rather than of the domain it was bootstrapped into. So the mismatch says nothing when it happens. `bootout` is handed a domain that holds nothing, fails with no such process, and the failure was discarded, leaving a stop that reported stopping every service it named while all of them kept running. The start that followed found them alive and left them as they were, so binaries built since were never the ones being run. Deregistering now asks each domain this user's services could be in, its own first, and says whether any of them gave the service up. A service that could not be stopped is logged rather than dropped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Motivation and Context
Closes #2166.
Two independent ways
container system stopreports success while leaving the services running.The bootout rides in the failing wait's
doblock. Stopping the services waits for the containers to exit first, and asks the API server which of them are still running to do it. Both the wait and the bootout of that server sat in onedoblock, so a server that could not answer took the bootout with it into thecatch: the command reported that it failed to wait for containers, said nothing about the server, and left it running.That is the case where stopping matters most. A server that cannot list its containers is the one process that needs to go, and the command that exists to stop it is the one that gives up. Waiting is best effort now, and the services are stopped either way. What made this hard to see is that stop reported success while every service kept running, so binaries built since were never the ones executed.
The domain a service sits in belongs to the session that registered it.
registerbootstraps into the domain of whoever called it, so a system started from a terminal no window server owns lands inuser/<uid>and the same command from a login session lands ingui/<uid>; stop looked only in the caller's own. Deregistration now tries each domain a service of this user's could be in, the caller's own first, and reports whether any domain gave the service up, so a caller looking in the wrong place is told rather than left to assume it was heard.Relation to #2050
#2050 addresses the same disagreement from the registration side, by making
getDomainString()always answeruser/<uid>so that start and stop compute the same domain from any shell. The two changes are compatible and the swallowed-bootout-failure half above is needed either way.One thing worth checking before #2050 lands on its own: a service bootstrapped into
gui/<uid>by an earlier build stays ingui/<uid>, and I do not know whetherlaunchctl bootout user/<uid>/<label>reaches a service registered in theguisubdomain. If it does not, existing installs would need a one-time manualbootoutfromgui/that neithersystem stopnor an upgrade performs. Searching the domains, as here, does not have that question. I could only probe theuserhalf directly: over ssh,launchctl managernamereportsBackground, the services are registered in and found underuser/501, andgui/501does not hold them.Testing
With the API server running,
container system stopleaves nocontainer-apiserverprocess andcontainer system startbrings one back. Verified from an ssh session (Background), which is the domain-mismatch case that motivated the second commit.Integration suite: 397 passed. Unit suite: 772 passed.
make fmt,make checkclean.