diff --git a/api/v1beta1/installation_types.go b/api/v1beta1/installation_types.go index 3e7380d69..8a1d6db8c 100644 --- a/api/v1beta1/installation_types.go +++ b/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 } diff --git a/controllers/helm.go b/controllers/helm.go index fe5f7d1b5..148ea4f5a 100644 --- a/controllers/helm.go +++ b/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 +} diff --git a/controllers/helm_test.go b/controllers/helm_test.go index b89400efd..8e4d53cb3 100644 --- a/controllers/helm_test.go +++ b/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) + }) + } +} diff --git a/controllers/installation_controller.go b/controllers/installation_controller.go index f578006cd..84c409bd2 100644 --- a/controllers/installation_controller.go +++ b/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) } diff --git a/controllers/installation_controller_test.go b/controllers/installation_controller_test.go index 1b96be6a6..7f23f7e86 100644 --- a/controllers/installation_controller_test.go +++ b/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 {