diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index 164dd90fd..929ce046d 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -49,8 +49,7 @@ extension Application { } ) - let launchdDomainString = try ServiceManager.getDomainString() - let fullLabel = "\(launchdDomainString)/\(prefix)apiserver" + let apiserverLabel = "\(prefix)apiserver" var running = true do { @@ -85,24 +84,47 @@ extension Application { } try await Task.sleep(for: .seconds(1)) } - - log.info("stopping service", metadata: ["label": "\(fullLabel)"]) - try ServiceManager.deregister(fullServiceLabel: fullLabel) } catch { log.warning("failed to wait for all containers", metadata: ["error": "\(error)"]) } } - // Note: The assumption here is that we would have registered the launchd services - // in the same domain as `launchdDomainString`. This is a fairly sane assumption since - // if somehow the launchd domain changed, XPC interactions would not be possible. + // Stopping the services is what this command is for, so it + // happens whether or not the containers could be waited for, and + // whether or not the API server answered its health check at all. + // Asking the API server about its containers is asking the + // service being stopped, and it is when that service is in a + // bad way that stopping it matters most: leaving it running + // because it could not answer leaves the one process that + // needed stopping. + log.info("stopping service", metadata: ["label": "\(apiserverLabel)"]) + do { + if try !ServiceManager.deregisterAnyDomain(serviceLabel: apiserverLabel) { + log.warning( + "failed to stop service", + metadata: ["label": "\(apiserverLabel)", "error": "no launchd domain holds it"]) + } + } catch { + log.warning("failed to stop service", metadata: ["label": "\(apiserverLabel)", "error": "\(error)"]) + } + + // The domain a service was registered in is the one belonging to + // the session that started it, which is not necessarily this one: + // a system brought up from a terminal no window server owns sits + // in `user/`, and this command run from a login session looks + // in `gui/`. Either session can drive the other's services, + // so a mismatch does not announce itself as a failure to reach + // them; it announces itself as a stop that says it stopped + // everything while every service it named is still running. try ServiceManager.enumerate() .filter { $0.hasPrefix(prefix) } - .filter { $0 != fullLabel } - .map { "\(launchdDomainString)/\($0)" } + .filter { $0 != apiserverLabel } .forEach { log.info("stopping service", metadata: ["label": "\($0)"]) - try? ServiceManager.deregister(fullServiceLabel: $0) + let stopped = (try? ServiceManager.deregisterAnyDomain(serviceLabel: $0)) ?? false + if !stopped { + log.warning("failed to stop service", metadata: ["label": "\($0)"]) + } } } } diff --git a/Sources/ContainerPlugin/ServiceManager.swift b/Sources/ContainerPlugin/ServiceManager.swift index 4d2c36246..b9ab06b03 100644 --- a/Sources/ContainerPlugin/ServiceManager.swift +++ b/Sources/ContainerPlugin/ServiceManager.swift @@ -49,6 +49,30 @@ public struct ServiceManager { status = try runLaunchctlCommand(args: ["bootout", label]) } + /// Deregister a service by its bare label, from whichever domain holds it. + /// + /// The domain a service sits in belongs to the session that registered it, + /// not to the session asking about it now: `register` bootstraps into the + /// domain of whoever called it, so a system started from a terminal no + /// window server owns lands in `user/`, and the same command from a + /// login session lands in `gui/`. Either one can drive the other's + /// services afterwards, because reaching them is a matter of the Mach + /// namespace rather than of the domain they were bootstrapped into. + /// + /// Returns whether a domain gave the service up, so that a caller looking + /// in the wrong place is told so rather than left to assume it was heard. + @discardableResult + public static func deregisterAnyDomain(serviceLabel label: String) throws -> Bool { + for domain in try Self.getDomainStrings() { + var status: Int32 = -1 + try Self.deregister(fullServiceLabel: "\(domain)/\(label)", status: &status) + if status == 0 { + return true + } + } + return false + } + /// Restart a service by a launchd label. public static func kickstart(fullServiceLabel label: String) throws { _ = try runLaunchctlCommand(args: ["kickstart", "-k", label]) @@ -121,6 +145,14 @@ public struct ServiceManager { return outputText.trimmingCharacters(in: .whitespacesAndNewlines) } + /// Every launchd domain a service of this user's could be registered in, + /// the caller's own first so that the common case is answered first. + public static func getDomainStrings() throws -> [String] { + let own = try Self.getDomainString() + let userDomains = ["user/\(getuid())", "gui/\(getuid())"] + return [own] + userDomains.filter { $0 != own } + } + public static func getDomainString() throws -> String { let currentSessionType = try getLaunchdSessionType() switch currentSessionType {