do not update charts if they are still being created (#86)

* watch k0s chart objects

* do not update k0s cluster object if it is still applying charts

api/v1beta1/installation_types.go

@@ -32,6 +32,7 @@ const (
 	InstallationStateObsolete               string = "Obsolete"
 	InstallationStateFailed                 string = "Failed"
 	InstallationStateUnknown                string = "Unknown"
+	InstallationStatePendingChartCreation   string = "PendingChartCreation"
 )
 
 // NodeStatus is used to keep track of the status of a cluster node, we
@@ -77,6 +78,7 @@ func (s *InstallationStatus) GetKubernetesInstalled() bool {
 	if s.State == InstallationStateInstalled ||
 		s.State == InstallationStateKubernetesInstalled ||
 		s.State == InstallationStateAddonsInstalling ||
+		s.State == InstallationStatePendingChartCreation ||
 		s.State == InstallationStateHelmChartUpdateFailure {
 		return true
 	}

controllers/helm.go

@@ -177,3 +177,38 @@ func generateDesiredCharts(meta *release.Meta, clusterconfig k0sv1beta1.ClusterC
 	}
 	return finalChartList, nil
 }
+
+// shouldNotUpdateClusterConfig returns true if there are charts within the clusterConfig that have not yet been applied
+// to the cluster. if we update the cluster config while charts are still being applied, k0s may attempt to apply the
+// same chart twice in parallel, which causes an error
+// 'can't install loadedChart `CHART_NAME_HERE`: cannot re-use a name that is still in use'
+// returns the list of charts that are still being applied
+func shouldNotUpdateClusterConfig(configCharts *k0sv1beta1.HelmExtensions, charts k0shelm.ChartList) []string {
+	pendingCharts := []string{}
+	if configCharts == nil {
+		return pendingCharts
+	}
+
+	for _, specChart := range configCharts.Charts {
+		foundChart := false
+		for _, clusterChart := range charts.Items {
+			if specChart.Name == clusterChart.Spec.ReleaseName {
+				foundChart = true
+
+				if clusterChart.Status.ReleaseName == "" {
+					// this chart has not yet been applied to the cluster, otherwise the ReleaseName would be set
+					pendingCharts = append(pendingCharts, specChart.Name)
+				}
+
+				break
+			}
+		}
+
+		if !foundChart {
+			// this chart is present in the spec, but not the cluster, and thus is still being applied
+			pendingCharts = append(pendingCharts, specChart.Name)
+		}
+	}
+
+	return pendingCharts
+}

controllers/helm_test.go

@@ -589,3 +589,146 @@ func Test_generateDesiredCharts(t *testing.T) {
 		})
 	}
 }
+
+func Test_shouldNotUpdateClusterConfig(t *testing.T) {
+	tests := []struct {
+		name         string
+		configCharts *k0sv1beta1.HelmExtensions
+		charts       k0shelm.ChartList
+		want         []string
+	}{
+		{
+			name:         "no charts",
+			configCharts: nil,
+			want:         []string{},
+		},
+		{
+			name:         "no config charts",
+			configCharts: &k0sv1beta1.HelmExtensions{},
+			want:         []string{},
+		},
+		{
+			name: "all charts present",
+			configCharts: &k0sv1beta1.HelmExtensions{
+				Charts: []k0sv1beta1.Chart{
+					{
+						Name: "test",
+					},
+					{
+						Name: "test2",
+					},
+				},
+			},
+			charts: k0shelm.ChartList{
+				Items: []k0shelm.Chart{
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "test",
+						},
+						Status: k0shelm.ChartStatus{
+							ReleaseName: "test",
+						},
+					},
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "test2",
+						},
+						Status: k0shelm.ChartStatus{
+							ReleaseName: "test2",
+						},
+					},
+				},
+			},
+			want: []string{},
+		},
+		{
+			name: "one chart not present in cluster",
+			configCharts: &k0sv1beta1.HelmExtensions{
+				Charts: []k0sv1beta1.Chart{
+					{
+						Name: "test",
+					},
+					{
+						Name: "test2",
+					},
+				},
+			},
+			charts: k0shelm.ChartList{
+				Items: []k0shelm.Chart{
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "test2",
+						},
+						Status: k0shelm.ChartStatus{
+							ReleaseName: "test2",
+						},
+					},
+				},
+			},
+			want: []string{"test"},
+		},
+		{
+			name: "chart present but not yet applied",
+			configCharts: &k0sv1beta1.HelmExtensions{
+				Charts: []k0sv1beta1.Chart{
+					{
+						Name: "test",
+					},
+				},
+			},
+			charts: k0shelm.ChartList{
+				Items: []k0shelm.Chart{
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "test",
+						},
+						Status: k0shelm.ChartStatus{},
+					},
+				},
+			},
+			want: []string{"test"},
+		},
+		{
+			name: "one chart ok, one not applied, one not present",
+			configCharts: &k0sv1beta1.HelmExtensions{
+				Charts: []k0sv1beta1.Chart{
+					{
+						Name: "okchart",
+					},
+					{
+						Name: "notapplied",
+					},
+					{
+						Name: "notpresent",
+					},
+				},
+			},
+			charts: k0shelm.ChartList{
+				Items: []k0shelm.Chart{
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "okchart",
+						},
+						Status: k0shelm.ChartStatus{
+							ReleaseName: "okchart",
+						},
+					},
+					{
+						Spec: k0shelm.ChartSpec{
+							ReleaseName: "notapplied",
+						},
+						Status: k0shelm.ChartStatus{},
+					},
+				},
+			},
+			want: []string{"notapplied", "notpresent"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := require.New(t)
+			got := shouldNotUpdateClusterConfig(tt.configCharts, tt.charts)
+			req.ElementsMatch(tt.want, got)
+		})
+	}
+}

controllers/installation_controller.go

@@ -327,12 +327,17 @@ func (r *InstallationReconciler) ReconcileHelmCharts(ctx context.Context, in *v1
 		return nil
 	}
 
-	if in.Status.State != v1beta1.InstallationStateKubernetesInstalled {
+	if in.Status.State == v1beta1.InstallationStateAddonsInstalling {
 		// after the first time we apply new helm charts, this will be set to InstallationStateAddonsInstalling
 		// and we will not re-apply the charts to the k0s cluster config while waiting for those changes to propagate
 		return nil
 	}
 
+	if pendingCharts := shouldNotUpdateClusterConfig(clusterconfig.Spec.Extensions.Helm, installedCharts); len(pendingCharts) != 0 {
+		in.Status.SetState(v1beta1.InstallationStatePendingChartCreation, fmt.Sprintf("Pending charts: %v", pendingCharts))
+		return nil
+	}
+
 	// Replace the current chart configs with the new chart configs
 	clusterconfig.Spec.Extensions.Helm = combinedConfigs
 	in.Status.SetState(v1beta1.InstallationStateAddonsInstalling, "Installing addons")
@@ -549,5 +554,6 @@ func (r *InstallationReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&v1beta1.Installation{}).
 		Watches(&corev1.Node{}, &handler.EnqueueRequestForObject{}).
 		Watches(&apv1b2.Plan{}, &handler.EnqueueRequestForObject{}).
+		Watches(&k0shelm.Chart{}, &handler.EnqueueRequestForObject{}).
 		Complete(r)
 }

controllers/installation_controller_test.go

@@ -443,14 +443,14 @@ password: overridden`,
 							Values: `abc: original
 password: original`,
 						},
-						Status: k0shelmv1beta1.ChartStatus{Version: "1"},
+						Status: k0shelmv1beta1.ChartStatus{Version: "1", ReleaseName: "metachart"},
 					},
 					&k0shelmv1beta1.Chart{
 						ObjectMeta: metav1.ObjectMeta{
 							Name: "extchart",
 						},
 						Spec:   k0shelmv1beta1.ChartSpec{ReleaseName: "extchart"},
-						Status: k0shelmv1beta1.ChartStatus{Version: "2"},
+						Status: k0shelmv1beta1.ChartStatus{Version: "2", ReleaseName: "extchart"},
 					},
 					&k0sv1beta1.ClusterConfig{
 						ObjectMeta: metav1.ObjectMeta{
@@ -480,6 +480,65 @@ password: original`,
 				},
 			},
 		},
+		{
+			name: "k8s install completed, values drift but chart not yet installed",
+			in: v1beta1.Installation{
+				Status: v1beta1.InstallationStatus{State: v1beta1.InstallationStateKubernetesInstalled},
+				Spec: v1beta1.InstallationSpec{
+					Config: &v1beta1.ConfigSpec{
+						Version: "goodver",
+					},
+				},
+			},
+			out: v1beta1.InstallationStatus{
+				State:  v1beta1.InstallationStatePendingChartCreation,
+				Reason: "Pending charts: [metachart]",
+			},
+			releaseMeta: release.Meta{
+				Configs: &k0sv1beta1.HelmExtensions{
+					Charts: []k0sv1beta1.Chart{
+						{
+							Name:    "metachart",
+							Version: "1",
+							Values:  `abc: xyz`,
+						},
+					},
+				},
+			},
+			fields: fields{
+				State: []runtime.Object{
+					&k0shelmv1beta1.Chart{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "metachart",
+						},
+						Spec: k0shelmv1beta1.ChartSpec{
+							ReleaseName: "metachart",
+							Values:      `abc: original`,
+						},
+						Status: k0shelmv1beta1.ChartStatus{},
+					},
+					&k0sv1beta1.ClusterConfig{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "k0s",
+							Namespace: "kube-system",
+						},
+						Spec: &k0sv1beta1.ClusterSpec{
+							Extensions: &k0sv1beta1.ClusterExtensions{
+								Helm: &k0sv1beta1.HelmExtensions{
+									Charts: []k0sv1beta1.Chart{
+										{
+											Name:    "metachart",
+											Version: "1",
+											Values:  `abc: original`,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {