Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Ensure ConfigMap and StatefulSet updates are applied during operator upgrades #1619

Merged
merged 12 commits into from
Jan 9, 2025
68 changes: 42 additions & 26 deletions controllers/argocd/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,48 +651,42 @@ func (r *ReconcileArgoCD) reconcileRedisConfiguration(cr *argoproj.ArgoCD, useTL
// reconcileRedisHAConfigMap will ensure that the Redis HA Health ConfigMap is present for the given ArgoCD.
func (r *ReconcileArgoCD) reconcileRedisHAHealthConfigMap(cr *argoproj.ArgoCD, useTLSForRedis bool) error {
cm := newConfigMapWithName(common.ArgoCDRedisHAHealthConfigMapName, cr)
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, cm) {
if !cr.Spec.HA.Enabled {
// ConfigMap exists but HA enabled flag has been set to false, delete the ConfigMap
argoutil.LogResourceDeletion(log, cm, "redis ha is disabled")
return r.Client.Delete(context.TODO(), cm)
}
return nil // ConfigMap found with nothing changed, move along...
}

if !cr.Spec.HA.Enabled {
return nil // HA not enabled, do nothing.
}

cm.Data = map[string]string{
"redis_liveness.sh": getRedisLivenessScript(useTLSForRedis),
"redis_readiness.sh": getRedisReadinessScript(useTLSForRedis),
"sentinel_liveness.sh": getSentinelLivenessScript(useTLSForRedis),
}
if !cr.Spec.HA.Enabled {
// If HA is not enabled, delete the ConfigMap if it exists
existingCM := &corev1.ConfigMap{}
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM) {
return r.Client.Delete(context.TODO(), existingCM)
Mangaal marked this conversation as resolved.
Show resolved Hide resolved
}
return nil // Nothing to do since HA is not enabled and ConfigMap does not exist
}

if err := controllerutil.SetControllerReference(cr, cm, r.Scheme); err != nil {
return err
}

existingCM := &corev1.ConfigMap{}
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM) {
// Check if the data has changed
if !reflect.DeepEqual(cm.Data, existingCM.Data) {
existingCM.Data = cm.Data
argoutil.LogResourceUpdate(log, existingCM, "updating", "Redis HA Health ConfigMap")
return r.Client.Update(context.TODO(), existingCM)
}
return nil // No changes detected
}
argoutil.LogResourceCreation(log, cm)
return r.Client.Create(context.TODO(), cm)
}

// reconcileRedisHAConfigMap will ensure that the Redis HA ConfigMap is present for the given ArgoCD.
func (r *ReconcileArgoCD) reconcileRedisHAConfigMap(cr *argoproj.ArgoCD, useTLSForRedis bool) error {
// Create or update the ConfigMap if HA is enabled
cm := newConfigMapWithName(common.ArgoCDRedisHAConfigMapName, cr)
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, cm) {
if !cr.Spec.HA.Enabled {
// ConfigMap exists but HA enabled flag has been set to false, delete the ConfigMap
argoutil.LogResourceDeletion(log, cm, "redis ha is disabled")
return r.Client.Delete(context.TODO(), cm)
}
return nil // ConfigMap found with nothing changed, move along...
}

if !cr.Spec.HA.Enabled {
return nil // HA not enabled, do nothing.
}

cm.Data = map[string]string{
"haproxy.cfg": getRedisHAProxyConfig(cr, useTLSForRedis),
"haproxy_init.sh": getRedisHAProxyScript(cr),
Expand All @@ -701,10 +695,32 @@ func (r *ReconcileArgoCD) reconcileRedisHAConfigMap(cr *argoproj.ArgoCD, useTLSF
"sentinel.conf": getRedisSentinelConf(useTLSForRedis),
}

if !cr.Spec.HA.Enabled {
// If HA is not enabled, delete the ConfigMap if it exists
existingCM := &corev1.ConfigMap{}
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM) {
return r.Client.Delete(context.TODO(), existingCM)
Mangaal marked this conversation as resolved.
Show resolved Hide resolved
}
return nil // Nothing to do since HA is not enabled and ConfigMap does not exist
}

// Set the ownership reference
if err := controllerutil.SetControllerReference(cr, cm, r.Scheme); err != nil {
return err
}

existingCM := &corev1.ConfigMap{}
if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM) {
// Check if the data has changed
if !reflect.DeepEqual(cm.Data, existingCM.Data) {
existingCM.Data = cm.Data
argoutil.LogResourceUpdate(log, existingCM, "updating", "Redis HA ConfigMap")
return r.Client.Update(context.TODO(), existingCM)
}
return nil // No changes detected
}
argoutil.LogResourceCreation(log, cm)
// Create the ConfigMap if it does not exist
return r.Client.Create(context.TODO(), cm)
}

Expand Down
74 changes: 74 additions & 0 deletions controllers/argocd/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,80 @@ func TestReconcileArgoCD_reconcileTLSCerts_configMapUpdate(t *testing.T) {
}
}

// TestReconcileArgoCD_reconcileRedisHAHealthConfigMap tests the reconcileRedisHAHealthConfigMap function.
func TestReconcileArgoCD_reconcileRedisHAHealthConfigMap(t *testing.T) {
logf.SetLogger(ZapLogger(true))

// Create a test ArgoCD resource with HA enabled
cr := makeTestArgoCD()
cr.Spec.HA.Enabled = true

// Initialize test objects
resObjs := []client.Object{cr}
subresObjs := []client.Object{}
runtimeObjs := []runtime.Object{}
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
r := makeTestReconciler(cl, sch)

// Perform initial reconciliation
assert.NoError(t, r.reconcileRedisHAHealthConfigMap(cr, false))

// Modify ConfigMap data to simulate external changes
existingCM := &corev1.ConfigMap{}
assert.True(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAHealthConfigMapName, existingCM))
existingCM.Data["redis_liveness.sh"] = "modified_script_content"
assert.NoError(t, cl.Update(context.TODO(), existingCM))

// Reconcile again and verify changes are reverted
assert.NoError(t, r.reconcileRedisHAHealthConfigMap(cr, false))
existingCMAfter := &corev1.ConfigMap{}
assert.True(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAHealthConfigMapName, existingCMAfter))
assert.Equal(t, getRedisLivenessScript(false), existingCMAfter.Data["redis_liveness.sh"])

// Disable HA and ensure ConfigMap is deleted
cr.Spec.HA.Enabled = false
assert.NoError(t, r.reconcileRedisHAHealthConfigMap(cr, false))
assert.False(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAHealthConfigMapName, existingCM))
}

// TestReconcileArgoCD_reconcileRedisHAConfigMap tests the reconcileRedisHAConfigMap function.
func TestReconcileArgoCD_reconcileRedisHAConfigMap(t *testing.T) {
logf.SetLogger(ZapLogger(true))

// Create a test ArgoCD resource with HA enabled
cr := makeTestArgoCD()
cr.Spec.HA.Enabled = true

// Initialize test objects
resObjs := []client.Object{cr}
subresObjs := []client.Object{}
runtimeObjs := []runtime.Object{}
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
r := makeTestReconciler(cl, sch)

// Perform initial reconciliation
assert.NoError(t, r.reconcileRedisHAConfigMap(cr, false))

// Modify ConfigMap data to simulate external changes
existingCM := &corev1.ConfigMap{}
assert.True(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAConfigMapName, existingCM))
existingCM.Data["haproxy.cfg"] = "modified_config_content"
assert.NoError(t, cl.Update(context.TODO(), existingCM))

// Reconcile again and verify changes are reverted
assert.NoError(t, r.reconcileRedisHAConfigMap(cr, false))
existingCMAfter := &corev1.ConfigMap{}
assert.True(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAConfigMapName, existingCMAfter))
assert.Equal(t, getRedisHAProxyConfig(cr, false), existingCMAfter.Data["haproxy.cfg"])

// Disable HA and ensure ConfigMap is deleted
cr.Spec.HA.Enabled = false
assert.NoError(t, r.reconcileRedisHAConfigMap(cr, false))
assert.False(t, argoutil.IsObjectFound(cl, cr.Namespace, common.ArgoCDRedisHAConfigMapName, existingCM))
}

func TestReconcileArgoCD_reconcileTLSCerts_withInitialCertsUpdate(t *testing.T) {
logf.SetLogger(ZapLogger(true))
a := makeTestArgoCD()
Expand Down
10 changes: 10 additions & 0 deletions controllers/argocd/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,11 @@ func (r *ReconcileArgoCD) reconcileRedisStatefulSet(cr *argoproj.ArgoCD) error {
explanation += fmt.Sprintf("container '%s' security context", container.Name)
changed = true
}

if !reflect.DeepEqual(ss.Spec.Template.Spec.Containers[i].Env, existing.Spec.Template.Spec.Containers[i].Env) {
existing.Spec.Template.Spec.Containers[i].Env = ss.Spec.Template.Spec.Containers[i].Env
changed = true
}
}

if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers[0].Resources, existing.Spec.Template.Spec.InitContainers[0].Resources) {
Expand All @@ -490,6 +495,11 @@ func (r *ReconcileArgoCD) reconcileRedisStatefulSet(cr *argoproj.ArgoCD) error {
changed = true
}

if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers[0].Env, existing.Spec.Template.Spec.InitContainers[0].Env) {
existing.Spec.Template.Spec.InitContainers[0].Env = ss.Spec.Template.Spec.InitContainers[0].Env
changed = true
}

if changed {
argoutil.LogResourceUpdate(log, existing, "updating", explanation)
return r.Client.Update(context.TODO(), existing)
Expand Down
Loading