[Bugfix] Propagate Lifecycle Mount (#1139)

CHANGELOG.md

@@ -6,6 +6,7 @@
 - (Feature) Add annotation to change architecture of a member
 - (Bugfix) Prevent Member Maintenance Error log
 - (Feature) ID ServerGroup
+- (Bugfix) Propagate Lifecycle Mount
 
 ## [1.2.19](https://github.com/arangodb/kube-arangodb/tree/1.2.19) (2022-10-05)
 - (Bugfix) Prevent changes when UID is wrong

pkg/deployment/resources/pod_creator_arangod.go

@@ -716,6 +716,12 @@ func addLifecycleSidecar(coreNames []string, sidecars []core.Container) error {
 				break
 			}
 
+			if !k8sutil.VolumeMountExists(sidecar.VolumeMounts, shared.LifecycleVolumeName) {
+				sidecars[i].VolumeMounts = append(sidecars[i].VolumeMounts, k8sutil.LifecycleVolumeMount())
+			}
+
+			sidecars[i].Env = k8sutil.AppendLifecycleEnv(sidecars[i].Env)
+
 			lifecycle, err := k8sutil.NewLifecycleFinalizers()
 			if err != nil {
 				return err

pkg/deployment/rotation/arangod_containers.go

@@ -29,8 +29,6 @@ import (
 
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
 	"github.com/arangodb/kube-arangodb/pkg/deployment/features"
-	"github.com/arangodb/kube-arangodb/pkg/deployment/resources"
-	"github.com/arangodb/kube-arangodb/pkg/deployment/topology"
 	"github.com/arangodb/kube-arangodb/pkg/util"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
@@ -62,7 +60,7 @@ func containersCompare(ds api.DeploymentSpec, g api.ServerGroup, spec, status *c
 
 					g := podContainerFuncGenerator(ds, g, ac, bc)
 
-					if m, p, err := comparePodContainer(builder, g(compareServerContainerVolumeMounts), g(compareServerContainerProbes)); err != nil {
+					if m, p, err := comparePodContainer(builder, g(compareServerContainerVolumeMounts), g(compareServerContainerProbes), g(compareServerContainerEnvs)); err != nil {
 						log.Err(err).Msg("Error while getting pod diff")
 						return SkippedRotation, nil, err
 					} else {
@@ -70,22 +68,6 @@ func containersCompare(ds api.DeploymentSpec, g api.ServerGroup, spec, status *c
 						plan = append(plan, p...)
 					}
 
-					if !equality.Semantic.DeepEqual(ac.Env, bc.Env) {
-						filter := func(a, b map[string]core.EnvVar) (map[string]core.EnvVar, map[string]core.EnvVar) {
-							for _, excludedEnv := range getExcludedEnv() {
-								delete(a, excludedEnv)
-								delete(b, excludedEnv)
-							}
-
-							return a, b
-						}
-						if areEnvsEqual(ac.Env, bc.Env, filter) {
-							// Envs are the same after filtering, but it were different before filtering, so it can be replaced.
-							bc.Env = ac.Env
-							mode = mode.And(SilentRotation)
-						}
-					}
-
 					if !equality.Semantic.DeepEqual(ac.EnvFrom, bc.EnvFrom) {
 						// Check EnvFromSource differences.
 						filter := func(a, b map[string]core.EnvFromSource) (map[string]core.EnvFromSource, map[string]core.EnvFromSource) {
@@ -113,6 +95,16 @@ func containersCompare(ds api.DeploymentSpec, g api.ServerGroup, spec, status *c
 						bc.Image = ac.Image
 						mode = mode.And(InPlaceRotation)
 					}
+
+					g := podContainerFuncGenerator(ds, g, ac, bc)
+
+					if m, p, err := comparePodContainer(builder, g(compareAnyContainerVolumeMounts), g(compareAnyContainerEnvs)); err != nil {
+						log.Err(err).Msg("Error while getting pod diff")
+						return SkippedRotation, nil, err
+					} else {
+						mode = mode.And(m)
+						plan = append(plan, p...)
+					}
 				}
 
 				if api.IsReservedServerGroupContainerName(ac.Name) {
@@ -217,27 +209,6 @@ func internalContainerLifecycleCompare(spec, status *core.Container) Mode {
 	return SkippedRotation
 }
 
-func areEnvsEqual(a, b []core.EnvVar, rules ...func(a, b map[string]core.EnvVar) (map[string]core.EnvVar, map[string]core.EnvVar)) bool {
-	am := getEnvs(a)
-	bm := getEnvs(b)
-
-	for _, r := range rules {
-		am, bm = r(am, bm)
-	}
-
-	return equality.Semantic.DeepEqual(am, bm)
-}
-
-func getEnvs(e []core.EnvVar) map[string]core.EnvVar {
-	m := map[string]core.EnvVar{}
-
-	for _, q := range e {
-		m[q.Name] = q
-	}
-
-	return m
-}
-
 func areProbesEqual(a, b *core.Probe) bool {
 	if a == nil && b == nil {
 		return true
@@ -286,10 +257,3 @@ func createEnvsFromMap(e []core.EnvFromSource) map[string]core.EnvFromSource {
 
 	return m
 }
-
-// getExcludedEnv returns environment variables which should not be compared when pod's rotation is considered.
-func getExcludedEnv() []string {
-	return []string{topology.ArangoDBZone, resources.ArangoDBOverrideServerGroupEnv,
-		resources.ArangoDBOverrideDeploymentModeEnv, resources.ArangoDBOverrideVersionEnv,
-		resources.ArangoDBOverrideEnterpriseEnv}
-}

pkg/deployment/rotation/arangod_envs.go

@@ -0,0 +1,135 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package rotation
+
+import (
+	"reflect"
+
+	core "k8s.io/api/core/v1"
+
+	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/deployment/resources"
+	"github.com/arangodb/kube-arangodb/pkg/deployment/topology"
+	"github.com/arangodb/kube-arangodb/pkg/util/constants"
+)
+
+func compareServerContainerEnvs(ds api.DeploymentSpec, g api.ServerGroup, spec, status *core.Container) comparePodContainerFunc {
+	return func(builder api.ActionBuilder) (mode Mode, plan api.Plan, err error) {
+		specV := mapEnvs(spec)
+		statusV := mapEnvs(status)
+
+		diff := getEnvDiffFromPods(specV, statusV)
+
+		if len(diff) == 0 {
+			return SkippedRotation, nil, nil
+		}
+
+		for k := range diff {
+			switch k {
+			case topology.ArangoDBZone, resources.ArangoDBOverrideServerGroupEnv,
+				resources.ArangoDBOverrideDeploymentModeEnv, resources.ArangoDBOverrideVersionEnv,
+				resources.ArangoDBOverrideEnterpriseEnv:
+				// Those envs can change without restart
+				continue
+			case constants.EnvOperatorPodName, constants.EnvOperatorPodNamespace, constants.EnvOperatorNodeName, constants.EnvOperatorNodeNameArango:
+				// Lifecycle envs can change without restart
+				continue
+			default:
+				return GracefulRotation, nil, nil
+			}
+		}
+
+		status.Env = spec.Env
+		return SilentRotation, nil, nil
+	}
+}
+
+func compareAnyContainerEnvs(ds api.DeploymentSpec, g api.ServerGroup, spec, status *core.Container) comparePodContainerFunc {
+	return func(builder api.ActionBuilder) (mode Mode, plan api.Plan, err error) {
+		specV := mapEnvs(spec)
+		statusV := mapEnvs(status)
+
+		diff := getEnvDiffFromPods(specV, statusV)
+
+		if len(diff) == 0 {
+			return SkippedRotation, nil, nil
+		}
+
+		for k := range diff {
+			switch k {
+			case constants.EnvOperatorPodName, constants.EnvOperatorPodNamespace, constants.EnvOperatorNodeName, constants.EnvOperatorNodeNameArango:
+				// Lifecycle envs can change without restart
+				continue
+			default:
+				return GracefulRotation, nil, nil
+			}
+		}
+
+		status.Env = spec.Env
+		return SilentRotation, nil, nil
+	}
+}
+
+type envDiff struct {
+	a, b []*core.EnvVar
+}
+
+func getEnvDiffFromPods(a, b map[string][]*core.EnvVar) map[string]envDiff {
+	d := map[string]envDiff{}
+
+	for k := range a {
+		if z, ok := b[k]; ok {
+			if !reflect.DeepEqual(a[k], z) {
+				d[k] = envDiff{
+					a: a[k],
+					b: z,
+				}
+			}
+		} else {
+			d[k] = envDiff{
+				a: a[k],
+				b: nil,
+			}
+		}
+	}
+	for k := range b {
+		if _, ok := a[k]; !ok {
+			d[k] = envDiff{
+				a: nil,
+				b: a[k],
+			}
+		}
+	}
+
+	return d
+}
+
+func mapEnvs(a *core.Container) map[string][]*core.EnvVar {
+	n := make(map[string][]*core.EnvVar, len(a.VolumeMounts))
+
+	for id := range a.Env {
+		v := &a.Env[id]
+
+		n[v.Name] = append(n[v.Name], v)
+	}
+
+	return n
+}

pkg/deployment/rotation/arangod_volume_mounts.go

@@ -0,0 +1,137 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package rotation
+
+import (
+	"reflect"
+
+	core "k8s.io/api/core/v1"
+
+	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/apis/shared"
+)
+
+func compareServerContainerVolumeMounts(ds api.DeploymentSpec, g api.ServerGroup, spec, status *core.Container) comparePodContainerFunc {
+	return func(builder api.ActionBuilder) (mode Mode, plan api.Plan, err error) {
+		specV := mapVolumeMounts(spec)
+		statusV := mapVolumeMounts(status)
+
+		diff := getVolumeMountsDiffFromPods(specV, statusV)
+
+		if len(diff) == 0 {
+			return SkippedRotation, nil, nil
+		}
+
+		for k, v := range diff {
+			switch k {
+			case shared.ArangoDTimezoneVolumeName:
+				// We are fine, should be just replaced
+				if v.a == nil {
+					// we remove volume
+					return GracefulRotation, nil, nil
+				}
+
+				if ds.Mode.Get().ServingGroup() == g {
+					// Always enforce on serving group
+					return GracefulRotation, nil, nil
+				}
+			case shared.LifecycleVolumeName:
+				// Do nothing
+			default:
+				return GracefulRotation, nil, nil
+			}
+		}
+
+		status.VolumeMounts = spec.VolumeMounts
+		return SilentRotation, nil, nil
+	}
+}
+
+func compareAnyContainerVolumeMounts(ds api.DeploymentSpec, g api.ServerGroup, spec, status *core.Container) comparePodContainerFunc {
+	return func(builder api.ActionBuilder) (mode Mode, plan api.Plan, err error) {
+		specV := mapVolumeMounts(spec)
+		statusV := mapVolumeMounts(status)
+
+		diff := getVolumeMountsDiffFromPods(specV, statusV)
+
+		if len(diff) == 0 {
+			return SkippedRotation, nil, nil
+		}
+
+		for k := range diff {
+			switch k {
+			case shared.LifecycleVolumeName:
+				// Do nothing
+			default:
+				return GracefulRotation, nil, nil
+			}
+		}
+
+		status.VolumeMounts = spec.VolumeMounts
+		return SilentRotation, nil, nil
+	}
+}
+
+type volumeMountDiff struct {
+	a, b []*core.VolumeMount
+}
+
+func getVolumeMountsDiffFromPods(a, b map[string][]*core.VolumeMount) map[string]volumeMountDiff {
+	d := map[string]volumeMountDiff{}
+
+	for k := range a {
+		if z, ok := b[k]; ok {
+			if !reflect.DeepEqual(a[k], z) {
+				d[k] = volumeMountDiff{
+					a: a[k],
+					b: z,
+				}
+			}
+		} else {
+			d[k] = volumeMountDiff{
+				a: a[k],
+				b: nil,
+			}
+		}
+	}
+	for k := range b {
+		if _, ok := a[k]; !ok {
+			d[k] = volumeMountDiff{
+				a: nil,
+				b: a[k],
+			}
+		}
+	}
+
+	return d
+}
+
+func mapVolumeMounts(a *core.Container) map[string][]*core.VolumeMount {
+	n := make(map[string][]*core.VolumeMount, len(a.VolumeMounts))
+
+	for id := range a.VolumeMounts {
+		v := &a.VolumeMounts[id]
+
+		n[v.Name] = append(n[v.Name], v)
+	}
+
+	return n
+}

pkg/deployment/rotation/arangod_volumes.go

@@ -108,83 +108,3 @@ func mapVolumes(a *core.PodSpec) map[string]*core.Volume {
 
 	return n
 }
-
-func compareServerContainerVolumeMounts(ds api.DeploymentSpec, g api.ServerGroup, spec, status *core.Container) comparePodContainerFunc {
-	return func(builder api.ActionBuilder) (mode Mode, plan api.Plan, err error) {
-		specV := mapVolumeMounts(spec)
-		statusV := mapVolumeMounts(status)
-
-		diff := getVolumeMountsDiffFromPods(specV, statusV)
-
-		if len(diff) == 0 {
-			return SkippedRotation, nil, nil
-		}
-
-		for k, v := range diff {
-			switch k {
-			case shared.ArangoDTimezoneVolumeName:
-				// We are fine, should be just replaced
-				if v.a == nil {
-					// we remove volume
-					return GracefulRotation, nil, nil
-				}
-
-				if ds.Mode.Get().ServingGroup() == g {
-					// Always enforce on serving group
-					return GracefulRotation, nil, nil
-				}
-			default:
-				return GracefulRotation, nil, nil
-			}
-		}
-
-		status.VolumeMounts = spec.VolumeMounts
-		return SilentRotation, nil, nil
-	}
-}
-
-type volumeMountDiff struct {
-	a, b []*core.VolumeMount
-}
-
-func getVolumeMountsDiffFromPods(a, b map[string][]*core.VolumeMount) map[string]volumeMountDiff {
-	d := map[string]volumeMountDiff{}
-
-	for k := range a {
-		if z, ok := b[k]; ok {
-			if !reflect.DeepEqual(a[k], z) {
-				d[k] = volumeMountDiff{
-					a: a[k],
-					b: z,
-				}
-			}
-		} else {
-			d[k] = volumeMountDiff{
-				a: a[k],
-				b: nil,
-			}
-		}
-	}
-	for k := range b {
-		if _, ok := a[k]; !ok {
-			d[k] = volumeMountDiff{
-				a: nil,
-				b: a[k],
-			}
-		}
-	}
-
-	return d
-}
-
-func mapVolumeMounts(a *core.Container) map[string][]*core.VolumeMount {
-	n := make(map[string][]*core.VolumeMount, len(a.VolumeMounts))
-
-	for id := range a.VolumeMounts {
-		v := &a.VolumeMounts[id]
-
-		n[v.Name] = append(n[v.Name], v)
-	}
-
-	return n
-}

pkg/util/k8sutil/lifecycle.go

@@ -103,6 +103,25 @@ func NewLifecycle(t string) (*core.Lifecycle, error) {
 	return lifecycle, nil
 }
 
+func AppendLifecycleEnv(in []core.EnvVar) []core.EnvVar {
+	for _, e := range GetLifecycleEnv() {
+		if !EnvExists(in, e.Name) {
+			in = append(in, e)
+		}
+	}
+	return in
+}
+
+func EnvExists(a []core.EnvVar, name string) bool {
+	for _, q := range a {
+		if q.Name == name {
+			return true
+		}
+	}
+
+	return false
+}
+
 func GetLifecycleEnv() []core.EnvVar {
 	return []core.EnvVar{
 		CreateEnvFieldPath(constants.EnvOperatorPodName, "metadata.name"),

pkg/util/k8sutil/volumes.go

@@ -0,0 +1,33 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package k8sutil
+
+import core "k8s.io/api/core/v1"
+
+func VolumeMountExists(vm []core.VolumeMount, name string) bool {
+	for _, m := range vm {
+		if m.Name == name {
+			return true
+		}
+	}
+
+	return false
+}