From a7e0add9a6623bd5a6e5f5fcdf14f44e063b05c1 Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 21 Nov 2022 15:20:30 +0200 Subject: [PATCH 1/3] K8SPXC-1067: fix statefulset updates https://jira.percona.com/browse/K8SPXC-1067 The `updatePod` function missed some fields to update statefulsets and mostly contained duplicated code from `pxc.Statefulset` function. This PR replaces duplicated code with `pxc.Statefulset` and also fixes [K8SPXC-1061](https://jira.percona.com/browse/K8SPXC-1061) --- pkg/controller/pxc/upgrade.go | 189 +++++++++++----------------------- pkg/pxc/statefulset.go | 20 ++-- 2 files changed, 73 insertions(+), 136 deletions(-) diff --git a/pkg/controller/pxc/upgrade.go b/pkg/controller/pxc/upgrade.go index 8a446bab01..66ce5e3012 100644 --- a/pkg/controller/pxc/upgrade.go +++ b/pkg/controller/pxc/upgrade.go @@ -26,55 +26,74 @@ import ( "github.com/percona/percona-xtradb-cluster-operator/pkg/pxc/queries" ) -func (r *ReconcilePerconaXtraDBCluster) updatePod(sfs api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraDBCluster, initContainers []corev1.Container) error { - currentSet := sfs.StatefulSet() +func (r *ReconcilePerconaXtraDBCluster) updatePod(stsApp api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraDBCluster, initContainers []corev1.Container) error { + currentSet := stsApp.StatefulSet() newAnnotations := currentSet.Spec.Template.Annotations // need this step to save all new annotations that was set to currentSet in this reconcile loop err := r.client.Get(context.TODO(), types.NamespacedName{Name: currentSet.Name, Namespace: currentSet.Namespace}, currentSet) if err != nil { return errors.Wrap(err, "failed to get sate") } - currentSet.Spec.UpdateStrategy = sfs.UpdateStrategy(cr) - - // support annotation adjustements - pxc.MergeTemplateAnnotations(currentSet, podSpec.Annotations) - - // change the pod size - currentSet.Spec.Replicas = &podSpec.Size - currentSet.Spec.Template.Spec.SecurityContext = podSpec.PodSecurityContext - currentSet.Spec.Template.Spec.ImagePullSecrets = podSpec.ImagePullSecrets - - if currentSet.Spec.Template.Labels == nil { - currentSet.Spec.Template.Labels = make(map[string]string) + secretsName := cr.Spec.SecretsName + if cr.CompareVersionWith("1.6.0") >= 0 { + secretsName = "internal-" + cr.Name } - for k, v := range podSpec.Labels { - currentSet.Spec.Template.Labels[k] = v + secret := new(corev1.Secret) + err = r.client.Get(context.TODO(), types.NamespacedName{ + Name: secretsName, Namespace: cr.Namespace, + }, secret) + if client.IgnoreNotFound(err) != nil { + return errors.Wrap(err, "get internal secret") } - err = r.reconcileConfigMap(cr) + sts, err := pxc.StatefulSet(stsApp, podSpec, cr, secret, initContainers, r.logger(cr.Name, cr.Namespace), r.getConfigVolume) if err != nil { - return errors.Wrap(err, "upgradePod/updateApp error: update db config error") + return errors.Wrap(err, "pxc statefulset") } + pxc.MergeTemplateAnnotations(sts, newAnnotations) - // embed DB configuration hash - // TODO: code duplication with deploy function - configHash, err := r.getConfigHash(cr, sfs) - if err != nil { - return errors.Wrap(err, "getting config hash") + // keep old labels and annotations + oldLabels := currentSet.Labels + if len(oldLabels) > 0 { + if sts.Labels == nil { + sts.Labels = make(map[string]string) + } + for k, v := range oldLabels { + if _, ok := sts.Labels[k]; !ok { + sts.Labels[k] = v + } + } } - if currentSet.Spec.Template.Annotations == nil { - currentSet.Spec.Template.Annotations = make(map[string]string) + oldAnnotations := currentSet.Annotations + if len(oldAnnotations) > 0 { + if sts.Annotations == nil { + sts.Annotations = make(map[string]string) + } + for k, v := range oldAnnotations { + if _, ok := sts.Annotations[k]; !ok { + sts.Annotations[k] = v + } + } } - pxc.MergeTemplateAnnotations(currentSet, newAnnotations) + err = r.reconcileConfigMap(cr) + if err != nil { + return errors.Wrap(err, "upgradePod/updateApp error: update db config error") + } if cr.CompareVersionWith("1.1.0") >= 0 { - currentSet.Spec.Template.Annotations["percona.com/configuration-hash"] = configHash - } - if cr.CompareVersionWith("1.5.0") >= 0 { - currentSet.Spec.Template.Spec.ServiceAccountName = podSpec.ServiceAccountName + // embed DB configuration hash + // TODO: code duplication with deploy function + configHash, err := r.getConfigHash(cr, stsApp) + if err != nil { + return errors.Wrap(err, "getting config hash") + } + if sts.Spec.Template.Annotations == nil { + sts.Spec.Template.Annotations = make(map[string]string) + } + sts.Spec.Template.Annotations["percona.com/configuration-hash"] = configHash } // change TLS secret configuration @@ -83,7 +102,7 @@ func (r *ReconcilePerconaXtraDBCluster) updatePod(sfs api.StatefulApp, podSpec * return errors.Wrap(err, "upgradePod/updateApp error: update secret error") } if sslHash != "" && cr.CompareVersionWith("1.1.0") >= 0 { - currentSet.Spec.Template.Annotations["percona.com/ssl-hash"] = sslHash + sts.Spec.Template.Annotations["percona.com/ssl-hash"] = sslHash } sslInternalHash, err := r.getSecretHash(cr, cr.Spec.PXC.SSLInternalSecretName, cr.Spec.AllowUnsafeConfig) @@ -91,124 +110,38 @@ func (r *ReconcilePerconaXtraDBCluster) updatePod(sfs api.StatefulApp, podSpec * return errors.Wrap(err, "upgradePod/updateApp error: update secret error") } if sslInternalHash != "" && cr.CompareVersionWith("1.1.0") >= 0 { - currentSet.Spec.Template.Annotations["percona.com/ssl-internal-hash"] = sslInternalHash + sts.Spec.Template.Annotations["percona.com/ssl-internal-hash"] = sslInternalHash } vaultConfigHash, err := r.getSecretHash(cr, cr.Spec.VaultSecretName, true) if err != nil { return errors.Wrap(err, "upgradePod/updateApp error: update secret error") } - if vaultConfigHash != "" && cr.CompareVersionWith("1.6.0") >= 0 && !isHAproxy(sfs) { - currentSet.Spec.Template.Annotations["percona.com/vault-config-hash"] = vaultConfigHash + if vaultConfigHash != "" && cr.CompareVersionWith("1.6.0") >= 0 && !isHAproxy(stsApp) { + sts.Spec.Template.Annotations["percona.com/vault-config-hash"] = vaultConfigHash } if cr.CompareVersionWith("1.9.0") >= 0 { envVarsHash, err := r.getSecretHash(cr, cr.Spec.PXC.EnvVarsSecretName, true) - if isHAproxy(sfs) { + if isHAproxy(stsApp) { envVarsHash, err = r.getSecretHash(cr, cr.Spec.HAProxy.EnvVarsSecretName, true) - } else if isProxySQL(sfs) { + } else if isProxySQL(stsApp) { envVarsHash, err = r.getSecretHash(cr, cr.Spec.ProxySQL.EnvVarsSecretName, true) } if err != nil { return errors.Wrap(err, "upgradePod/updateApp error: update secret error") } if envVarsHash != "" { - currentSet.Spec.Template.Annotations["percona.com/env-secret-config-hash"] = envVarsHash + sts.Spec.Template.Annotations["percona.com/env-secret-config-hash"] = envVarsHash } } - if isHAproxy(sfs) && cr.CompareVersionWith("1.6.0") >= 0 { - delete(currentSet.Spec.Template.Annotations, "percona.com/ssl-internal-hash") - delete(currentSet.Spec.Template.Annotations, "percona.com/ssl-hash") - } - - var newContainers []corev1.Container - var newInitContainers []corev1.Container - - secretsName := cr.Spec.SecretsName - if cr.CompareVersionWith("1.6.0") >= 0 { - secretsName = "internal-" + cr.Name + if isHAproxy(stsApp) && cr.CompareVersionWith("1.6.0") >= 0 { + delete(sts.Spec.Template.Annotations, "percona.com/ssl-internal-hash") + delete(sts.Spec.Template.Annotations, "percona.com/ssl-hash") } - secret := new(corev1.Secret) - err = r.client.Get(context.TODO(), types.NamespacedName{ - Name: secretsName, Namespace: cr.Namespace, - }, secret) - if client.IgnoreNotFound(err) != nil { - return errors.Wrap(err, "get internal secret") - } - // pmm container - if cr.Spec.PMM != nil && cr.Spec.PMM.IsEnabled(secret) { - pmmC, err := sfs.PMMContainer(cr.Spec.PMM, secret, cr) - if err != nil { - return errors.Wrap(err, "pmm container error") - } - if pmmC != nil { - newContainers = append(newContainers, *pmmC) - } - } - - // log-collector container - if cr.Spec.LogCollector != nil && cr.Spec.LogCollector.Enabled && cr.CompareVersionWith("1.7.0") >= 0 { - logCollectorC, err := sfs.LogCollectorContainer(cr.Spec.LogCollector, cr.Spec.LogCollectorSecretName, secretsName, cr) - if err != nil { - return errors.Wrap(err, "logcollector container error") - } - if logCollectorC != nil { - newContainers = append(newContainers, logCollectorC...) - } - } - - // volumes - sfsVolume, err := sfs.Volumes(podSpec, cr, r.getConfigVolume) - if err != nil { - return errors.Wrap(err, "volumes error") - } - - // application container - appC, err := sfs.AppContainer(podSpec, secretsName, cr, sfsVolume.Volumes) - if err != nil { - return errors.Wrap(err, "app container error") - } - - newContainers = append(newContainers, appC) - - if len(initContainers) > 0 { - newInitContainers = append(newInitContainers, initContainers...) - } - - if podSpec.ForceUnsafeBootstrap { - r.log.Info("spec.pxc.forceUnsafeBootstrap option is not supported since v1.10") - - if cr.CompareVersionWith("1.10.0") < 0 { - ic := appC.DeepCopy() - ic.Name = ic.Name + "-init-unsafe" - ic.Resources = podSpec.Resources - ic.ReadinessProbe = nil - ic.LivenessProbe = nil - ic.Command = []string{"/var/lib/mysql/unsafe-bootstrap.sh"} - newInitContainers = append(newInitContainers, *ic) - } - } - - // sidecars - sideC, err := sfs.SidecarContainers(podSpec, secretsName, cr) - if err != nil { - return errors.Wrap(err, "sidecar container error") - } - newContainers = append(newContainers, sideC...) - - newContainers = api.AddSidecarContainers(r.logger(cr.Name, cr.Namespace), newContainers, podSpec.Sidecars) - - currentSet.Spec.Template.Spec.Containers = newContainers - currentSet.Spec.Template.Spec.InitContainers = newInitContainers - currentSet.Spec.Template.Spec.Affinity = pxc.PodAffinity(podSpec.Affinity, sfs) - if sfsVolume != nil && sfsVolume.Volumes != nil { - currentSet.Spec.Template.Spec.Volumes = sfsVolume.Volumes - } - currentSet.Spec.Template.Spec.Volumes = api.AddSidecarVolumes(r.logger(cr.Name, cr.Namespace), currentSet.Spec.Template.Spec.Volumes, podSpec.SidecarVolumes) - currentSet.Spec.Template.Spec.Tolerations = podSpec.Tolerations - err = r.createOrUpdate(cr, currentSet) + err = r.createOrUpdate(cr, sts) if err != nil { return errors.Wrap(err, "update error") } @@ -217,7 +150,7 @@ func (r *ReconcilePerconaXtraDBCluster) updatePod(sfs api.StatefulApp, podSpec * return nil } - return r.smartUpdate(sfs, cr) + return r.smartUpdate(stsApp, cr) } func (r *ReconcilePerconaXtraDBCluster) smartUpdate(sfs api.StatefulApp, cr *api.PerconaXtraDBCluster) error { diff --git a/pkg/pxc/statefulset.go b/pkg/pxc/statefulset.go index e69b5046c0..dee12c4627 100644 --- a/pkg/pxc/statefulset.go +++ b/pkg/pxc/statefulset.go @@ -81,14 +81,18 @@ func StatefulSet(sfs api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraD pod.InitContainers = append(pod.InitContainers, initContainers...) } - if podSpec.ForceUnsafeBootstrap && cr.CompareVersionWith("1.10.0") < 0 { - ic := appC.DeepCopy() - ic.Name = ic.Name + "-init-unsafe" - ic.Resources = podSpec.Resources - ic.ReadinessProbe = nil - ic.LivenessProbe = nil - ic.Command = []string{"/var/lib/mysql/unsafe-bootstrap.sh"} - pod.InitContainers = append(pod.InitContainers, *ic) + if podSpec.ForceUnsafeBootstrap { + log.Info("spec.pxc.forceUnsafeBootstrap option is not supported since v1.10") + + if cr.CompareVersionWith("1.10.0") < 0 { + ic := appC.DeepCopy() + ic.Name = ic.Name + "-init-unsafe" + ic.Resources = podSpec.Resources + ic.ReadinessProbe = nil + ic.LivenessProbe = nil + ic.Command = []string{"/var/lib/mysql/unsafe-bootstrap.sh"} + pod.InitContainers = append(pod.InitContainers, *ic) + } } sideC, err := sfs.SidecarContainers(podSpec, secrets, cr) From cdf3b7e293aa343635912188ff6c29ffd7a8edfa Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 8 Jan 2024 14:34:23 +0200 Subject: [PATCH 2/3] small improvement --- pkg/controller/pxc/upgrade.go | 4 +--- pkg/pxc/statefulset.go | 9 ++++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/controller/pxc/upgrade.go b/pkg/controller/pxc/upgrade.go index 6579102415..9be890ac33 100644 --- a/pkg/controller/pxc/upgrade.go +++ b/pkg/controller/pxc/upgrade.go @@ -27,8 +27,6 @@ import ( ) func (r *ReconcilePerconaXtraDBCluster) updatePod(ctx context.Context, stsApp api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraDBCluster, initContainers []corev1.Container) error { - log := logf.FromContext(ctx) - currentSet := stsApp.StatefulSet() newAnnotations := currentSet.Spec.Template.Annotations // need this step to save all new annotations that was set to currentSet in this reconcile loop err := r.client.Get(context.TODO(), types.NamespacedName{Name: currentSet.Name, Namespace: currentSet.Namespace}, currentSet) @@ -49,7 +47,7 @@ func (r *ReconcilePerconaXtraDBCluster) updatePod(ctx context.Context, stsApp ap return errors.Wrap(err, "get internal secret") } - sts, err := pxc.StatefulSet(stsApp, podSpec, cr, secret, initContainers, r.logger(cr.Name, cr.Namespace), r.getConfigVolume) + sts, err := pxc.StatefulSet(ctx, stsApp, podSpec, cr, secret, initContainers, r.getConfigVolume) if err != nil { return errors.Wrap(err, "pxc statefulset") } diff --git a/pkg/pxc/statefulset.go b/pkg/pxc/statefulset.go index 6afc18f718..3aec8fcfa8 100644 --- a/pkg/pxc/statefulset.go +++ b/pkg/pxc/statefulset.go @@ -1,22 +1,25 @@ package pxc import ( + "context" "fmt" "strings" - "github.com/go-logr/logr" "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" api "github.com/percona/percona-xtradb-cluster-operator/pkg/apis/pxc/v1" ) // StatefulSet returns StatefulSet according for app to podSpec -func StatefulSet(sfs api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraDBCluster, secret *corev1.Secret, - initContainers []corev1.Container, log logr.Logger, vg api.CustomVolumeGetter, +func StatefulSet(ctx context.Context, sfs api.StatefulApp, podSpec *api.PodSpec, cr *api.PerconaXtraDBCluster, secret *corev1.Secret, + initContainers []corev1.Container, vg api.CustomVolumeGetter, ) (*appsv1.StatefulSet, error) { + log := logf.FromContext(ctx) + pod := corev1.PodSpec{ SecurityContext: podSpec.PodSecurityContext, NodeSelector: podSpec.NodeSelector, From 942a5564d177c06326e7894c78b3f4c97be5d1a5 Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 8 Jan 2024 14:39:34 +0200 Subject: [PATCH 3/3] fi --- pkg/controller/pxc/controller.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/controller/pxc/controller.go b/pkg/controller/pxc/controller.go index a1d4fd31b3..fb76da64cb 100644 --- a/pkg/controller/pxc/controller.go +++ b/pkg/controller/pxc/controller.go @@ -519,7 +519,6 @@ func (r *ReconcilePerconaXtraDBCluster) reconcileHAProxy(ctx context.Context, cr } func (r *ReconcilePerconaXtraDBCluster) deploy(ctx context.Context, cr *api.PerconaXtraDBCluster) error { - log := logf.FromContext(ctx) stsApp := statefulset.NewNode(cr) err := r.reconcileConfigMap(cr) if err != nil { @@ -554,7 +553,7 @@ func (r *ReconcilePerconaXtraDBCluster) deploy(ctx context.Context, cr *api.Perc if client.IgnoreNotFound(err) != nil { return errors.Wrap(err, "get internal secret") } - nodeSet, err := pxc.StatefulSet(stsApp, cr.Spec.PXC.PodSpec, cr, secrets, inits, log, r.getConfigVolume) + nodeSet, err := pxc.StatefulSet(ctx, stsApp, cr.Spec.PXC.PodSpec, cr, secrets, inits, r.getConfigVolume) if err != nil { return errors.Wrap(err, "get pxc statefulset") } @@ -657,7 +656,7 @@ func (r *ReconcilePerconaXtraDBCluster) deploy(ctx context.Context, cr *api.Perc // HAProxy StatefulSet if cr.HAProxyEnabled() { sfsHAProxy := statefulset.NewHAProxy(cr) - haProxySet, err := pxc.StatefulSet(sfsHAProxy, &cr.Spec.HAProxy.PodSpec, cr, secrets, proxyInits, log, r.getConfigVolume) + haProxySet, err := pxc.StatefulSet(ctx, sfsHAProxy, &cr.Spec.HAProxy.PodSpec, cr, secrets, proxyInits, r.getConfigVolume) if err != nil { return errors.Wrap(err, "create HAProxy StatefulSet") } @@ -711,7 +710,7 @@ func (r *ReconcilePerconaXtraDBCluster) deploy(ctx context.Context, cr *api.Perc if cr.Spec.ProxySQLEnabled() { sfsProxy := statefulset.NewProxy(cr) - proxySet, err := pxc.StatefulSet(sfsProxy, &cr.Spec.ProxySQL.PodSpec, cr, secrets, proxyInits, log, r.getConfigVolume) + proxySet, err := pxc.StatefulSet(ctx, sfsProxy, &cr.Spec.ProxySQL.PodSpec, cr, secrets, proxyInits, r.getConfigVolume) if err != nil { return errors.Wrap(err, "create ProxySQL Service") }