From 43be10020cbb8d34a48d5088d54896ec0b8dab38 Mon Sep 17 00:00:00 2001
From: Ole Markus With <olemarkus@gmail.com>
Date: Tue, 20 Dec 2022 20:50:02 +0100
Subject: [PATCH] Use enums for needsRollingUpdate in the channels API

---
 channels/pkg/api/channel.go                            | 10 +++++++++-
 channels/pkg/channels/addon.go                         |  4 ++--
 channels/pkg/channels/addons_test.go                   | 10 +++++-----
 .../bootstrapchannelbuilder/bootstrapchannelbuilder.go |  4 ++--
 upup/pkg/fi/cloudup/bootstrapchannelbuilder/cilium.go  |  2 +-
 5 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/channels/pkg/api/channel.go b/channels/pkg/api/channel.go
index 39ddd1da90..2d637ceefa 100644
--- a/channels/pkg/api/channel.go
+++ b/channels/pkg/api/channel.go
@@ -35,6 +35,14 @@ type AddonsSpec struct {
 	Addons []*AddonSpec `json:"addons,omitempty"`
 }
 
+type NeedsRollingUpdate string
+
+const (
+	NeedsRollingUpdateControlPlane NeedsRollingUpdate = "control-plane"
+	NeedsRollingUpdateWorkers      NeedsRollingUpdate = "workers"
+	NeedsRollingUpdateAll          NeedsRollingUpdate = "all"
+)
+
 type AddonSpec struct {
 	Name *string `json:"name,omitempty"`
 
@@ -63,7 +71,7 @@ type AddonSpec struct {
 	// NeedsRollingUpdate determines if we should mark nodes as needing an update.
 	// Legal values are control-plane, workers, and all
 	// Empty value means no update needed
-	NeedsRollingUpdate string `json:"needsRollingUpdate,omitempty"`
+	NeedsRollingUpdate NeedsRollingUpdate `json:"needsRollingUpdate,omitempty"`
 
 	// NeedsPKI determines if channels should provision a CA and a cert-manager issuer for the addon.
 	NeedsPKI bool `json:"needsPKI,omitempty"`
diff --git a/channels/pkg/channels/addon.go b/channels/pkg/channels/addon.go
index 0e8d02c124..a72e0b7184 100644
--- a/channels/pkg/channels/addon.go
+++ b/channels/pkg/channels/addon.go
@@ -233,9 +233,9 @@ func (a *Addon) patchNeedsUpdateLabel(ctx context.Context, k8sClient kubernetes.
 	klog.Infof("addon %v wants to update %v nodes", a.Name, a.Spec.NeedsRollingUpdate)
 	selector := ""
 	switch a.Spec.NeedsRollingUpdate {
-	case "control-plane":
+	case api.NeedsRollingUpdateControlPlane:
 		selector = "node-role.kubernetes.io/master="
-	case "worker":
+	case api.NeedsRollingUpdateWorkers:
 		selector = "node-role.kubernetes.io/node="
 	}
 
diff --git a/channels/pkg/channels/addons_test.go b/channels/pkg/channels/addons_test.go
index 0e3d9133cb..747513b418 100644
--- a/channels/pkg/channels/addons_test.go
+++ b/channels/pkg/channels/addons_test.go
@@ -175,7 +175,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 				Spec: &api.AddonSpec{
 					Name:               fi.PtrTo("test"),
 					ManifestHash:       "originalHash",
-					NeedsRollingUpdate: "all",
+					NeedsRollingUpdate: api.NeedsRollingUpdateAll,
 				},
 			},
 		},
@@ -185,7 +185,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 				Spec: &api.AddonSpec{
 					Name:               fi.PtrTo("test"),
 					ManifestHash:       "newHash",
-					NeedsRollingUpdate: "all",
+					NeedsRollingUpdate: api.NeedsRollingUpdateAll,
 				},
 			},
 			updateRequired:      true,
@@ -197,7 +197,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 				Spec: &api.AddonSpec{
 					Name:               fi.PtrTo("test"),
 					ManifestHash:       "newHash",
-					NeedsRollingUpdate: "worker",
+					NeedsRollingUpdate: api.NeedsRollingUpdateWorkers,
 				},
 			},
 			updateRequired:      true,
@@ -209,7 +209,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 				Spec: &api.AddonSpec{
 					Name:               fi.PtrTo("test"),
 					ManifestHash:       "newHash",
-					NeedsRollingUpdate: "control-plane",
+					NeedsRollingUpdate: api.NeedsRollingUpdateControlPlane,
 				},
 			},
 			updateRequired:      true,
@@ -221,7 +221,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 				Spec: &api.AddonSpec{
 					Name:               fi.PtrTo("test"),
 					ManifestHash:       "newHash",
-					NeedsRollingUpdate: "all",
+					NeedsRollingUpdate: api.NeedsRollingUpdateAll,
 				},
 			},
 			originalAnnotations: map[string]string{
diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go
index 52b10e187c..aaa1d2c637 100644
--- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go
+++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go
@@ -340,7 +340,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.CloudupModelBuilderContext)
 				Name:               fi.PtrTo(key),
 				Selector:           map[string]string{"k8s-addon": key},
 				Manifest:           fi.PtrTo(location),
-				NeedsRollingUpdate: "control-plane",
+				NeedsRollingUpdate: channelsapi.NeedsRollingUpdateControlPlane,
 				Id:                 id,
 			})
 		}
@@ -1057,7 +1057,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.CloudupModelBuilderContext)
 				Selector:           networkingSelector(),
 				Manifest:           fi.PtrTo(location),
 				Id:                 id,
-				NeedsRollingUpdate: "all",
+				NeedsRollingUpdate: channelsapi.NeedsRollingUpdateAll,
 			})
 		}
 	}
diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/cilium.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/cilium.go
index 1f26faf757..ddfd5ce755 100644
--- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/cilium.go
+++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/cilium.go
@@ -35,7 +35,7 @@ func addCiliumAddon(b *BootstrapChannelBuilder, addons *AddonList) error {
 				Selector:           networkingSelector(),
 				Manifest:           fi.PtrTo(location),
 				Id:                 id,
-				NeedsRollingUpdate: "all",
+				NeedsRollingUpdate: api.NeedsRollingUpdateAll,
 			}
 			if cilium.Hubble != nil && fi.ValueOf(cilium.Hubble.Enabled) {
 				addon.NeedsPKI = true