From a5f646579731ce59995e508daa233c8f2e5602ae Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 15:42:15 +0100 Subject: [PATCH 1/8] Perform unit tests always --- .buildkite/pipeline.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 0824c14a3..285b4a361 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -23,11 +23,6 @@ steps: - key: k8s-operator label: K8s Operator - if: | - build.env("K8S_NIGHTLY") == "1" || - build.branch == "main" || - (build.pull_request.labels includes "k8s/tests") || - build.tag != null timeout: 180 notify: - github_commit_status: From 7616c1114a2cfa6bd69191b21b504b72cccaceb6 Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:04:41 +0100 Subject: [PATCH 2/8] Bump golangci-lint to the latest v1.55.2 --- taskfiles/dev.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taskfiles/dev.yml b/taskfiles/dev.yml index 749be7a96..995eb0573 100644 --- a/taskfiles/dev.yml +++ b/taskfiles/dev.yml @@ -6,7 +6,7 @@ vars: GOLANG_INSTALL_DIR: '{{.BUILD_ROOT}}/go/{{.GOLANG_VERSION}}' KUBECTL_VERSION: '1.28.2' KUBECTL_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/kubectl/{{.KUBECTL_VERSION}}' - GOLANGCI_LINT_VERSION: '1.55.0' + GOLANGCI_LINT_VERSION: '1.55.2' GOLANGCI_LINT_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/golangci-lint/{{.GOLANGCI_LINT_VERSION}}' HELM_VERSION: '3.6.3' HELM_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/helm/{{.HELM_VERSION}}' From 50877e1f3a8bb1a4cd9ae6ee0401f7c6cd5651d9 Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:06:07 +0100 Subject: [PATCH 3/8] Replace repeated strings with consts --- .../redpanda/redpanda_controller.go | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index c27e68b88..355d6a95c 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -53,6 +53,9 @@ const ( resourceTypeHelmRelease = "HelmRelease" managedPath = "/managed" + + revisionPath = "/revision" + componentLabelValue = "redpanda-statefulset" ) // RedpandaReconciler reconciles a Redpanda object @@ -212,7 +215,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update Cluster custom resource" log.V(logger.DebugLevel).Info(msg, "cluster-name", annotatedCluster.Name, "annotations", annotatedCluster.Annotations, "finalizers", annotatedCluster.Finalizers) - r.EventRecorder.AnnotatedEventf(annotatedCluster, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedCluster, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var console vectorzied_v1alpha1.Console @@ -246,7 +249,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update Console custom resource" log.V(logger.DebugLevel).Info(msg, "console-name", annotatedConsole.Name, "annotations", annotatedConsole.Annotations, "finalizers", annotatedConsole.Finalizers) - r.EventRecorder.AnnotatedEventf(annotatedConsole, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedConsole, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var pl v1.PodList @@ -259,14 +262,14 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, } for i := range pl.Items { - if l, exist := pl.Items[i].Labels["app.kubernetes.io/component"]; exist && l == "redpanda-statefulset" && !controllerutil.ContainsFinalizer(&pl.Items[i], FinalizerKey) { + if l, exist := pl.Items[i].Labels["app.kubernetes.io/component"]; exist && l == componentLabelValue && !controllerutil.ContainsFinalizer(&pl.Items[i], FinalizerKey) { continue } newPod := pl.Items[i].DeepCopy() if newPod.Labels == nil { newPod.Labels = make(map[string]string) } - newPod.Labels["app.kubernetes.io/component"] = "redpanda-statefulset" + newPod.Labels["app.kubernetes.io/component"] = componentLabelValue controllerutil.RemoveFinalizer(newPod, FinalizerKey) @@ -277,7 +280,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update Redpanda Pod" log.V(logger.DebugLevel).Info(msg, "pod-name", newPod.Name, "labels", newPod.Labels) - r.EventRecorder.AnnotatedEventf(newPod, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(newPod, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } resourcesName := rp.Name @@ -310,7 +313,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update internal Service" log.V(logger.DebugLevel).Info(msg, "service-name", internalService.Name, "labels", internalService.Labels, "annotations", internalService.Annotations, "selector", internalService.Spec.Selector) - r.EventRecorder.AnnotatedEventf(internalService, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(internalService, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } externalSVCName := fmt.Sprintf("%s-external", resourcesName) @@ -331,7 +334,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update external Service" log.V(logger.DebugLevel).Info(msg, "service-account-name", externalService.Name, "labels", externalService.Labels, "annotations", externalService.Annotations) - r.EventRecorder.AnnotatedEventf(externalService, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(externalService, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var sa v1.ServiceAccount @@ -352,7 +355,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update ServiceAccount" log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedSA.Name, "labels", annotatedSA.Labels, "annotations", annotatedSA.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedSA, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var pdb policyv1.PodDisruptionBudget @@ -373,7 +376,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update PodDistributionBudget" log.V(logger.DebugLevel).Info(msg, "pod-distribution-budget-name", annotatedPDB.Name, "labels", annotatedPDB.Labels, "annotations", annotatedPDB.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedPDB, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedPDB, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var sts appsv1.StatefulSet @@ -394,7 +397,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "delete StatefulSet with orphant propagation mode" log.V(logger.DebugLevel).Info(msg, "stateful-set-name", sts.Name) - r.EventRecorder.AnnotatedEventf(&sts, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(&sts, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } if ptr.Deref(rp.Spec.ClusterSpec.Console.Enabled, true) { @@ -420,7 +423,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update console ServiceAccount" log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedConsoleSA.Name, "labels", annotatedConsoleSA.Labels, "annotations", annotatedConsoleSA.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedConsoleSA, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedConsoleSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } err = r.Get(ctx, types.NamespacedName{ @@ -447,7 +450,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update console Service" log.V(logger.DebugLevel).Info(msg, "service-name", annotatedConsoleSVC.Name, "labels", annotatedConsoleSVC.Labels, "annotations", annotatedConsoleSVC.Annotations, "selector", annotatedConsoleSVC.Spec.Selector) - r.EventRecorder.AnnotatedEventf(annotatedConsoleSVC, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedConsoleSVC, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var deploy appsv1.Deployment @@ -465,7 +468,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "delete console Deployment" log.V(logger.DebugLevel).Info(msg, "deployment-name", deploy.Name) - r.EventRecorder.AnnotatedEventf(&deploy, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(&deploy, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } var ing networkingv1.Ingress @@ -486,7 +489,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, msg := "update console Ingress" log.V(logger.DebugLevel).Info(msg, "ingress-name", annotatedIngress.Name, "labels", annotatedIngress.Labels, "annotations", annotatedIngress.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedIngress, map[string]string{v2.GroupVersion.Group + "/revision": rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + r.EventRecorder.AnnotatedEventf(annotatedIngress, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } } return errorResult @@ -849,7 +852,7 @@ func (r *RedpandaReconciler) patchRedpandaStatus(ctx context.Context, rp *v1alph func (r *RedpandaReconciler) event(rp *v1alpha1.Redpanda, revision, severity, msg string) { var metaData map[string]string if revision != "" { - metaData = map[string]string{v2.GroupVersion.Group + "/revision": revision} + metaData = map[string]string{v2.GroupVersion.Group + revisionPath: revision} } eventType := "Normal" if severity == v1alpha1.EventSeverityError { From 0bf7edaa6a6d6a2c9af560595b0d567fa6c102cc Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:06:32 +0100 Subject: [PATCH 4/8] Allow duplicated code in migration function --- .../controller/redpanda/redpanda_controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index 355d6a95c..725fb211b 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -293,7 +293,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: resourcesName, }, &svc) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get internal service (%s): %w", resourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&svc, rp) || !maps.Equal(svc.Spec.Selector, map[string]string{ "app.kubernetes.io/instance": rp.Name, @@ -321,7 +321,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: externalSVCName, }, &svc) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get external service (%s): %w", externalSVCName, err), errorResult) } else if !hasLabelsAndAnnotations(&svc, rp) { externalService := svc.DeepCopy() @@ -342,7 +342,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: resourcesName, }, &sa) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get service account (%s): %w", resourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&sa, rp) { annotatedSA := sa.DeepCopy() @@ -363,7 +363,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: resourcesName, }, &pdb) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get pod disruption budget (%s): %w", resourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&pdb, rp) { annotatedPDB := pdb.DeepCopy() @@ -410,7 +410,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: consoleResourcesName, }, &sa) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get console service account (%s): %w", consoleResourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&sa, rp) { annotatedConsoleSA := sa.DeepCopy() @@ -430,7 +430,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: consoleResourcesName, }, &svc) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get console service (%s): %w", consoleResourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&svc, rp) || !maps.Equal(svc.Spec.Selector, map[string]string{ "app.kubernetes.io/instance": rp.Name, @@ -476,7 +476,7 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, Namespace: rp.Namespace, Name: consoleResourcesName, }, &ing) - if err != nil { + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value errorResult = errors.Join(fmt.Errorf("get console ingress (%s): %w", consoleResourcesName, err), errorResult) } else if !hasLabelsAndAnnotations(&ing, rp) { annotatedIngress := ing.DeepCopy() From 833c84277b71784b4ecea77ae9efb3ef96dfaa6c Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:07:12 +0100 Subject: [PATCH 5/8] Remove leading new line --- src/go/k8s/internal/controller/redpanda/redpanda_controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index 725fb211b..e13f0b67d 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -236,7 +236,6 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, } else if isConsoleManaged(log, &console) || controllerutil.ContainsFinalizer(&console, consolepkg.ConsoleSAFinalizer) || controllerutil.ContainsFinalizer(&console, consolepkg.ConsoleACLFinalizer) { - annotatedConsole := console.DeepCopy() disableConsoleReconciliation(annotatedConsole) controllerutil.RemoveFinalizer(annotatedConsole, consolepkg.ConsoleSAFinalizer) From 77b439a89f92f85053683cbcf2f14081c4d0539d Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:11:26 +0100 Subject: [PATCH 6/8] Extract Console migration code to its own function --- .../redpanda/redpanda_controller.go | 177 ++++++++++-------- 1 file changed, 95 insertions(+), 82 deletions(-) diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index e13f0b67d..752b959c3 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -400,96 +400,109 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, } if ptr.Deref(rp.Spec.ClusterSpec.Console.Enabled, true) { - log.V(logger.DebugLevel).Info("migrate console") - consoleResourcesName := rp.Name - if overwriteSAName := ptr.Deref(rp.Spec.ClusterSpec.Console.FullNameOverride, ""); overwriteSAName != "" { - consoleResourcesName = overwriteSAName - } - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &sa) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get console service account (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&sa, rp) { - annotatedConsoleSA := sa.DeepCopy() - setHelmLabelsAndAnnotations(annotatedConsoleSA, rp) - - err = r.Update(ctx, annotatedConsoleSA) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating console service account (%s): %w", annotatedConsoleSA.Name, err), errorResult) - } + err = r.tryMigrateConsole(ctx, log, rp) + if err != nil { + errorResult = errors.Join(err, errorResult) + } + } + return errorResult +} - msg := "update console ServiceAccount" - log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedConsoleSA.Name, "labels", annotatedConsoleSA.Labels, "annotations", annotatedConsoleSA.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedConsoleSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &svc) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get console service (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&svc, rp) || !maps.Equal(svc.Spec.Selector, map[string]string{ - "app.kubernetes.io/instance": rp.Name, - "app.kubernetes.io/name": "console", - }) { - annotatedConsoleSVC := svc.DeepCopy() - setHelmLabelsAndAnnotations(annotatedConsoleSVC, rp) - - annotatedConsoleSVC.Spec.Selector = make(map[string]string) - annotatedConsoleSVC.Spec.Selector["app.kubernetes.io/instance"] = rp.Name - annotatedConsoleSVC.Spec.Selector["app.kubernetes.io/name"] = "console" - - err = r.Update(ctx, annotatedConsoleSVC) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating console service (%s): %w", annotatedConsoleSVC.Name, err), errorResult) - } +func (r *RedpandaReconciler) tryMigrateConsole(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) error { + log.V(logger.DebugLevel).Info("migrate console") + consoleResourcesName := rp.Name + if overwriteSAName := ptr.Deref(rp.Spec.ClusterSpec.Console.FullNameOverride, ""); overwriteSAName != "" { + consoleResourcesName = overwriteSAName + } + + var errorResult error + + var sa v1.ServiceAccount + err := r.Get(ctx, types.NamespacedName{ + Namespace: rp.Namespace, + Name: consoleResourcesName, + }, &sa) + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value + errorResult = errors.Join(fmt.Errorf("get console service account (%s): %w", consoleResourcesName, err), errorResult) + } else if !hasLabelsAndAnnotations(&sa, rp) { + annotatedConsoleSA := sa.DeepCopy() + setHelmLabelsAndAnnotations(annotatedConsoleSA, rp) + + err = r.Update(ctx, annotatedConsoleSA) + if err != nil { + errorResult = errors.Join(fmt.Errorf("updating console service account (%s): %w", annotatedConsoleSA.Name, err), errorResult) + } + + msg := "update console ServiceAccount" + log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedConsoleSA.Name, "labels", annotatedConsoleSA.Labels, "annotations", annotatedConsoleSA.Annotations) + r.EventRecorder.AnnotatedEventf(annotatedConsoleSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + } + + var svc v1.Service + err = r.Get(ctx, types.NamespacedName{ + Namespace: rp.Namespace, + Name: consoleResourcesName, + }, &svc) + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value + errorResult = errors.Join(fmt.Errorf("get console service (%s): %w", consoleResourcesName, err), errorResult) + } else if !hasLabelsAndAnnotations(&svc, rp) || !maps.Equal(svc.Spec.Selector, map[string]string{ + "app.kubernetes.io/instance": rp.Name, + "app.kubernetes.io/name": "console", + }) { + annotatedConsoleSVC := svc.DeepCopy() + setHelmLabelsAndAnnotations(annotatedConsoleSVC, rp) + + annotatedConsoleSVC.Spec.Selector = make(map[string]string) + annotatedConsoleSVC.Spec.Selector["app.kubernetes.io/instance"] = rp.Name + annotatedConsoleSVC.Spec.Selector["app.kubernetes.io/name"] = "console" - msg := "update console Service" - log.V(logger.DebugLevel).Info(msg, "service-name", annotatedConsoleSVC.Name, "labels", annotatedConsoleSVC.Labels, "annotations", annotatedConsoleSVC.Annotations, "selector", annotatedConsoleSVC.Spec.Selector) - r.EventRecorder.AnnotatedEventf(annotatedConsoleSVC, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + err = r.Update(ctx, annotatedConsoleSVC) + if err != nil { + errorResult = errors.Join(fmt.Errorf("updating console service (%s): %w", annotatedConsoleSVC.Name, err), errorResult) } - var deploy appsv1.Deployment - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &deploy) + msg := "update console Service" + log.V(logger.DebugLevel).Info(msg, "service-name", annotatedConsoleSVC.Name, "labels", annotatedConsoleSVC.Labels, "annotations", annotatedConsoleSVC.Annotations, "selector", annotatedConsoleSVC.Spec.Selector) + r.EventRecorder.AnnotatedEventf(annotatedConsoleSVC, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + } + + var deploy appsv1.Deployment + err = r.Get(ctx, types.NamespacedName{ + Namespace: rp.Namespace, + Name: consoleResourcesName, + }, &deploy) + if err != nil { + errorResult = errors.Join(fmt.Errorf("get console deployment (%s): %w", consoleResourcesName, err), errorResult) + } else if !hasLabelsAndAnnotations(&deploy, rp) { + err = r.Delete(ctx, &deploy) if err != nil { - errorResult = errors.Join(fmt.Errorf("get console deployment (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&sts, rp) { - err = r.Delete(ctx, &deploy) - if err != nil { - errorResult = errors.Join(fmt.Errorf("deleting console deployment (%s): %w", deploy.Name, err), errorResult) - } + errorResult = errors.Join(fmt.Errorf("deleting console deployment (%s): %w", deploy.Name, err), errorResult) + } - msg := "delete console Deployment" - log.V(logger.DebugLevel).Info(msg, "deployment-name", deploy.Name) - r.EventRecorder.AnnotatedEventf(&deploy, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - - var ing networkingv1.Ingress - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &ing) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get console ingress (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&ing, rp) { - annotatedIngress := ing.DeepCopy() - setHelmLabelsAndAnnotations(annotatedIngress, rp) - - err = r.Update(ctx, annotatedIngress) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating console ingress (%s): %w", annotatedIngress.Name, err), errorResult) - } + msg := "delete console Deployment" + log.V(logger.DebugLevel).Info(msg, "deployment-name", deploy.Name) + r.EventRecorder.AnnotatedEventf(&deploy, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + } - msg := "update console Ingress" - log.V(logger.DebugLevel).Info(msg, "ingress-name", annotatedIngress.Name, "labels", annotatedIngress.Labels, "annotations", annotatedIngress.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedIngress, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + var ing networkingv1.Ingress + err = r.Get(ctx, types.NamespacedName{ + Namespace: rp.Namespace, + Name: consoleResourcesName, + }, &ing) + if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value + errorResult = errors.Join(fmt.Errorf("get console ingress (%s): %w", consoleResourcesName, err), errorResult) + } else if !hasLabelsAndAnnotations(&ing, rp) { + annotatedIngress := ing.DeepCopy() + setHelmLabelsAndAnnotations(annotatedIngress, rp) + + err = r.Update(ctx, annotatedIngress) + if err != nil { + errorResult = errors.Join(fmt.Errorf("updating console ingress (%s): %w", annotatedIngress.Name, err), errorResult) } + + msg := "update console Ingress" + log.V(logger.DebugLevel).Info(msg, "ingress-name", annotatedIngress.Name, "labels", annotatedIngress.Labels, "annotations", annotatedIngress.Annotations) + r.EventRecorder.AnnotatedEventf(annotatedIngress, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } return errorResult } From 3e48ea1448363f83c98e5cba8dcd3109085f9b8d Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:40:38 +0100 Subject: [PATCH 7/8] Refactor Redpanda migration function --- .../api/redpanda/v1alpha1/redpanda_types.go | 52 ++++ .../redpanda/redpanda_controller.go | 253 +++++++----------- 2 files changed, 148 insertions(+), 157 deletions(-) diff --git a/src/go/k8s/api/redpanda/v1alpha1/redpanda_types.go b/src/go/k8s/api/redpanda/v1alpha1/redpanda_types.go index 766f8bf0c..aaa03c226 100644 --- a/src/go/k8s/api/redpanda/v1alpha1/redpanda_types.go +++ b/src/go/k8s/api/redpanda/v1alpha1/redpanda_types.go @@ -227,3 +227,55 @@ func (in *Redpanda) OwnerShipRefObj() metav1.OwnerReference { UID: in.UID, } } + +// GetMigrationConsoleName returns Console custom resource namespace which will be taken out from +// old reconciler, so that underlying resources could be migrated. +func (in *Redpanda) GetMigrationConsoleName() string { + if in.Spec.Migration == nil { + return "" + } + name := in.Spec.Migration.ConsoleRef.Name + if name == "" { + name = in.Name + } + return name +} + +// GetMigrationConsoleNamespace returns Console custom resource name which will be taken out from +// old reconciler, so that underlying resources could be migrated. +func (in *Redpanda) GetMigrationConsoleNamespace() string { + if in.Spec.Migration == nil { + return "" + } + namespace := in.Spec.Migration.ConsoleRef.Namespace + if namespace == "" { + namespace = in.Namespace + } + return namespace +} + +// GetMigrationClusterName returns Cluster custom resource namespace which will be taken out from +// old reconciler, so that underlying resources could be migrated. +func (in *Redpanda) GetMigrationClusterName() string { + if in.Spec.Migration == nil { + return "" + } + name := in.Spec.Migration.ClusterRef.Name + if name == "" { + name = in.Name + } + return name +} + +// GetMigrationClusterNamespace returns Cluster custom resource name which will be taken out from +// old reconciler, so that underlying resources could be migrated. +func (in *Redpanda) GetMigrationClusterNamespace() string { + if in.Spec.Migration == nil { + return "" + } + namespace := in.Spec.Migration.ClusterRef.Namespace + if namespace == "" { + namespace = in.Namespace + } + return namespace +} diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index 752b959c3..bd5579dba 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -24,7 +24,6 @@ import ( "github.com/fluxcd/pkg/runtime/logger" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/go-logr/logr" - consolepkg "github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/console" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -43,6 +42,7 @@ import ( "github.com/redpanda-data/redpanda-operator/src/go/k8s/api/redpanda/v1alpha1" vectorzied_v1alpha1 "github.com/redpanda-data/redpanda-operator/src/go/k8s/api/vectorized/v1alpha1" + consolepkg "github.com/redpanda-data/redpanda-operator/src/go/k8s/pkg/console" ) const ( @@ -185,25 +185,25 @@ func (r *RedpandaReconciler) Reconcile(c context.Context, req ctrl.Request) (ctr return result, err } +type resourceToMigrate struct { + resourceName string + helperString string + resource client.Object +} + func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) error { log = log.WithName("tryMigration") var errorResult error var cluster vectorzied_v1alpha1.Cluster - namespace := rp.Spec.Migration.ClusterRef.Namespace - if namespace == "" { - namespace = rp.Namespace - } - name := rp.Spec.Migration.ClusterRef.Name - if name == "" { - name = rp.Name - } + clusterNamespace := rp.GetMigrationClusterNamespace() + clusterName := rp.GetMigrationClusterName() err := r.Get(ctx, types.NamespacedName{ - Namespace: namespace, - Name: name, + Namespace: clusterNamespace, + Name: clusterName, }, &cluster) if err != nil { - errorResult = errors.Join(fmt.Errorf("get cluster reference (%s/%s): %w", namespace, name, err), errorResult) + errorResult = errors.Join(fmt.Errorf("get cluster reference (%s/%s): %w", clusterNamespace, clusterName, err), errorResult) } else if isRedpandaClusterManaged(log, &cluster) { annotatedCluster := cluster.DeepCopy() disableRedpandaReconciliation(annotatedCluster) @@ -219,20 +219,14 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, } var console vectorzied_v1alpha1.Console - namespace = rp.Spec.Migration.ConsoleRef.Namespace - if namespace == "" { - namespace = rp.Namespace - } - name = rp.Spec.Migration.ConsoleRef.Name - if name == "" { - name = rp.Name - } + consoleNamespace := rp.GetMigrationConsoleNamespace() + consoleName := rp.GetMigrationConsoleName() err = r.Get(ctx, types.NamespacedName{ - Namespace: namespace, - Name: name, + Namespace: consoleNamespace, + Name: consoleName, }, &console) if err != nil { - errorResult = errors.Join(fmt.Errorf("get cluster reference (%s/%s): %w", namespace, name, err), errorResult) + errorResult = errors.Join(fmt.Errorf("get cluster reference (%s/%s): %w", consoleNamespace, consoleName, err), errorResult) } else if isConsoleManaged(log, &console) || controllerutil.ContainsFinalizer(&console, consolepkg.ConsoleSAFinalizer) || controllerutil.ContainsFinalizer(&console, consolepkg.ConsoleACLFinalizer) { @@ -251,44 +245,58 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, r.EventRecorder.AnnotatedEventf(annotatedConsole, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } - var pl v1.PodList - err = r.List(ctx, &pl, []client.ListOption{ - client.InNamespace(rp.Namespace), - client.MatchingLabels(map[string]string{"app.kubernetes.io/instance": rp.Name, "app.kubernetes.io/name": "redpanda"}), - }...) + redpandaResourcesToMigrate, err := r.tryMigrateRedpanda(ctx, log, rp) if err != nil { - errorResult = errors.Join(fmt.Errorf("listing pods: %w", err), errorResult) + errorResult = errors.Join(err, errorResult) } - for i := range pl.Items { - if l, exist := pl.Items[i].Labels["app.kubernetes.io/component"]; exist && l == componentLabelValue && !controllerutil.ContainsFinalizer(&pl.Items[i], FinalizerKey) { - continue - } - newPod := pl.Items[i].DeepCopy() - if newPod.Labels == nil { - newPod.Labels = make(map[string]string) - } - newPod.Labels["app.kubernetes.io/component"] = componentLabelValue - - controllerutil.RemoveFinalizer(newPod, FinalizerKey) + var allResourcesToMigrate []resourceToMigrate + allResourcesToMigrate = append(allResourcesToMigrate, redpandaResourcesToMigrate...) - err = r.Update(ctx, newPod) + if ptr.Deref(rp.Spec.ClusterSpec.Console.Enabled, true) { + consoleResourcesToMigrate, err := r.tryMigrateConsole(ctx, log, rp) if err != nil { - errorResult = errors.Join(fmt.Errorf("updating component Pod label (%s): %w", newPod.Name, err), errorResult) + errorResult = errors.Join(err, errorResult) } + allResourcesToMigrate = append(allResourcesToMigrate, consoleResourcesToMigrate...) + } - msg := "update Redpanda Pod" - log.V(logger.DebugLevel).Info(msg, "pod-name", newPod.Name, "labels", newPod.Labels) - r.EventRecorder.AnnotatedEventf(newPod, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + for _, obj := range allResourcesToMigrate { + err := r.Get(ctx, types.NamespacedName{ + Namespace: rp.Namespace, + Name: obj.resourceName, + }, obj.resource) + if err != nil { + errorResult = errors.Join(fmt.Errorf("get %s (%s): %w", obj.helperString, obj.resourceName, err), errorResult) + } else if !hasLabelsAndAnnotations(obj.resource, rp) { + annotatedObject := obj.resource.DeepCopyObject() + setHelmLabelsAndAnnotations(annotatedObject.(client.Object), rp) + + resourceName := annotatedObject.(client.Object).GetName() + err = r.Update(ctx, annotatedObject.(client.Object)) + if err != nil { + errorResult = errors.Join(fmt.Errorf("updating %s (%s): %w", obj.helperString, resourceName, err), errorResult) + } + + msg := "update " + obj.helperString + log.V(logger.DebugLevel).Info(msg, "object-name", resourceName, "labels", annotatedObject.(client.Object).GetLabels(), "annotations", annotatedObject.(client.Object).GetAnnotations()) + r.EventRecorder.AnnotatedEventf(annotatedObject, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) + } } + return errorResult +} + +func (r *RedpandaReconciler) tryMigrateRedpanda(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) ([]resourceToMigrate, error) { + errorResult := r.migrateRedpandaPods(ctx, log, rp) + resourcesName := rp.Name if rp.Spec.ClusterSpec.FullNameOverride != "" { resourcesName = rp.Spec.ClusterSpec.FullNameOverride } var svc v1.Service - err = r.Get(ctx, types.NamespacedName{ + err := r.Get(ctx, types.NamespacedName{ Namespace: rp.Namespace, Name: resourcesName, }, &svc) @@ -315,69 +323,6 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, r.EventRecorder.AnnotatedEventf(internalService, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } - externalSVCName := fmt.Sprintf("%s-external", resourcesName) - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: externalSVCName, - }, &svc) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get external service (%s): %w", externalSVCName, err), errorResult) - } else if !hasLabelsAndAnnotations(&svc, rp) { - externalService := svc.DeepCopy() - setHelmLabelsAndAnnotations(externalService, rp) - - err = r.Update(ctx, externalService) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating external service (%s): %w", externalService.Name, err), errorResult) - } - - msg := "update external Service" - log.V(logger.DebugLevel).Info(msg, "service-account-name", externalService.Name, "labels", externalService.Labels, "annotations", externalService.Annotations) - r.EventRecorder.AnnotatedEventf(externalService, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - - var sa v1.ServiceAccount - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: resourcesName, - }, &sa) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get service account (%s): %w", resourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&sa, rp) { - annotatedSA := sa.DeepCopy() - setHelmLabelsAndAnnotations(annotatedSA, rp) - - err = r.Update(ctx, annotatedSA) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating service account (%s): %w", annotatedSA.Name, err), errorResult) - } - - msg := "update ServiceAccount" - log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedSA.Name, "labels", annotatedSA.Labels, "annotations", annotatedSA.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - - var pdb policyv1.PodDisruptionBudget - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: resourcesName, - }, &pdb) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get pod disruption budget (%s): %w", resourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&pdb, rp) { - annotatedPDB := pdb.DeepCopy() - setHelmLabelsAndAnnotations(annotatedPDB, rp) - - err = r.Update(ctx, annotatedPDB) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating pod disruption budget (%s): %w", annotatedPDB.Name, err), errorResult) - } - - msg := "update PodDistributionBudget" - log.V(logger.DebugLevel).Info(msg, "pod-distribution-budget-name", annotatedPDB.Name, "labels", annotatedPDB.Labels, "annotations", annotatedPDB.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedPDB, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - var sts appsv1.StatefulSet err = r.Get(ctx, types.NamespacedName{ Namespace: rp.Namespace, @@ -398,17 +343,50 @@ func (r *RedpandaReconciler) tryMigration(ctx context.Context, log logr.Logger, log.V(logger.DebugLevel).Info(msg, "stateful-set-name", sts.Name) r.EventRecorder.AnnotatedEventf(&sts, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } + return []resourceToMigrate{ + {fmt.Sprintf("%s-external", resourcesName), "external Service", &svc}, + {resourcesName, "ServiceAccount", &v1.ServiceAccount{}}, + {resourcesName, "PodDistributionBudget", &policyv1.PodDisruptionBudget{}}, + }, errorResult +} - if ptr.Deref(rp.Spec.ClusterSpec.Console.Enabled, true) { - err = r.tryMigrateConsole(ctx, log, rp) +func (r *RedpandaReconciler) migrateRedpandaPods(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) error { + var errorResult error + + var pl v1.PodList + err := r.List(ctx, &pl, []client.ListOption{ + client.InNamespace(rp.Namespace), + client.MatchingLabels(map[string]string{"app.kubernetes.io/instance": rp.Name, "app.kubernetes.io/name": "redpanda"}), + }...) + if err != nil { + errorResult = errors.Join(fmt.Errorf("listing pods: %w", err), errorResult) + } + + for i := range pl.Items { + if l, exist := pl.Items[i].Labels["app.kubernetes.io/component"]; exist && l == componentLabelValue && !controllerutil.ContainsFinalizer(&pl.Items[i], FinalizerKey) { + continue + } + newPod := pl.Items[i].DeepCopy() + if newPod.Labels == nil { + newPod.Labels = make(map[string]string) + } + newPod.Labels["app.kubernetes.io/component"] = componentLabelValue + + controllerutil.RemoveFinalizer(newPod, FinalizerKey) + + err = r.Update(ctx, newPod) if err != nil { - errorResult = errors.Join(err, errorResult) + errorResult = errors.Join(fmt.Errorf("updating component Pod label (%s): %w", newPod.Name, err), errorResult) } + + msg := "update Redpanda Pod" + log.V(logger.DebugLevel).Info(msg, "pod-name", newPod.Name, "labels", newPod.Labels, "finalizers", newPod.Finalizers) + r.EventRecorder.AnnotatedEventf(newPod, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } return errorResult } -func (r *RedpandaReconciler) tryMigrateConsole(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) error { +func (r *RedpandaReconciler) tryMigrateConsole(ctx context.Context, log logr.Logger, rp *v1alpha1.Redpanda) ([]resourceToMigrate, error) { log.V(logger.DebugLevel).Info("migrate console") consoleResourcesName := rp.Name if overwriteSAName := ptr.Deref(rp.Spec.ClusterSpec.Console.FullNameOverride, ""); overwriteSAName != "" { @@ -417,29 +395,8 @@ func (r *RedpandaReconciler) tryMigrateConsole(ctx context.Context, log logr.Log var errorResult error - var sa v1.ServiceAccount - err := r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &sa) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get console service account (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&sa, rp) { - annotatedConsoleSA := sa.DeepCopy() - setHelmLabelsAndAnnotations(annotatedConsoleSA, rp) - - err = r.Update(ctx, annotatedConsoleSA) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating console service account (%s): %w", annotatedConsoleSA.Name, err), errorResult) - } - - msg := "update console ServiceAccount" - log.V(logger.DebugLevel).Info(msg, "service-account-name", annotatedConsoleSA.Name, "labels", annotatedConsoleSA.Labels, "annotations", annotatedConsoleSA.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedConsoleSA, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - var svc v1.Service - err = r.Get(ctx, types.NamespacedName{ + err := r.Get(ctx, types.NamespacedName{ Namespace: rp.Namespace, Name: consoleResourcesName, }, &svc) @@ -483,28 +440,10 @@ func (r *RedpandaReconciler) tryMigrateConsole(ctx context.Context, log logr.Log log.V(logger.DebugLevel).Info(msg, "deployment-name", deploy.Name) r.EventRecorder.AnnotatedEventf(&deploy, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) } - - var ing networkingv1.Ingress - err = r.Get(ctx, types.NamespacedName{ - Namespace: rp.Namespace, - Name: consoleResourcesName, - }, &ing) - if err != nil { // nolint:dupl // Repetition in tryMigration function is acceptable as generalised function would not bring any value - errorResult = errors.Join(fmt.Errorf("get console ingress (%s): %w", consoleResourcesName, err), errorResult) - } else if !hasLabelsAndAnnotations(&ing, rp) { - annotatedIngress := ing.DeepCopy() - setHelmLabelsAndAnnotations(annotatedIngress, rp) - - err = r.Update(ctx, annotatedIngress) - if err != nil { - errorResult = errors.Join(fmt.Errorf("updating console ingress (%s): %w", annotatedIngress.Name, err), errorResult) - } - - msg := "update console Ingress" - log.V(logger.DebugLevel).Info(msg, "ingress-name", annotatedIngress.Name, "labels", annotatedIngress.Labels, "annotations", annotatedIngress.Annotations) - r.EventRecorder.AnnotatedEventf(annotatedIngress, map[string]string{v2.GroupVersion.Group + revisionPath: rp.Status.LastAttemptedRevision}, "Normal", v1alpha1.EventSeverityInfo, msg) - } - return errorResult + return []resourceToMigrate{ + {consoleResourcesName, "console ServiceAccount", &v1.ServiceAccount{}}, + {consoleResourcesName, "console Ingress", &networkingv1.Ingress{}}, + }, errorResult } func hasLabelsAndAnnotations(object client.Object, rp *v1alpha1.Redpanda) bool { From 3133680648040ec1c9a290c4c5440dfc1ca65d29 Mon Sep 17 00:00:00 2001 From: Rafal Korepta Date: Mon, 20 Nov 2023 16:43:37 +0100 Subject: [PATCH 8/8] Exteract error as static error --- .../k8s/internal/controller/redpanda/redpanda_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go index bd5579dba..64a1dc86f 100644 --- a/src/go/k8s/internal/controller/redpanda/redpanda_controller.go +++ b/src/go/k8s/internal/controller/redpanda/redpanda_controller.go @@ -58,6 +58,8 @@ const ( componentLabelValue = "redpanda-statefulset" ) +var errWaitForReleaseDeletion = errors.New("wait for helm release deletion") + // RedpandaReconciler reconciles a Redpanda object type RedpandaReconciler struct { client.Client @@ -702,7 +704,7 @@ func (r *RedpandaReconciler) deleteHelmRelease(ctx context.Context, rp *v1alpha1 return fmt.Errorf("deleting helm release connected with Redpanda (%s): %w", rp.Name, err) } - return errors.New("wait for helm release deletion") + return errWaitForReleaseDeletion } func (r *RedpandaReconciler) createHelmReleaseFromTemplate(ctx context.Context, rp *v1alpha1.Redpanda) (*helmv2beta1.HelmRelease, error) {