From 699e71835a147160711816d1256f7ec071f9636b Mon Sep 17 00:00:00 2001 From: Ivo Petrov Date: Wed, 16 Oct 2024 16:47:46 +0300 Subject: [PATCH] Move away from using annotations in favour of labels (#100) * Move to label usage * Provide better readibility for label addition * Bring back release as annotation --- internal/controller/helm.go | 16 ++++-- internal/controller/reconcile_kubernetes.go | 6 +-- internal/controller/reconcile_os.go | 8 +-- internal/controller/release_manifest.go | 4 +- internal/controller/upgradeplan_controller.go | 44 +++++++--------- internal/upgrade/base.go | 21 ++++---- internal/upgrade/base_test.go | 14 ++--- internal/upgrade/kubernetes.go | 16 +++--- internal/upgrade/kubernetes_test.go | 52 ++++++++++++------- internal/upgrade/os.go | 29 +++++------ internal/upgrade/os_test.go | 32 +++++++----- internal/upgrade/release_manifest.go | 8 +-- internal/upgrade/release_manifest_test.go | 12 ++--- 13 files changed, 137 insertions(+), 125 deletions(-) diff --git a/internal/controller/helm.go b/internal/controller/helm.go index 25a91d9..aee0cc0 100644 --- a/internal/controller/helm.go +++ b/internal/controller/helm.go @@ -86,11 +86,16 @@ func (r *UpgradePlanReconciler) updateHelmChart(ctx context.Context, upgradePlan return fmt.Errorf("merging chart values: %w", err) } + if chart.Labels == nil { + chart.Labels = map[string]string{} + } + if chart.Annotations == nil { chart.Annotations = map[string]string{} } - chart.Annotations[upgrade.PlanNameAnnotation] = upgradePlan.Name - chart.Annotations[upgrade.PlanNamespaceAnnotation] = upgradePlan.Namespace + + chart.Labels[upgrade.PlanNameLabel] = upgradePlan.Name + chart.Labels[upgrade.PlanNamespaceLabel] = upgradePlan.Namespace chart.Annotations[upgrade.ReleaseAnnotation] = upgradePlan.Spec.ReleaseVersion chart.Spec.ChartContent = "" chart.Spec.Chart = releaseChart.Name @@ -120,8 +125,10 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan return fmt.Errorf("merging chart values: %w", err) } - annotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace) - annotations[upgrade.ReleaseAnnotation] = upgradePlan.Spec.ReleaseVersion + labels := upgrade.PlanIdentifierLabels(upgradePlan.Name, upgradePlan.Namespace) + annotations := map[string]string{ + upgrade.ReleaseAnnotation: upgradePlan.Spec.ReleaseVersion, + } chart := &helmcattlev1.HelmChart{ TypeMeta: metav1.TypeMeta{ @@ -131,6 +138,7 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan ObjectMeta: metav1.ObjectMeta{ Name: installedChart.Name, Namespace: upgrade.HelmChartNamespace, + Labels: labels, Annotations: annotations, }, Spec: helmcattlev1.HelmChartSpec{ diff --git a/internal/controller/reconcile_kubernetes.go b/internal/controller/reconcile_kubernetes.go index 986fe49..65eaab1 100644 --- a/internal/controller/reconcile_kubernetes.go +++ b/internal/controller/reconcile_kubernetes.go @@ -31,9 +31,9 @@ func (r *UpgradePlanReconciler) reconcileKubernetes( conditionType := lifecyclev1alpha1.KubernetesUpgradedCondition - identifierAnnotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace) + identifierLabels := upgrade.PlanIdentifierLabels(upgradePlan.Name, upgradePlan.Namespace) drainControlPlane, drainWorker := parseDrainOptions(nodeList, upgradePlan) - controlPlanePlan := upgrade.KubernetesControlPlanePlan(nameSuffix, kubernetesVersion, drainControlPlane, identifierAnnotations) + controlPlanePlan := upgrade.KubernetesControlPlanePlan(nameSuffix, kubernetesVersion, drainControlPlane, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err @@ -56,7 +56,7 @@ func (r *UpgradePlanReconciler) reconcileKubernetes( return ctrl.Result{Requeue: true}, nil } - workerPlan := upgrade.KubernetesWorkerPlan(nameSuffix, kubernetesVersion, drainWorker, identifierAnnotations) + workerPlan := upgrade.KubernetesWorkerPlan(nameSuffix, kubernetesVersion, drainWorker, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(workerPlan), workerPlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err diff --git a/internal/controller/reconcile_os.go b/internal/controller/reconcile_os.go index 2bbaef5..be2431c 100644 --- a/internal/controller/reconcile_os.go +++ b/internal/controller/reconcile_os.go @@ -20,10 +20,10 @@ func (r *UpgradePlanReconciler) reconcileOS( releaseOS *lifecyclev1alpha1.OperatingSystem, nodeList *corev1.NodeList, ) (ctrl.Result, error) { - identifierAnnotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace) + identifierLabels := upgrade.PlanIdentifierLabels(upgradePlan.Name, upgradePlan.Namespace) nameSuffix := upgradePlan.Status.SUCNameSuffix - secret, err := upgrade.OSUpgradeSecret(nameSuffix, releaseOS, identifierAnnotations) + secret, err := upgrade.OSUpgradeSecret(nameSuffix, releaseOS, identifierLabels) if err != nil { return ctrl.Result{}, fmt.Errorf("generating OS upgrade secret: %w", err) } @@ -39,7 +39,7 @@ func (r *UpgradePlanReconciler) reconcileOS( conditionType := lifecyclev1alpha1.OperatingSystemUpgradedCondition drainControlPlane, drainWorker := parseDrainOptions(nodeList, upgradePlan) - controlPlanePlan := upgrade.OSControlPlanePlan(nameSuffix, releaseVersion, secret.Name, releaseOS, drainControlPlane, identifierAnnotations) + controlPlanePlan := upgrade.OSControlPlanePlan(nameSuffix, releaseVersion, secret.Name, releaseOS, drainControlPlane, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err @@ -62,7 +62,7 @@ func (r *UpgradePlanReconciler) reconcileOS( return ctrl.Result{Requeue: true}, nil } - workerPlan := upgrade.OSWorkerPlan(nameSuffix, releaseVersion, secret.Name, releaseOS, drainWorker, identifierAnnotations) + workerPlan := upgrade.OSWorkerPlan(nameSuffix, releaseVersion, secret.Name, releaseOS, drainWorker, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(workerPlan), workerPlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err diff --git a/internal/controller/release_manifest.go b/internal/controller/release_manifest.go index 2cc1a61..cb4a849 100644 --- a/internal/controller/release_manifest.go +++ b/internal/controller/release_manifest.go @@ -34,12 +34,12 @@ func (r *UpgradePlanReconciler) retrieveReleaseManifest(ctx context.Context, upg } func (r *UpgradePlanReconciler) createReleaseManifest(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan) error { - annotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace) + labels := upgrade.PlanIdentifierLabels(upgradePlan.Name, upgradePlan.Namespace) releaseManifest := upgrade.ContainerImage{ Name: r.ReleaseManifestImage, Version: strings.TrimPrefix(upgradePlan.Spec.ReleaseVersion, "v"), } - job, err := upgrade.ReleaseManifestInstallJob(releaseManifest, r.Kubectl, r.ServiceAccount, upgradePlan.Namespace, annotations) + job, err := upgrade.ReleaseManifestInstallJob(releaseManifest, r.Kubectl, r.ServiceAccount, upgradePlan.Namespace, labels) if err != nil { return err } diff --git a/internal/controller/upgradeplan_controller.go b/internal/controller/upgradeplan_controller.go index 5a3f731..ffb74b3 100644 --- a/internal/controller/upgradeplan_controller.go +++ b/internal/controller/upgradeplan_controller.go @@ -116,20 +116,22 @@ func (r *UpgradePlanReconciler) Reconcile(ctx context.Context, req ctrl.Request) } func (r *UpgradePlanReconciler) reconcileDelete(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan) error { - sucPlans := &upgradecattlev1.PlanList{} + labelSelector := client.MatchingLabels{ + upgrade.PlanNameLabel: upgradePlan.Name, + upgrade.PlanNamespaceLabel: upgradePlan.Namespace, + } - if err := r.List(ctx, sucPlans, &client.ListOptions{ + listOpts := &client.ListOptions{ Namespace: upgrade.SUCNamespace, - }); err != nil { + } + labelSelector.ApplyToList(listOpts) + + sucPlans := &upgradecattlev1.PlanList{} + if err := r.List(ctx, sucPlans, listOpts); err != nil { return fmt.Errorf("retrieving SUC plans: %w", err) } for _, plan := range sucPlans.Items { - if plan.Annotations[upgrade.PlanNameAnnotation] != upgradePlan.Name || - plan.Annotations[upgrade.PlanNamespaceAnnotation] != upgradePlan.Namespace { - continue - } - if len(plan.Status.Applying) != 0 { return errUpgradeInProgress } @@ -140,19 +142,11 @@ func (r *UpgradePlanReconciler) reconcileDelete(ctx context.Context, upgradePlan } secrets := &corev1.SecretList{} - - if err := r.List(ctx, secrets, &client.ListOptions{ - Namespace: upgrade.SUCNamespace, - }); err != nil { + if err := r.List(ctx, secrets, listOpts); err != nil { return fmt.Errorf("retrieving SUC secrets: %w", err) } for _, secret := range secrets.Items { - if secret.Annotations[upgrade.PlanNameAnnotation] != upgradePlan.Name || - secret.Annotations[upgrade.PlanNamespaceAnnotation] != upgradePlan.Namespace { - continue - } - if err := r.Delete(ctx, &secret); err != nil { return fmt.Errorf("deleting SUC secret %s: %w", secret.Name, err) } @@ -343,7 +337,7 @@ func setSkippedCondition(plan *lifecyclev1alpha1.UpgradePlan, conditionType, mes func (r *UpgradePlanReconciler) findUpgradePlanFromJob(ctx context.Context, job client.Object) []reconcile.Request { // Check whether the Job was created by the Upgrade Controller first - requests := r.findUpgradePlanFromAnnotations(ctx, job) + requests := r.findUpgradePlanFromLabel(ctx, job) if len(requests) != 0 { return requests } @@ -363,19 +357,19 @@ func (r *UpgradePlanReconciler) findUpgradePlanFromJob(ctx context.Context, job return []reconcile.Request{} } - return r.findUpgradePlanFromAnnotations(ctx, helmChart) + return r.findUpgradePlanFromLabel(ctx, helmChart) } -func (r *UpgradePlanReconciler) findUpgradePlanFromAnnotations(_ context.Context, object client.Object) []reconcile.Request { - annotations := object.GetAnnotations() +func (r *UpgradePlanReconciler) findUpgradePlanFromLabel(_ context.Context, object client.Object) []reconcile.Request { + labels := object.GetLabels() - planName, ok := annotations[upgrade.PlanNameAnnotation] + planName, ok := labels[upgrade.PlanNameLabel] if !ok || planName == "" { // Object is not managed by the Upgrade controller. return []reconcile.Request{} } - planNamespace := annotations[upgrade.PlanNamespaceAnnotation] + planNamespace := labels[upgrade.PlanNamespaceLabel] return []reconcile.Request{ {NamespacedName: types.NamespacedName{Namespace: planNamespace, Name: planName}}, @@ -398,7 +392,7 @@ func (r *UpgradePlanReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&lifecyclev1alpha1.UpgradePlan{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Watches(&upgradecattlev1.Plan{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromAnnotations), builder.WithPredicates(predicate.Funcs{ + Watches(&upgradecattlev1.Plan{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromLabel), builder.WithPredicates(predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return false }, @@ -436,7 +430,7 @@ func (r *UpgradePlanReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, })). - Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromAnnotations), builder.WithPredicates(predicate.Funcs{ + Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromLabel), builder.WithPredicates(predicate.Funcs{ DeleteFunc: func(e event.DeleteEvent) bool { return false }, diff --git a/internal/upgrade/base.go b/internal/upgrade/base.go index c729ee3..1266998 100644 --- a/internal/upgrade/base.go +++ b/internal/upgrade/base.go @@ -11,9 +11,10 @@ import ( ) const ( - PlanNameAnnotation = "lifecycle.suse.com/upgrade-plan-name" - PlanNamespaceAnnotation = "lifecycle.suse.com/upgrade-plan-namespace" - ReleaseAnnotation = "lifecycle.suse.com/release" + PlanNameLabel = "lifecycle.suse.com/upgrade-plan-name" + PlanNamespaceLabel = "lifecycle.suse.com/upgrade-plan-namespace" + + ReleaseAnnotation = "lifecycle.suse.com/release" ControlPlaneLabel = "node-role.kubernetes.io/control-plane" @@ -27,10 +28,10 @@ const ( randomByteNum = 5 ) -func PlanIdentifierAnnotations(name, namespace string) map[string]string { +func PlanIdentifierLabels(name, namespace string) map[string]string { return map[string]string{ - PlanNameAnnotation: name, - PlanNamespaceAnnotation: namespace, + PlanNameLabel: name, + PlanNamespaceLabel: namespace, } } @@ -42,7 +43,7 @@ func GenerateSuffix() (string, error) { return hex.EncodeToString(bytes), nil } -func baseUpgradePlan(name string, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func baseUpgradePlan(name string, drain bool, labels map[string]string) *upgradecattlev1.Plan { const ( kind = "Plan" apiVersion = "upgrade.cattle.io/v1" @@ -55,9 +56,9 @@ func baseUpgradePlan(name string, drain bool, annotations map[string]string) *up APIVersion: apiVersion, }, ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: SUCNamespace, - Annotations: annotations, + Name: name, + Namespace: SUCNamespace, + Labels: labels, }, Spec: upgradecattlev1.PlanSpec{ ServiceAccountName: serviceAccountName, diff --git a/internal/upgrade/base_test.go b/internal/upgrade/base_test.go index e9051d9..d108633 100644 --- a/internal/upgrade/base_test.go +++ b/internal/upgrade/base_test.go @@ -9,12 +9,12 @@ import ( "k8s.io/utils/ptr" ) -func TestPlanIdentifierAnnotations(t *testing.T) { - annotations := PlanIdentifierAnnotations("upgrade-plan-1", "upgrade-controller-system") - require.Len(t, annotations, 2) +func TestPlanIdentifierLabels(t *testing.T) { + labels := PlanIdentifierLabels("upgrade-plan-1", "upgrade-controller-system") + require.Len(t, labels, 2) - assert.Equal(t, "upgrade-plan-1", annotations["lifecycle.suse.com/upgrade-plan-name"]) - assert.Equal(t, "upgrade-controller-system", annotations["lifecycle.suse.com/upgrade-plan-namespace"]) + assert.Equal(t, "upgrade-plan-1", labels["lifecycle.suse.com/upgrade-plan-name"]) + assert.Equal(t, "upgrade-controller-system", labels["lifecycle.suse.com/upgrade-plan-namespace"]) } func TestGenerateSuffix(t *testing.T) { @@ -35,7 +35,7 @@ func TestBaseUpgradePlan_DrainEnabled(t *testing.T) { assert.Equal(t, "upgrade-plan-1", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Nil(t, upgradePlan.ObjectMeta.Annotations) + assert.Nil(t, upgradePlan.ObjectMeta.Labels) assert.Equal(t, "system-upgrade-controller", upgradePlan.Spec.ServiceAccountName) assert.Nil(t, upgradePlan.Spec.Drain) @@ -49,7 +49,7 @@ func TestBaseUpgradePlan_DrainDisabled(t *testing.T) { assert.Equal(t, "upgrade-plan-1", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Nil(t, upgradePlan.ObjectMeta.Annotations) + assert.Nil(t, upgradePlan.ObjectMeta.Labels) assert.Equal(t, "system-upgrade-controller", upgradePlan.Spec.ServiceAccountName) require.NotNil(t, upgradePlan.Spec.Drain) diff --git a/internal/upgrade/kubernetes.go b/internal/upgrade/kubernetes.go index bde274d..7d0bd9a 100644 --- a/internal/upgrade/kubernetes.go +++ b/internal/upgrade/kubernetes.go @@ -27,14 +27,12 @@ func kubernetesUpgradeImage(version string) string { return rke2UpgradeImage } -func KubernetesControlPlanePlan(nameSuffix, version string, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func KubernetesControlPlanePlan(nameSuffix, version string, drain bool, labels map[string]string) *upgradecattlev1.Plan { controlPlanePlanName := kubernetesPlanName(controlPlaneKey, version, nameSuffix) upgradeImage := kubernetesUpgradeImage(version) - controlPlanePlan := baseUpgradePlan(controlPlanePlanName, drain, annotations) - controlPlanePlan.Labels = map[string]string{ - "k8s-upgrade": "control-plane", - } + labels["k8s-upgrade"] = "control-plane" + controlPlanePlan := baseUpgradePlan(controlPlanePlanName, drain, labels) controlPlanePlan.Spec.NodeSelector = &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ { @@ -76,15 +74,13 @@ func KubernetesControlPlanePlan(nameSuffix, version string, drain bool, annotati return controlPlanePlan } -func KubernetesWorkerPlan(nameSuffix, version string, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func KubernetesWorkerPlan(nameSuffix, version string, drain bool, labels map[string]string) *upgradecattlev1.Plan { controlPlanePlanName := kubernetesPlanName(controlPlaneKey, version, nameSuffix) workerPlanName := kubernetesPlanName(workersKey, version, nameSuffix) upgradeImage := kubernetesUpgradeImage(version) - workerPlan := baseUpgradePlan(workerPlanName, drain, annotations) - workerPlan.Labels = map[string]string{ - "k8s-upgrade": "worker", - } + labels["k8s-upgrade"] = "worker" + workerPlan := baseUpgradePlan(workerPlanName, drain, labels) workerPlan.Spec.Concurrency = 1 workerPlan.Spec.NodeSelector = &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ diff --git a/internal/upgrade/kubernetes_test.go b/internal/upgrade/kubernetes_test.go index 56d3679..6e8d78b 100644 --- a/internal/upgrade/kubernetes_test.go +++ b/internal/upgrade/kubernetes_test.go @@ -10,11 +10,16 @@ import ( func TestKubernetesControlPlanePlan_RKE2(t *testing.T) { version := "v1.30.2+rke2r1" - annotations := map[string]string{ + addLabels := map[string]string{ "lifecycle.suse.com/x": "z", } - upgradePlan := KubernetesControlPlanePlan(planNameSuffix, version, false, annotations) + expectedLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + "k8s-upgrade": "control-plane", + } + + upgradePlan := KubernetesControlPlanePlan(planNameSuffix, version, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -22,9 +27,7 @@ func TestKubernetesControlPlanePlan_RKE2(t *testing.T) { assert.Equal(t, "control-plane-v1-30-2-rke2r1-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "control-plane", upgradePlan.ObjectMeta.Labels["k8s-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) @@ -72,11 +75,16 @@ func TestKubernetesControlPlanePlan_RKE2(t *testing.T) { func TestKubernetesControlPlanePlan_K3s(t *testing.T) { version := "v1.30.2+k3s1" - annotations := map[string]string{ + addLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + } + + expectedLabels := map[string]string{ "lifecycle.suse.com/x": "z", + "k8s-upgrade": "control-plane", } - upgradePlan := KubernetesControlPlanePlan(planNameSuffix, version, false, annotations) + upgradePlan := KubernetesControlPlanePlan(planNameSuffix, version, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -84,9 +92,7 @@ func TestKubernetesControlPlanePlan_K3s(t *testing.T) { assert.Equal(t, "control-plane-v1-30-2-k3s1-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "control-plane", upgradePlan.ObjectMeta.Labels["k8s-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) @@ -134,11 +140,16 @@ func TestKubernetesControlPlanePlan_K3s(t *testing.T) { func TestKubernetesWorkerPlan_RKE2(t *testing.T) { version := "v1.30.2+rke2r1" - annotations := map[string]string{ + addLabels := map[string]string{ "lifecycle.suse.com/x": "z", } - upgradePlan := KubernetesWorkerPlan(planNameSuffix, version, false, annotations) + expectedLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + "k8s-upgrade": "worker", + } + + upgradePlan := KubernetesWorkerPlan(planNameSuffix, version, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -146,9 +157,7 @@ func TestKubernetesWorkerPlan_RKE2(t *testing.T) { assert.Equal(t, "workers-v1-30-2-rke2r1-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "worker", upgradePlan.ObjectMeta.Labels["k8s-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) @@ -182,11 +191,16 @@ func TestKubernetesWorkerPlan_RKE2(t *testing.T) { func TestKubernetesWorkerPlan_K3s(t *testing.T) { version := "v1.30.2+k3s1" - annotations := map[string]string{ + addLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + } + + expectedLabels := map[string]string{ "lifecycle.suse.com/x": "z", + "k8s-upgrade": "worker", } - upgradePlan := KubernetesWorkerPlan(planNameSuffix, version, false, annotations) + upgradePlan := KubernetesWorkerPlan(planNameSuffix, version, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -194,9 +208,7 @@ func TestKubernetesWorkerPlan_K3s(t *testing.T) { assert.Equal(t, "workers-v1-30-2-k3s1-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "worker", upgradePlan.ObjectMeta.Labels["k8s-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) diff --git a/internal/upgrade/os.go b/internal/upgrade/os.go index 557ce6d..ac2d906 100644 --- a/internal/upgrade/os.go +++ b/internal/upgrade/os.go @@ -22,7 +22,7 @@ const ( //go:embed templates/os-upgrade.sh.tpl var osUpgradeScript string -func OSUpgradeSecret(nameSuffix string, releaseOS *lifecyclev1alpha1.OperatingSystem, annotations map[string]string) (*corev1.Secret, error) { +func OSUpgradeSecret(nameSuffix string, releaseOS *lifecyclev1alpha1.OperatingSystem, labels map[string]string) (*corev1.Secret, error) { const ( apiVersion = "v1" kind = "Secret" @@ -55,9 +55,9 @@ func OSUpgradeSecret(nameSuffix string, releaseOS *lifecyclev1alpha1.OperatingSy APIVersion: apiVersion, }, ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: SUCNamespace, - Annotations: annotations, + Name: secretName, + Namespace: SUCNamespace, + Labels: labels, }, Type: corev1.SecretTypeOpaque, StringData: map[string]string{ @@ -68,13 +68,11 @@ func OSUpgradeSecret(nameSuffix string, releaseOS *lifecyclev1alpha1.OperatingSy return secret, nil } -func OSControlPlanePlan(nameSuffix, releaseVersion, secretName string, releaseOS *lifecyclev1alpha1.OperatingSystem, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func OSControlPlanePlan(nameSuffix, releaseVersion, secretName string, releaseOS *lifecyclev1alpha1.OperatingSystem, drain bool, labels map[string]string) *upgradecattlev1.Plan { controlPlanePlanName := osPlanName(controlPlaneKey, releaseOS.ZypperID, releaseOS.Version, nameSuffix) - controlPlanePlan := baseOSPlan(controlPlanePlanName, releaseVersion, secretName, drain, annotations) - controlPlanePlan.Labels = map[string]string{ - "os-upgrade": "control-plane", - } + labels["os-upgrade"] = "control-plane" + controlPlanePlan := baseOSPlan(controlPlanePlanName, releaseVersion, secretName, drain, labels) controlPlanePlan.Spec.Concurrency = 1 controlPlanePlan.Spec.NodeSelector = &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -111,14 +109,11 @@ func OSControlPlanePlan(nameSuffix, releaseVersion, secretName string, releaseOS return controlPlanePlan } -func OSWorkerPlan(nameSuffix, releaseVersion, secretName string, releaseOS *lifecyclev1alpha1.OperatingSystem, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func OSWorkerPlan(nameSuffix, releaseVersion, secretName string, releaseOS *lifecyclev1alpha1.OperatingSystem, drain bool, labels map[string]string) *upgradecattlev1.Plan { workerPlanName := osPlanName(workersKey, releaseOS.ZypperID, releaseOS.Version, nameSuffix) - workerPlan := baseOSPlan(workerPlanName, releaseVersion, secretName, drain, annotations) - - workerPlan.Labels = map[string]string{ - "os-upgrade": "worker", - } + labels["os-upgrade"] = "worker" + workerPlan := baseOSPlan(workerPlanName, releaseVersion, secretName, drain, labels) workerPlan.Spec.Concurrency = 2 workerPlan.Spec.NodeSelector = &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -135,12 +130,12 @@ func OSWorkerPlan(nameSuffix, releaseVersion, secretName string, releaseOS *life return workerPlan } -func baseOSPlan(planName, releaseVersion, secretName string, drain bool, annotations map[string]string) *upgradecattlev1.Plan { +func baseOSPlan(planName, releaseVersion, secretName string, drain bool, labels map[string]string) *upgradecattlev1.Plan { const ( planImage = "registry.suse.com/bci/bci-base:15.6" ) - baseOSplan := baseUpgradePlan(planName, drain, annotations) + baseOSplan := baseUpgradePlan(planName, drain, labels) secretPathRelativeToHost := fmt.Sprintf("/run/system-upgrade/secrets/%s", secretName) mountPath := filepath.Join("/host", secretPathRelativeToHost) diff --git a/internal/upgrade/os_test.go b/internal/upgrade/os_test.go index 26f3a6a..c6faebd 100644 --- a/internal/upgrade/os_test.go +++ b/internal/upgrade/os_test.go @@ -20,11 +20,11 @@ func TestOSUpgradeSecret(t *testing.T) { ZypperID: "SL-Micro", CPEScheme: "some-cpe-scheme", } - annotations := map[string]string{ + labels := map[string]string{ "lifecycle.suse.com/x": "z", } - secret, err := OSUpgradeSecret(planNameSuffix, os, annotations) + secret, err := OSUpgradeSecret(planNameSuffix, os, labels) require.NoError(t, err) assert.Equal(t, "Secret", secret.TypeMeta.Kind) @@ -32,7 +32,7 @@ func TestOSUpgradeSecret(t *testing.T) { assert.Equal(t, "os-upgrade-secret-sl-micro-6-0-abcdef", secret.ObjectMeta.Name) assert.Equal(t, "cattle-system", secret.ObjectMeta.Namespace) - assert.Equal(t, annotations, secret.ObjectMeta.Annotations) + assert.Equal(t, labels, secret.ObjectMeta.Labels) assert.EqualValues(t, "Opaque", secret.Type) @@ -50,11 +50,16 @@ func TestOSControlPlanePlan(t *testing.T) { Version: "6.0", ZypperID: "SL-Micro", } - annotations := map[string]string{ + addLabels := map[string]string{ "lifecycle.suse.com/x": "z", } - upgradePlan := OSControlPlanePlan(planNameSuffix, releaseVersion, secretName, os, false, annotations) + expectedLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + "os-upgrade": "control-plane", + } + + upgradePlan := OSControlPlanePlan(planNameSuffix, releaseVersion, secretName, os, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -62,9 +67,7 @@ func TestOSControlPlanePlan(t *testing.T) { assert.Equal(t, "control-plane-sl-micro-6-0-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "control-plane", upgradePlan.ObjectMeta.Labels["os-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) @@ -119,11 +122,16 @@ func TestOSWorkerPlan(t *testing.T) { Version: "6.0", ZypperID: "SL-Micro", } - annotations := map[string]string{ + addLabels := map[string]string{ + "lifecycle.suse.com/x": "z", + } + + expectedLabels := map[string]string{ "lifecycle.suse.com/x": "z", + "os-upgrade": "worker", } - upgradePlan := OSWorkerPlan(planNameSuffix, releaseVersion, secretName, os, false, annotations) + upgradePlan := OSWorkerPlan(planNameSuffix, releaseVersion, secretName, os, false, addLabels) require.NotNil(t, upgradePlan) assert.Equal(t, "Plan", upgradePlan.TypeMeta.Kind) @@ -131,9 +139,7 @@ func TestOSWorkerPlan(t *testing.T) { assert.Equal(t, "workers-sl-micro-6-0-abcdef", upgradePlan.ObjectMeta.Name) assert.Equal(t, "cattle-system", upgradePlan.ObjectMeta.Namespace) - assert.Equal(t, annotations, upgradePlan.ObjectMeta.Annotations) - require.Len(t, upgradePlan.ObjectMeta.Labels, 1) - assert.Equal(t, "worker", upgradePlan.ObjectMeta.Labels["os-upgrade"]) + assert.Equal(t, expectedLabels, upgradePlan.ObjectMeta.Labels) require.Len(t, upgradePlan.Spec.NodeSelector.MatchLabels, 0) require.Len(t, upgradePlan.Spec.NodeSelector.MatchExpressions, 1) diff --git a/internal/upgrade/release_manifest.go b/internal/upgrade/release_manifest.go index 968ae66..796fd49 100644 --- a/internal/upgrade/release_manifest.go +++ b/internal/upgrade/release_manifest.go @@ -18,7 +18,7 @@ func (image ContainerImage) String() string { return fmt.Sprintf("%s:%s", image.Name, image.Version) } -func ReleaseManifestInstallJob(releaseManifest, kubectl ContainerImage, serviceAccount, namespace string, annotations map[string]string) (*batchv1.Job, error) { +func ReleaseManifestInstallJob(releaseManifest, kubectl ContainerImage, serviceAccount, namespace string, labels map[string]string) (*batchv1.Job, error) { if releaseManifest.Name == "" { return nil, fmt.Errorf("release manifest image is empty") } else if releaseManifest.Version == "" { @@ -40,9 +40,9 @@ func ReleaseManifestInstallJob(releaseManifest, kubectl ContainerImage, serviceA Kind: "Job", }, ObjectMeta: metav1.ObjectMeta{ - Name: workloadName, - Namespace: namespace, - Annotations: annotations, + Name: workloadName, + Namespace: namespace, + Labels: labels, }, Spec: batchv1.JobSpec{ Template: corev1.PodTemplateSpec{ diff --git a/internal/upgrade/release_manifest_test.go b/internal/upgrade/release_manifest_test.go index 45af17f..b19b047 100644 --- a/internal/upgrade/release_manifest_test.go +++ b/internal/upgrade/release_manifest_test.go @@ -18,11 +18,11 @@ func TestReleaseManifestInstallJob(t *testing.T) { } serviceAccount := "upgrade-controller-sa" namespace := "upgrade-controller-ns" - annotations := map[string]string{ + labels := map[string]string{ "lifecycle.suse.com/x": "z", } - job, err := ReleaseManifestInstallJob(releaseManifest, kubectl, serviceAccount, namespace, annotations) + job, err := ReleaseManifestInstallJob(releaseManifest, kubectl, serviceAccount, namespace, labels) require.NoError(t, err) assert.Equal(t, "batch/v1", job.TypeMeta.APIVersion) @@ -30,8 +30,8 @@ func TestReleaseManifestInstallJob(t *testing.T) { assert.Equal(t, "apply-release-manifest-3-1-0", job.ObjectMeta.Name) assert.Equal(t, "upgrade-controller-ns", job.ObjectMeta.Namespace) - require.Len(t, job.ObjectMeta.Annotations, 1) - assert.Equal(t, annotations, job.ObjectMeta.Annotations) + require.Len(t, job.ObjectMeta.Labels, 1) + assert.Equal(t, labels, job.ObjectMeta.Labels) assert.Equal(t, "apply-release-manifest-3-1-0", job.Spec.Template.ObjectMeta.Name) assert.Equal(t, "upgrade-controller-ns", job.Spec.Template.ObjectMeta.Namespace) @@ -70,12 +70,12 @@ func TestReleaseManifestInstallJob(t *testing.T) { ttl := int32(0) assert.Equal(t, &ttl, job.Spec.TTLSecondsAfterFinished) - job, err = ReleaseManifestInstallJob(ContainerImage{Version: "3.1.0"}, kubectl, serviceAccount, namespace, annotations) + job, err = ReleaseManifestInstallJob(ContainerImage{Version: "3.1.0"}, kubectl, serviceAccount, namespace, labels) require.Error(t, err) assert.EqualError(t, err, "release manifest image is empty") assert.Nil(t, job) - job, err = ReleaseManifestInstallJob(ContainerImage{Name: "registry.suse.com/edge/release-manifest"}, kubectl, serviceAccount, namespace, annotations) + job, err = ReleaseManifestInstallJob(ContainerImage{Name: "registry.suse.com/edge/release-manifest"}, kubectl, serviceAccount, namespace, labels) require.Error(t, err) assert.EqualError(t, err, "release manifest version is empty") assert.Nil(t, job)