CLOUDP-66661: Enable graceful shutdowns (#105)

Author: fabianlindfors

.evergreen.yml

@@ -184,8 +184,8 @@ tasks:
       - func: setup_virtualenv
       - func: build_and_push_image
         vars:
-          image: quay.io/mongodb/community-operator-pre-stop-hook:${version_id}
-          image_type: prehook
+          image: quay.io/mongodb/community-operator-version-upgrade-post-start-hook:${version_id}
+          image_type: versionhook
 
   - name: build_testrunner_image
     priority: 60

architecture.md

@@ -16,7 +16,7 @@ You create and update MongoDB resources by defining a MongoDB resource definitio
 1. Writes the Automation configuration as a [ConfigMap](https://kubernetes.io/docs/concepts/configuration/configmap/) and mounts it to each pod. 
 1. Creates one [init container](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/) and two [containers](https://kubernetes.io/docs/concepts/containers/overview/) in each pod:
 
-   - An init container which copies the `cmd/prestop` binary to the main `mongod` container. [This pre-stop hook](https://kubernetes.io/docs/concepts/containers/container-lifecycle-hooks/) is used during [version upgrades](#example-mongodb-version-upgrade).
+   - An init container which copies the `cmd/versionhook` binary to the main `mongod` container. This is run before `mongod` starts to handle [version upgrades](#example-mongodb-version-upgrade).
 
    - A container for the [`mongod`](https://docs.mongodb.com/manual/reference/program/mongod/index.html) process binary. `mongod` is the primary daemon process for the MongoDB system. It handles data requests, manages data access, and performs background management operations.
 
@@ -61,7 +61,7 @@ When you update the MongoDB version in your resource definition and reapply it t
 
 1. The MongoDB Agent chooses the first pod to upgrade and stops the `mongod` process using a local connection and [`db.shutdownServer`](https://docs.mongodb.com/manual/reference/method/db.shutdownServer/#db.shutdownServer).
 
-1. A pre-stop hook on the database container checks the state of the MongoDB Agent. If the MongoDB Agent expects the `mongod` process to start with a new version, the hook uses a Kubernetes API call to delete the pod.
+1. Kubernetes will restart the `mongod` container causing the version change hook to run and check the state of the MongoDB Agent. If the MongoDB Agent expects the `mongod` process to start with a new version, the hook uses a Kubernetes API call to delete the pod.
 
 1. The Kubernetes Controller downloads the target version of MongoDB from its default docker registry and restarts the pod with the target version of `mongod` in the database container.
 

cmd/testrunner/main.go

@@ -30,34 +30,34 @@ import (
 )
 
 type flags struct {
-	deployDir      string
-	namespace      string
-	operatorImage  string
-	preHookImage   string
-	testImage      string
-	test           string
-	performCleanup string
+	deployDir               string
+	namespace               string
+	operatorImage           string
+	versionUpgradeHookImage string
+	testImage               string
+	test                    string
+	performCleanup          string
 }
 
 func parseFlags() flags {
-	var namespace, deployDir, operatorImage, preHookImage, testImage, test, performCleanup *string
+	var namespace, deployDir, operatorImage, versionUpgradeHookImage, testImage, test, performCleanup *string
 	namespace = flag.String("namespace", "default", "the namespace the operator and tests should be deployed in")
 	deployDir = flag.String("deployDir", "deploy/", "the path to the directory which contains the yaml deployment files")
 	operatorImage = flag.String("operatorImage", "quay.io/mongodb/community-operator-dev:latest", "the image which should be used for the operator deployment")
-	preHookImage = flag.String("preHookImage", "quay.io/mongodb/community-operator-prehook:latest", "the prestophook image")
+	versionUpgradeHookImage = flag.String("versionUpgradeHookImage", "quay.io/mongodb/community-operator-pre-stop-hook:latest", "the version upgrade post-start hook image")
 	testImage = flag.String("testImage", "quay.io/mongodb/community-operator-e2e:latest", "the image which should be used for the operator e2e tests")
 	test = flag.String("test", "", "test e2e test that should be run. (name of folder containing the test)")
 	performCleanup = flag.String("performCleanup", "1", "specifies whether to performing a cleanup the context or not")
 	flag.Parse()
 
 	return flags{
-		deployDir:      *deployDir,
-		namespace:      *namespace,
-		operatorImage:  *operatorImage,
-		preHookImage:   *preHookImage,
-		testImage:      *testImage,
-		test:           *test,
-		performCleanup: *performCleanup,
+		deployDir:               *deployDir,
+		namespace:               *namespace,
+		operatorImage:           *operatorImage,
+		versionUpgradeHookImage: *versionUpgradeHookImage,
+		testImage:               *testImage,
+		test:                    *test,
+		performCleanup:          *performCleanup,
 	}
 }
 
@@ -174,7 +174,7 @@ func deployOperator(f flags, c client.Client) error {
 		&appsv1.Deployment{},
 		withNamespace(f.namespace),
 		withOperatorImage(f.operatorImage),
-		withPreHookImage(f.preHookImage)); err != nil {
+		withVersionUpgradeHookImage(f.versionUpgradeHookImage)); err != nil {
 		return fmt.Errorf("error building operator deployment: %v", err)
 	}
 	fmt.Println("Successfully created the operator Deployment")
@@ -219,25 +219,25 @@ func withEnvVar(key, val string) func(obj runtime.Object) {
 	}
 }
 
-// withPreHookImage sets the value of the PRE_STOP_HOOK_IMAGE
+// withVersionUpgradeHookImage sets the value of the VERSION_UPGRADE_HOOK_IMAGE
 // EnvVar from first container to `image`. The EnvVar is updated
 // if it exists. Or appended if there is no EnvVar with this `Name`.
-func withPreHookImage(image string) func(runtime.Object) {
+func withVersionUpgradeHookImage(image string) func(runtime.Object) {
 	return func(obj runtime.Object) {
 		if dep, ok := obj.(*appsv1.Deployment); ok {
-			preHookEnv := corev1.EnvVar{
-				Name:  "PRE_STOP_HOOK_IMAGE",
+			versionUpgradeHookEnv := corev1.EnvVar{
+				Name:  "VERSION_UPGRADE_HOOK_IMAGE",
 				Value: image,
 			}
 			found := false
 			for idx := range dep.Spec.Template.Spec.Containers[0].Env {
-				if dep.Spec.Template.Spec.Containers[0].Env[idx].Name == preHookEnv.Name {
-					dep.Spec.Template.Spec.Containers[0].Env[idx].Value = preHookEnv.Value
+				if dep.Spec.Template.Spec.Containers[0].Env[idx].Name == versionUpgradeHookEnv.Name {
+					dep.Spec.Template.Spec.Containers[0].Env[idx].Value = versionUpgradeHookEnv.Value
 					found = true
 				}
 			}
 			if !found {
-				dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, preHookEnv)
+				dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, versionUpgradeHookEnv)
 			}
 		}
 	}

cmd/prestop/main.go renamed to cmd/versionhook/main.go

@@ -20,7 +20,6 @@ import (
 
 const (
 	agentStatusFilePathEnv = "AGENT_STATUS_FILEPATH"
-	logFilePathEnv         = "PRE_STOP_HOOK_LOG_PATH"
 
 	defaultNamespace = "default"
 
@@ -29,19 +28,27 @@ const (
 )
 
 func main() {
-	fmt.Println("Calling pre-stop hook!")
+	logger := setupLogger()
 
-	if err := ensureEnvironmentVariables(logFilePathEnv, agentStatusFilePathEnv); err != nil {
-		zap.S().Fatal("Not all required environment variables are present: %s", err)
-		os.Exit(1)
-	}
+	logger.Info("Running version change post-start hook")
 
-	logger := setupLogger()
+	if statusPath := os.Getenv(agentStatusFilePathEnv); statusPath == "" {
+		logger.Fatalf(`Required environment variable "%s" not set`, agentStatusFilePathEnv)
+		return
+	}
 
 	logger.Info("Waiting for agent health status...")
 	health, err := waitForAgentHealthStatus()
 	if err != nil {
-		logger.Errorf("Error getting the agent health file: %s", err)
+		// If the pod has just restarted then the status file will not exist.
+		// In that case we return and let mongod start again.
+		if os.IsNotExist(err) {
+			logger.Info("Agent health status file not found, mongod will start")
+		} else {
+			logger.Errorf("Error getting the agent health file: %s", err)
+		}
+
+		return
 	}
 
 	shouldDelete, err := shouldDeletePod(health)
@@ -63,32 +70,15 @@ func main() {
 		// is killed by Kubernetes, bringing the new container image
 		// into play.
 		var quit = make(chan struct{})
-		logger.Info("A Pod killed itself, waiting...")
+		logger.Info("Pod killed itself, waiting...")
 		<-quit
 	} else {
-		logger.Info("Pod should not be deleted, container will restart...")
+		logger.Info("Pod should not be deleted, mongod started")
 	}
 }
 
-func ensureEnvironmentVariables(requiredEnvVars ...string) error {
-	var missingEnvVars []string
-	for _, envVar := range requiredEnvVars {
-		if val := os.Getenv(envVar); val == "" {
-			missingEnvVars = append(missingEnvVars, envVar)
-		}
-	}
-	if len(missingEnvVars) > 0 {
-		return fmt.Errorf("missing envars: %s", strings.Join(missingEnvVars, ","))
-	}
-	return nil
-}
-
 func setupLogger() *zap.SugaredLogger {
-	cfg := zap.NewDevelopmentConfig()
-	cfg.OutputPaths = []string{
-		os.Getenv(logFilePathEnv),
-	}
-	log, err := cfg.Build()
+	log, err := zap.NewDevelopment()
 	if err != nil {
 		zap.S().Errorf("Error building logger config: %s", err)
 		os.Exit(1)

contributing.md

@@ -31,7 +31,7 @@ by the operator and mounted in the Agent's Pod.
 
 * MongoDB image: Docker image that includes the MongoDB server.
 
-* Pre-stop Hook: This image includes a binary that helps orchestrate the
+* Version upgrade post-start hook image: This image includes a binary that helps orchestrate the
   restarts of the MongoDB Replica Set members, in particular, when dealing with
   version upgrades, which requires a very precise set of operations to allow for
   seamless upgrades and downgrades, with no downtime.

pkg/controller/mongodb/build_statefulset_test.go

@@ -14,8 +14,7 @@ import (
 )
 
 func init() {
-
-	os.Setenv(preStopHookImageEnv, "pre-stop-hook-image")
+	os.Setenv(versionUpgradeHookImageEnv, "version-upgrade-hook-image")
 }
 
 func TestMultipleCalls_DoNotCauseSideEffects(t *testing.T) {
@@ -46,7 +45,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDB, sts *app
 	assert.Equal(t, appsv1.RollingUpdateStatefulSetStrategyType, sts.Spec.UpdateStrategy.Type)
 	assert.Equal(t, operatorServiceAccountName, sts.Spec.Template.Spec.ServiceAccountName)
 	assert.Len(t, sts.Spec.Template.Spec.Containers[0].Env, 1)
-	assert.Len(t, sts.Spec.Template.Spec.Containers[1].Env, 2)
+	assert.Len(t, sts.Spec.Template.Spec.Containers[1].Env, 1)
 
 	agentContainer := sts.Spec.Template.Spec.Containers[0]
 	assert.Equal(t, "agent-image", agentContainer.Image)
@@ -62,7 +61,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDB, sts *app
 	assert.Len(t, mongodContainer.VolumeMounts, 3)
 
 	initContainer := sts.Spec.Template.Spec.InitContainers[0]
-	assert.Equal(t, preStopHookName, initContainer.Name)
-	assert.Equal(t, "pre-stop-hook-image", initContainer.Image)
+	assert.Equal(t, versionUpgradeHookName, initContainer.Name)
+	assert.Equal(t, "version-upgrade-hook-image", initContainer.Image)
 	assert.Len(t, initContainer.VolumeMounts, 1)
 }

pkg/controller/mongodb/mongodb_controller.go

@@ -43,14 +43,13 @@ import (
 
 const (
 	agentImageEnv                = "AGENT_IMAGE"
-	preStopHookImageEnv          = "PRE_STOP_HOOK_IMAGE"
+	versionUpgradeHookImageEnv   = "VERSION_UPGRADE_HOOK_IMAGE"
 	agentHealthStatusFilePathEnv = "AGENT_STATUS_FILEPATH"
-	preStopHookLogFilePathEnv    = "PRE_STOP_HOOK_LOG_PATH"
 
 	AutomationConfigKey            = "automation-config"
 	agentName                      = "mongodb-agent"
 	mongodbName                    = "mongod"
-	preStopHookName                = "mongod-prehook"
+	versionUpgradeHookName         = "mongod-posthook"
 	dataVolumeName                 = "data-volume"
 	versionManifestFilePath        = "/usr/local/version_manifest.json"
 	readinessProbePath             = "/var/lib/mongodb-mms-automation/probes/readinessprobe"
@@ -511,11 +510,11 @@ func mongodbAgentContainer(volumeMounts []corev1.VolumeMount) container.Modifica
 	)
 }
 
-func preStopHookInit(volumeMount []corev1.VolumeMount) container.Modification {
+func versionUpgradeHookInit(volumeMount []corev1.VolumeMount) container.Modification {
 	return container.Apply(
-		container.WithName(preStopHookName),
-		container.WithCommand([]string{"cp", "pre-stop-hook", "/hooks/pre-stop-hook"}),
-		container.WithImage(os.Getenv(preStopHookImageEnv)),
+		container.WithName(versionUpgradeHookName),
+		container.WithCommand([]string{"cp", "version-upgrade-hook", "/hooks/version-upgrade"}),
+		container.WithImage(os.Getenv(versionUpgradeHookImageEnv)),
 		container.WithImagePullPolicy(corev1.PullAlways),
 		container.WithVolumeMounts(volumeMount),
 	)
@@ -525,15 +524,15 @@ func mongodbContainer(version string, volumeMounts []corev1.VolumeMount) contain
 	mongoDbCommand := []string{
 		"/bin/sh",
 		"-c",
-		// we execute the pre-stop hook once the mongod has been gracefully shut down by the agent.
-		`while [ ! -f /data/automation-mongod.conf ]; do sleep 3 ; done ; sleep 2 ;
-# start mongod with this configuration
-mongod -f /data/automation-mongod.conf ;
+		`
+# run post-start hook to handle version changes
+/hooks/version-upgrade
+
+# wait for config to be created by the agent
+while [ ! -f /data/automation-mongod.conf ]; do sleep 3 ; done ; sleep 2 ;
 
-# start the pre-stop-hook to restart the Pod when needed
-# If the Pod does not require to be restarted, the pre-stop-hook will
-# exit(0) for Kubernetes to restart the container.
-/hooks/pre-stop-hook ;
+# start mongod with this configuration
+exec mongod -f /data/automation-mongod.conf ;
 `,
 	}
 
@@ -547,10 +546,6 @@ mongod -f /data/automation-mongod.conf ;
 				Name:  agentHealthStatusFilePathEnv,
 				Value: "/healthstatus/agent-health-status.json",
 			},
-			corev1.EnvVar{
-				Name:  preStopHookLogFilePathEnv,
-				Value: "/hooks/pre-stop-hook.log",
-			},
 		),
 		container.WithVolumeMounts(volumeMounts),
 	)
@@ -604,7 +599,7 @@ func buildStatefulSetModificationFunction(mdb mdbv1.MongoDB) statefulset.Modific
 				podtemplatespec.WithServiceAccount(operatorServiceAccountName),
 				podtemplatespec.WithContainer(agentName, mongodbAgentContainer([]corev1.VolumeMount{agentHealthStatusVolumeMount, automationConfigVolumeMount, dataVolume})),
 				podtemplatespec.WithContainer(mongodbName, mongodbContainer(mdb.Spec.Version, []corev1.VolumeMount{mongodHealthStatusVolumeMount, dataVolume, hooksVolumeMount})),
-				podtemplatespec.WithInitContainer(preStopHookName, preStopHookInit([]corev1.VolumeMount{hooksVolumeMount})),
+				podtemplatespec.WithInitContainer(versionUpgradeHookName, versionUpgradeHookInit([]corev1.VolumeMount{hooksVolumeMount})),
 				buildTLSPodSpecModification(mdb),
 				buildScramPodSpecModification(mdb),
 			),

scripts/ci/config.json

@@ -3,6 +3,6 @@
     "repo_url": "quay.io/mongodb",
     "operator_image": "community-operator-dev",
     "e2e_image": "community-operator-e2e",
-    "prestop_hook_image": "community-operator-pre-stop-hook",
+    "version_upgrade_hook_image": "community-operator-version-upgrade-post-start-hook",
     "testrunner_image": "community-operator-testrunner"
 }

scripts/dev/dev_config.py

@@ -35,8 +35,8 @@ def e2e_image(self) -> str:
         return self._config["e2e_image"]
 
     @property
-    def prestop_hook_image(self) -> str:
-        return self._config["prestop_hook_image"]
+    def version_upgrade_hook_image(self) -> str:
+        return self._config["version_upgrade_hook_image"]
 
     @property
     def testrunner_image(self) -> str:

scripts/dev/e2e.py

@@ -123,11 +123,11 @@ def build_and_push_e2e(repo_url: str, tag: str, path: str) -> None:
     build_and_push_image(repo_url, tag, path, "e2e")
 
 
-def build_and_push_prehook(repo_url: str, tag: str, path: str) -> None:
+def build_and_push_version_upgrade_hook(repo_url: str, tag: str, path: str) -> None:
     """
-    build_and_push_prehook builds and pushes the pre-stop-hook image.
+    build_and_push_version_upgrade_hook builds and pushes the version upgrade hook image.
     """
-    build_and_push_image(repo_url, tag, path, "prehook")
+    build_and_push_image(repo_url, tag, path, "versionhook")
 
 
 def _delete_testrunner_pod(config_file: str) -> None:
@@ -216,8 +216,8 @@ def _get_testrunner_pod_body(
                         "./runner",
                         "--operatorImage",
                         f"{dev_config.repo_url}/{dev_config.operator_image}:{tag}",
-                        "--preHookImage",
-                        f"{dev_config.repo_url}/{dev_config.prestop_hook_image}:{tag}",
+                        "--versionUpgradeHookImage",
+                        f"{dev_config.repo_url}/{dev_config.version_upgrade_hook_image}:{tag}",
                         "--testImage",
                         f"{dev_config.repo_url}/{dev_config.e2e_image}:{tag}",
                         f"--test={test}",
@@ -240,7 +240,7 @@ def parse_args() -> argparse.Namespace:
     )
     parser.add_argument(
         "--build-images",
-        help="Build testrunner, e2e and prestop-hook images",
+        help="Build testrunner, e2e and version upgrade hook images",
         action="store_true",
     )
     parser.add_argument(
@@ -284,10 +284,10 @@ def build_and_push_images(args: argparse.Namespace, dev_config: DevConfig) -> No
             "{}/{}:{}".format(dev_config.repo_url, dev_config.e2e_image, args.tag),
             ".",
         )
-        build_and_push_prehook(
+        build_and_push_version_upgrade_hook(
             dev_config.repo_url,
             "{}/{}:{}".format(
-                dev_config.repo_url, dev_config.prestop_hook_image, args.tag
+                dev_config.repo_url, dev_config.version_upgrade_hook_image, args.tag
             ),
             ".",
         )