From 1bc27a37e3525dd969cf9b260ece13261d9c4571 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Fri, 8 Mar 2024 10:20:39 -0800 Subject: [PATCH] Revert "Restart services when their NodeSelector changes" This reverts commit df33f3ef495813f393a1cc9fc18555118463b9a1. PR #353 was thought to be the solution to a problem observed while testing updates to how we manage cinder's LVM backend. This pertained to the use of a nodeSelector label that pins the c-vol pod to the k8s node where the LVM backend data is stored. Unfortunately, two subsequent issues were uncovered. First, the nodeSelector already affects a service's statefulset, which in turn causes pods to be restarted automatically, There's no need to add the nodeSelector to the collection of hashes we use to restart pods when their configuration changes. Second, the actual problem scenario we hoped to solve was getting a pod to be recreated when it got stuck in the Pending state due to a bad nodeSelector value. Currently we use an "Ordered" pod management policy, which means a Pending pod will need to be manually deleted. Hashing the nodeSelector has no impact on pods stuck in the Pending state. --- controllers/cinderapi_controller.go | 14 -------------- controllers/cinderbackup_controller.go | 14 -------------- controllers/cinderscheduler_controller.go | 14 -------------- controllers/cindervolume_controller.go | 14 -------------- pkg/cinder/funcs.go | 11 ----------- 5 files changed, 67 deletions(-) diff --git a/controllers/cinderapi_controller.go b/controllers/cinderapi_controller.go index 9dfa712d..5f6688be 100644 --- a/controllers/cinderapi_controller.go +++ b/controllers/cinderapi_controller.go @@ -706,20 +706,6 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin // all cert input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) - // - // Hash the nodeSelector so the pod is recreated when it changes - // - err = cinder.AddNodeSelectorHash(instance.Spec.NodeSelector, &configVars) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - // // Create secrets required as input for the Service and calculate an overall hash of hashes // diff --git a/controllers/cinderbackup_controller.go b/controllers/cinderbackup_controller.go index 505e1ded..0c31c956 100644 --- a/controllers/cinderbackup_controller.go +++ b/controllers/cinderbackup_controller.go @@ -410,20 +410,6 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance * // all cert input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) - // - // Hash the nodeSelector so the pod is recreated when it changes - // - err = cinder.AddNodeSelectorHash(instance.Spec.NodeSelector, &configVars) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - // // Create secrets required as input for the Service and calculate an overall hash of hashes // diff --git a/controllers/cinderscheduler_controller.go b/controllers/cinderscheduler_controller.go index 072ee50d..ef158015 100644 --- a/controllers/cinderscheduler_controller.go +++ b/controllers/cinderscheduler_controller.go @@ -409,20 +409,6 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc // all cert input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) - // - // Hash the nodeSelector so the pod is recreated when it changes - // - err = cinder.AddNodeSelectorHash(instance.Spec.NodeSelector, &configVars) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - // // Create ConfigMaps required as input for the Service and calculate an overall hash of hashes // diff --git a/controllers/cindervolume_controller.go b/controllers/cindervolume_controller.go index 554f9d78..86a9f5ee 100644 --- a/controllers/cindervolume_controller.go +++ b/controllers/cindervolume_controller.go @@ -434,20 +434,6 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance * return ctrl.Result{}, err } - // - // Hash the nodeSelector so the pod is recreated when it changes - // - err = cinder.AddNodeSelectorHash(instance.Spec.NodeSelector, &configVars) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - // // create hash over all the different input resources to identify if any those changed // and a restart/recreate is required. diff --git a/pkg/cinder/funcs.go b/pkg/cinder/funcs.go index 6b7e39e5..4e86c067 100644 --- a/pkg/cinder/funcs.go +++ b/pkg/cinder/funcs.go @@ -3,8 +3,6 @@ package cinder import ( common "github.com/openstack-k8s-operators/lib-common/modules/common" "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" - "github.com/openstack-k8s-operators/lib-common/modules/common/env" - "github.com/openstack-k8s-operators/lib-common/modules/common/util" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -49,12 +47,3 @@ func GetPodAffinity(componentName string) *corev1.Affinity { corev1.LabelHostname, ) } - -// AddNodeSelectorHash - Adds a hash of a nodeSelector map to the envVars. -func AddNodeSelectorHash(nodeSelector map[string]string, envVars *map[string]env.Setter) error { - hash, err := util.ObjectHash(nodeSelector) - if err != nil { - (*envVars)["NodeSelectorHash"] = env.SetValue(hash) - } - return err -}