Skip to content

Commit

Permalink
Move away from using annotations in favour of labels (#100)
Browse files Browse the repository at this point in the history 
* Move to label usage

* Provide better readibility for label addition

* Bring back release as annotation
  • Loading branch information
@ipetrov117
ipetrov117 authored Oct 16, 2024
1 parent 705093f commit 699e718
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 125 deletions.
16 changes: 12 additions & 4 deletions internal/controller/helm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/reconcile_kubernetes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/reconcile_os.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/release_manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
44 changes: 19 additions & 25 deletions internal/controller/upgradeplan_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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}},
Expand All @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down
21 changes: 11 additions & 10 deletions internal/upgrade/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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,
}
}

Expand All @@ -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"
Expand All @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions internal/upgrade/base_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
16 changes: 6 additions & 10 deletions internal/upgrade/kubernetes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
Expand Down
Loading

0 comments on commit 699e718

Please sign in to comment.