Stop health check futures when upgrading the Supervisor

components/sup/src/manager.rs

@@ -1065,7 +1065,17 @@ impl Manager {
         ctl_shutdown_tx.send(()).ok();
 
         match shutdown_mode {
-            ShutdownMode::Restarting => {}
+            ShutdownMode::Restarting => {
+                outputln!("Preparing services for Supervisor restart");
+                for (_ident, svc) in self.state
+                                         .services
+                                         .write()
+                                         .expect("Services lock is poisoned!")
+                                         .iter_mut()
+                {
+                    svc.detach();
+                }
+            }
             ShutdownMode::Normal | ShutdownMode::Departed => {
                 outputln!("Gracefully departing from butterfly network.");
                 self.butterfly.set_departed_mlw();

components/sup/src/manager/service.rs

@@ -344,6 +344,7 @@ impl Service {
     /// Consumes the handle to that future in the process.
     fn stop_health_checks(&mut self) {
         if let Some(h) = self.health_check_handle.take() {
+            debug!("Stopping health checks for {}", self.pkg.ident);
             h.terminate();
         }
     }
@@ -358,6 +359,29 @@ impl Service {
         self.start_health_checks(executor);
     }
 
+    /// Called when the Supervisor reattaches itself to an already
+    /// running service. Use this to re-initiate any associated
+    /// processes, futures, etc.
+    ///
+    /// This should generally be the opposite of `Service::detach`.
+    fn reattach(&mut self, executor: &TaskExecutor) {
+        outputln!("Reattaching to {}", self.service_group);
+        self.initialized = true;
+        self.restart_health_checks(executor);
+    }
+
+    /// Called when stopping the Supervisor for an update. Should
+    /// *not* stop the service process itself, but should stop any
+    /// associated processes, futures, etc., that would otherwise
+    /// prevent the Supervisor from shutting itself down.
+    ///
+    /// Currently, this means stopping any associated long-running
+    /// futures.
+    ///
+    /// See also `Service::reattach`, as these methods should
+    /// generally be mirror images of each other.
+    pub fn detach(&mut self) { self.stop_health_checks(); }
+
     /// Return a future that will shut down a service, performing any
     /// necessary cleanup, and run its post-stop hook, if any.
     pub fn stop(&mut self,
@@ -924,8 +948,7 @@ impl Service {
     fn execute_hooks(&mut self, launcher: &LauncherCli, executor: &TaskExecutor) {
         if !self.initialized {
             if self.check_process() {
-                outputln!("Reattached to {}", self.service_group);
-                self.initialized = true;
+                self.reattach(executor);
                 return;
             }
             self.initialize();

test/end-to-end/hup-does-not-abandon-services.exp

@@ -72,6 +72,10 @@ expect {
     }
 }
 
+# Let the control gateway come up. :/
+# Alternatively, use `nc -wv localhost 9632`
+sleep 1
+
 # Load up Redis on that Supervisor
 spawn hab svc load core/redis
 expect {
@@ -115,7 +119,7 @@ expect {
     }
 }
 expect {
-    "Reattached to redis.default" {
+    "Reattaching to redis.default" {
         log "Supervisor reattached"
     }
     timeout {