Add a new annotation 'juju.io/charm-modified-version' to main workloa…

…d resource to triger pods restarted after charm upgraded;
juju · Jul 20, 2020 · 1b7265d · 1b7265d
1 parent 2267270
commit 1b7265d
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 99 deletions.
diff --git a/apiserver/params/kubernetes.go b/apiserver/params/kubernetes.go
@@ -26,7 +26,7 @@ type KubernetesProvisioningInfo struct {
 	Volumes              []KubernetesVolumeParams     `json:"volumes,omitempty"`
 	Devices              []KubernetesDeviceParams     `json:"devices,omitempty"`
 	OperatorImagePath    string                       `json:"operator-image-path,omitempty"`
-	CharmModifiedVersion int                          `json:"charm-modified-version"`
+	CharmModifiedVersion int                          `json:"charm-modified-version,omitempty"`
 }
 
 // KubernetesProvisioningInfoResult holds unit provisioning info or an error.

diff --git a/caas/broker.go b/caas/broker.go
@@ -118,9 +118,8 @@ const (
 
 // DeploymentParams defines parameters for specifying how a service is deployed.
 type DeploymentParams struct {
-	DeploymentType       DeploymentType
-	ServiceType          ServiceType
-	CharmModifiedVersion int
+	DeploymentType DeploymentType
+	ServiceType    ServiceType
 }
 
 // ServiceParams defines parameters used to create a service.
@@ -149,6 +148,9 @@ type ServiceParams struct {
 
 	// OperatorImagePath is the path to the OCI image shared by the operator and pod init.
 	OperatorImagePath string
+
+	// CharmModifiedVersion increases when the charm changes in some way.
+	CharmModifiedVersion int
 }
 
 // OperatorState is returned by the OperatorExists call.

diff --git a/caas/kubernetes/provider/admissionregistration_test.go b/caas/kubernetes/provider/admissionregistration_test.go
@@ -45,8 +45,9 @@ func (s *K8sBrokerSuite) assertMutatingWebhookConfigurations(c *gc.C, cfgs []k8s
 			Name:   "app-name",
 			Labels: map[string]string{"juju-app": "app-name"},
 			Annotations: map[string]string{
-				"juju-app-uuid":      "appuuid",
-				"juju.io/controller": testing.ControllerTag.Id(),
+				"juju-app-uuid":                  "appuuid",
+				"juju.io/controller":             testing.ControllerTag.Id(),
+				"juju.io/charm-modified-version": "0",
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
@@ -62,6 +63,7 @@ func (s *K8sBrokerSuite) assertMutatingWebhookConfigurations(c *gc.C, cfgs []k8s
 						"apparmor.security.beta.kubernetes.io/pod": "runtime/default",
 						"seccomp.security.beta.kubernetes.io/pod":  "docker/default",
 						"juju.io/controller":                       testing.ControllerTag.Id(),
+						"juju.io/charm-modified-version":           "0",
 					},
 				},
 				Spec: podSpec,
@@ -338,8 +340,9 @@ func (s *K8sBrokerSuite) assertValidatingWebhookConfigurations(c *gc.C, cfgs []k
 			Name:   "app-name",
 			Labels: map[string]string{"juju-app": "app-name"},
 			Annotations: map[string]string{
-				"juju-app-uuid":      "appuuid",
-				"juju.io/controller": testing.ControllerTag.Id(),
+				"juju-app-uuid":                  "appuuid",
+				"juju.io/controller":             testing.ControllerTag.Id(),
+				"juju.io/charm-modified-version": "0",
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
@@ -355,6 +358,7 @@ func (s *K8sBrokerSuite) assertValidatingWebhookConfigurations(c *gc.C, cfgs []k
 						"apparmor.security.beta.kubernetes.io/pod": "runtime/default",
 						"seccomp.security.beta.kubernetes.io/pod":  "docker/default",
 						"juju.io/controller":                       testing.ControllerTag.Id(),
+						"juju.io/charm-modified-version":           "0",
 					},
 				},
 				Spec: podSpec,

diff --git a/caas/kubernetes/provider/customresourcedefinitions_test.go b/caas/kubernetes/provider/customresourcedefinitions_test.go
@@ -45,8 +45,9 @@ func (s *K8sBrokerSuite) assertCustomerResourceDefinitions(c *gc.C, crds []k8ssp
 			Name:   "app-name",
 			Labels: map[string]string{"juju-app": "app-name"},
 			Annotations: map[string]string{
-				"juju-app-uuid":      "appuuid",
-				"juju.io/controller": testing.ControllerTag.Id(),
+				"juju-app-uuid":                  "appuuid",
+				"juju.io/controller":             testing.ControllerTag.Id(),
+				"juju.io/charm-modified-version": "0",
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
@@ -62,6 +63,7 @@ func (s *K8sBrokerSuite) assertCustomerResourceDefinitions(c *gc.C, crds []k8ssp
 						"apparmor.security.beta.kubernetes.io/pod": "runtime/default",
 						"seccomp.security.beta.kubernetes.io/pod":  "docker/default",
 						"juju.io/controller":                       testing.ControllerTag.Id(),
+						"juju.io/charm-modified-version":           "0",
 					},
 				},
 				Spec: podSpec,
@@ -367,8 +369,9 @@ func (s *K8sBrokerSuite) assertCustomerResources(c *gc.C, crs map[string][]unstr
 			Name:   "app-name",
 			Labels: map[string]string{"juju-app": "app-name"},
 			Annotations: map[string]string{
-				"juju-app-uuid":      "appuuid",
-				"juju.io/controller": testing.ControllerTag.Id(),
+				"juju-app-uuid":                  "appuuid",
+				"juju.io/controller":             testing.ControllerTag.Id(),
+				"juju.io/charm-modified-version": "0",
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
@@ -384,6 +387,7 @@ func (s *K8sBrokerSuite) assertCustomerResources(c *gc.C, crs map[string][]unstr
 						"apparmor.security.beta.kubernetes.io/pod": "runtime/default",
 						"seccomp.security.beta.kubernetes.io/pod":  "docker/default",
 						"juju.io/controller":                       testing.ControllerTag.Id(),
+						"juju.io/charm-modified-version":           "0",
 					},
 				},
 				Spec: podSpec,

diff --git a/caas/kubernetes/provider/ingress_test.go b/caas/kubernetes/provider/ingress_test.go
@@ -39,8 +39,9 @@ func (s *K8sBrokerSuite) assertIngressResources(c *gc.C, IngressResources []k8ss
 			Name:   "app-name",
 			Labels: map[string]string{"juju-app": "app-name"},
 			Annotations: map[string]string{
-				"juju-app-uuid":      "appuuid",
-				"juju.io/controller": testing.ControllerTag.Id(),
+				"juju-app-uuid":                  "appuuid",
+				"juju.io/controller":             testing.ControllerTag.Id(),
+				"juju.io/charm-modified-version": "0",
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
@@ -56,6 +57,7 @@ func (s *K8sBrokerSuite) assertIngressResources(c *gc.C, IngressResources []k8ss
 						"apparmor.security.beta.kubernetes.io/pod": "runtime/default",
 						"seccomp.security.beta.kubernetes.io/pod":  "docker/default",
 						"juju.io/controller":                       testing.ControllerTag.Id(),
+						"juju.io/charm-modified-version":           "0",
 					},
 				},
 				Spec: podSpec,

diff --git a/caas/kubernetes/provider/k8s.go b/caas/kubernetes/provider/k8s.go
@@ -1012,13 +1012,7 @@ func (k *kubernetesClient) ensureService(
 		return errors.Annotatef(err, "parsing unit spec for %s", appName)
 	}
 
-	annotations := resourceTagsToAnnotations(params.ResourceTags).
-		// To solve https://bugs.launchpad.net/juju/+bug/1875481/comments/23 (`jujud caas-unit-init --upgrade`
-		// does NOT work on containers are not using root as default USER),
-		// CharmModifiedVersion is added for triggering rolling upgrade on workload pods to synchronise
-		// charm files to workload pods via init container when charm was upgraded.
-		// This approach was inspired from `kubectl rollout restart`.
-		Add(annotationCharmModifiedVersionKey, strconv.Itoa(params.Deployment.CharmModifiedVersion))
+	annotations := resourceTagsToAnnotations(params.ResourceTags)
 
 	// ensure configmap.
 	if len(workloadSpec.ConfigMaps) > 0 {
@@ -1179,24 +1173,32 @@ func (k *kubernetesClient) ensureService(
 	}
 
 	numPods := int32(numUnits)
+	workloadResourceAnnotations := annotations.Copy().
+		// To solve https://bugs.launchpad.net/juju/+bug/1875481/comments/23 (`jujud caas-unit-init --upgrade`
+		// does NOT work on containers are not using root as default USER),
+		// CharmModifiedVersion is added for triggering rolling upgrade on workload pods to synchronise
+		// charm files to workload pods via init container when charm was upgraded.
+		// This approach was inspired from `kubectl rollout restart`.
+		Add(annotationCharmModifiedVersionKey, strconv.Itoa(params.CharmModifiedVersion))
+
 	switch params.Deployment.DeploymentType {
 	case caas.DeploymentStateful:
 		if err := k.configureHeadlessService(appName, deploymentName, annotations.Copy()); err != nil {
 			return errors.Annotate(err, "creating or updating headless service")
 		}
 		cleanups = append(cleanups, func() { _ = k.deleteService(headlessServiceName(deploymentName)) })
-		if err := k.configureStatefulSet(appName, deploymentName, annotations.Copy(), workloadSpec, params.PodSpec.Containers, &numPods, params.Filesystems); err != nil {
+		if err := k.configureStatefulSet(appName, deploymentName, workloadResourceAnnotations.Copy(), workloadSpec, params.PodSpec.Containers, &numPods, params.Filesystems); err != nil {
 			return errors.Annotate(err, "creating or updating StatefulSet")
 		}
 		cleanups = append(cleanups, func() { _ = k.deleteDeployment(appName) })
 	case caas.DeploymentStateless:
-		cleanUpDeployment, err := k.configureDeployment(appName, deploymentName, annotations.Copy(), workloadSpec, params.PodSpec.Containers, &numPods, params.Filesystems)
+		cleanUpDeployment, err := k.configureDeployment(appName, deploymentName, workloadResourceAnnotations.Copy(), workloadSpec, params.PodSpec.Containers, &numPods, params.Filesystems)
 		cleanups = append(cleanups, cleanUpDeployment...)
 		if err != nil {
 			return errors.Annotate(err, "creating or updating Deployment")
 		}
 	case caas.DeploymentDaemon:
-		cleanUpDaemonSet, err := k.configureDaemonSet(appName, deploymentName, annotations.Copy(), workloadSpec, params.PodSpec.Containers, params.Filesystems)
+		cleanUpDaemonSet, err := k.configureDaemonSet(appName, deploymentName, workloadResourceAnnotations.Copy(), workloadSpec, params.PodSpec.Containers, params.Filesystems)
 		cleanups = append(cleanups, cleanUpDaemonSet...)
 		if err != nil {
 			return errors.Annotate(err, "creating or updating DaemonSet")