change snapinstalldata to pointer

canonical · Oct 11, 2024 · 89922b7 · 89922b7
1 parent ae9f913
commit 89922b7
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 36 deletions.
diff --git a/bootstrap/controllers/ck8sconfig_controller.go b/bootstrap/controllers/ck8sconfig_controller.go
@@ -258,11 +258,14 @@ func (r *CK8sConfigReconciler) joinControlplane(ctx context.Context, scope *Scop
 		return err
 	}
 
-	snapInstallData := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	snapInstallData, err := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	if err != nil {
+		return err
+	}
 
 	// If the machine has an in-place upgrade annotation, use it to set the snap install data
 	inPlaceInstallData := r.resolveInPlaceUpgradeRelease(machine)
-	if inPlaceInstallData.Option != "" && inPlaceInstallData.Value != "" {
+	if inPlaceInstallData != nil {
 		snapInstallData = inPlaceInstallData
 	}
 
@@ -353,7 +356,10 @@ func (r *CK8sConfigReconciler) joinWorker(ctx context.Context, scope *Scope) err
 		return err
 	}
 
-	snapInstallData := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	snapInstallData, err := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	if err != nil {
+		return err
+	}
 
 	// If the machine has an in-place upgrade annotation, use it to set the snap install data
 	inPlaceInstallData := r.resolveInPlaceUpgradeRelease(machine)
@@ -419,67 +425,82 @@ func (r *CK8sConfigReconciler) resolveFiles(ctx context.Context, cfg *bootstrapv
 	return collected, nil
 }
 
-func (r *CK8sConfigReconciler) resolveInPlaceUpgradeRelease(machine *clusterv1.Machine) cloudinit.SnapInstallData {
+func (r *CK8sConfigReconciler) resolveInPlaceUpgradeRelease(machine *clusterv1.Machine) *cloudinit.SnapInstallData {
 	mAnnotations := machine.GetAnnotations()
 
 	if mAnnotations == nil {
-		return cloudinit.SnapInstallData{}
+		return nil
 	}
 
 	val, ok := mAnnotations[bootstrapv1.InPlaceUpgradeReleaseAnnotation]
 	if !ok {
-		return cloudinit.SnapInstallData{}
+		return nil
 	}
 
 	optionKv := strings.Split(val, "=")
 
 	if len(optionKv) != 2 {
 		r.Log.Info("Invalid in-place upgrade release annotation, ignoring", "annotation", val)
-		return cloudinit.SnapInstallData{}
+		return nil
 	}
 
 	switch optionKv[0] {
 	case "channel":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionChannel,
 			Value:  optionKv[1],
 		}
 	case "revision":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionRevision,
 			Value:  optionKv[1],
 		}
 	case "localPath":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionLocalPath,
 			Value:  optionKv[1],
 		}
 	default:
 		r.Log.Info("Unknown in-place upgrade release option, ignoring", "option", optionKv[0])
 	}
 
-	return cloudinit.SnapInstallData{}
+	return nil
 }
 
-func (r *CK8sConfigReconciler) getSnapInstallDataFromSpec(spec bootstrapv1.CK8sConfigSpec) cloudinit.SnapInstallData {
+func (r *CK8sConfigReconciler) getSnapInstallDataFromSpec(spec bootstrapv1.CK8sConfigSpec) (*cloudinit.SnapInstallData, error) {
+	// Ensure that exactly one option is set
+	count := 0
+	if spec.Channel != "" {
+		count++
+	}
+	if spec.Revision != "" {
+		count++
+	}
+	if spec.LocalPath != "" {
+		count++
+	}
+	if count > 1 {
+		return nil, fmt.Errorf("only one of Channel, Revision, or LocalPath can be set, but multiple were provided")
+	}
+
 	switch {
 	case spec.Channel != "":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionChannel,
 			Value:  spec.Channel,
-		}
+		}, nil
 	case spec.Revision != "":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionRevision,
 			Value:  spec.Revision,
-		}
+		}, nil
 	case spec.LocalPath != "":
-		return cloudinit.SnapInstallData{
+		return &cloudinit.SnapInstallData{
 			Option: cloudinit.InstallOptionLocalPath,
 			Value:  spec.LocalPath,
-		}
+		}, nil
 	default:
-		return cloudinit.SnapInstallData{}
+		return &cloudinit.SnapInstallData{}, nil
 	}
 }
 
@@ -621,7 +642,10 @@ func (r *CK8sConfigReconciler) handleClusterNotInitialized(ctx context.Context,
 		return ctrl.Result{}, fmt.Errorf("failed to render k8sd-proxy daemonset: %w", err)
 	}
 
-	snapInstallData := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	snapInstallData, err := r.getSnapInstallDataFromSpec(scope.Config.Spec)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
 	cpinput := cloudinit.InitControlPlaneInput{
 		BaseUserData: cloudinit.BaseUserData{

diff --git a/pkg/cloudinit/common.go b/pkg/cloudinit/common.go
@@ -28,7 +28,7 @@ type BaseUserData struct {
 	// KubernetesVersion is the Kubernetes version from the cluster object.
 	KubernetesVersion string
 	// SnapInstallData is the snap install data.
-	SnapInstallData SnapInstallData
+	SnapInstallData *SnapInstallData
 	// BootCommands is a list of commands to run early in the boot process.
 	BootCommands []string
 	// PreRunCommands is a list of commands to run prior to k8s installation.
@@ -65,9 +65,11 @@ func NewBaseCloudConfig(data BaseUserData) (CloudConfig, error) {
 
 	snapInstall := data.SnapInstallData
 	// Default to k8s version if snap install option is not set or empty.
-	if snapInstall.Option == "" || snapInstall.Value == "" {
-		snapInstall.Option = InstallOptionChannel
-		snapInstall.Value = fmt.Sprintf("%d.%d-classic/stable", kubernetesVersion.Major(), kubernetesVersion.Minor())
+	if snapInstall == nil {
+		snapInstall = &SnapInstallData{
+			Option: InstallOptionChannel,
+			Value:  fmt.Sprintf("%d.%d-classic/stable", kubernetesVersion.Major(), kubernetesVersion.Minor()),
+		}
 	}
 
 	config := CloudConfig{

diff --git a/pkg/cloudinit/controlplane_init_test.go b/pkg/cloudinit/controlplane_init_test.go
@@ -158,28 +158,28 @@ func TestNewInitControlPlaneSnapInstallOverrides(t *testing.T) {
 
 	tests := []struct {
 		name        string
-		snapInstall cloudinit.SnapInstallData
+		snapInstall *cloudinit.SnapInstallData
 		notOptions  []cloudinit.InstallOption
 	}{
 		{
 			name: "ChannelOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionChannel,
 				Value:  "v1.30/edge",
 			},
 			notOptions: []cloudinit.InstallOption{cloudinit.InstallOptionRevision, cloudinit.InstallOptionLocalPath},
 		},
 		{
 			name: "RevisionOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionRevision,
 				Value:  "123",
 			},
 			notOptions: []cloudinit.InstallOption{cloudinit.InstallOptionChannel, cloudinit.InstallOptionLocalPath},
 		},
 		{
 			name: "LocalPathOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionLocalPath,
 				Value:  "/path/to/k8s.snap",
 			},

diff --git a/pkg/cloudinit/controlplane_join_test.go b/pkg/cloudinit/controlplane_join_test.go
@@ -139,25 +139,25 @@ func TestNewJoinControlPlaneSnapInstall(t *testing.T) {
 
 	tests := []struct {
 		name        string
-		snapInstall cloudinit.SnapInstallData
+		snapInstall *cloudinit.SnapInstallData
 	}{
 		{
 			name: "ChannelOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionChannel,
 				Value:  "v1.30/stable",
 			},
 		},
 		{
 			name: "RevisionOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionRevision,
 				Value:  "123",
 			},
 		},
 		{
 			name: "LocalPathOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionLocalPath,
 				Value:  "/path/to/k8s.snap",
 			},

diff --git a/pkg/cloudinit/worker_join_test.go b/pkg/cloudinit/worker_join_test.go
@@ -139,25 +139,25 @@ func TestNewJoinWorkerSnapInstall(t *testing.T) {
 
 	tests := []struct {
 		name        string
-		snapInstall cloudinit.SnapInstallData
+		snapInstall *cloudinit.SnapInstallData
 	}{
 		{
 			name: "ChannelOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionChannel,
 				Value:  "v1.30/stable",
 			},
 		},
 		{
 			name: "RevisionOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionRevision,
 				Value:  "123",
 			},
 		},
 		{
 			name: "LocalPathOverride",
-			snapInstall: cloudinit.SnapInstallData{
+			snapInstall: &cloudinit.SnapInstallData{
 				Option: cloudinit.InstallOptionLocalPath,
 				Value:  "/path/to/k8s.snap",
 			},