Cleanup unknown Helm chart manifest files

Keep track of the generated filenames when reconciling Helm Chart
extensions. After all files have been synchronized, remove any remaining
unknown Helm chart manifest files. This way the synchronization will
work correctly even if the Helm chart extension names and orders are
changed.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
(cherry picked from commit c1f8c75)

cmd/controller/controller.go

@@ -418,7 +418,6 @@ func (c *command) start(ctx context.Context) error {
 		}
 		clusterComponents.Add(ctx, controller.NewCRD(helmSaver, []string{"helm"}))
 		clusterComponents.Add(ctx, controller.NewExtensionsController(
-			helmSaver,
 			c.K0sVars,
 			adminClientFactory,
 			leaderElector,

inttest/addons/addons_test.go

@@ -21,13 +21,17 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/k0sproject/k0s/internal/pkg/templatewriter"
 	"github.com/k0sproject/k0s/inttest/common"
-	"github.com/k0sproject/k0s/pkg/apis/helm/v1beta1"
+	helmv1beta1 "github.com/k0sproject/k0s/pkg/apis/helm/v1beta1"
+	k0sv1beta1 "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
+	k0sclientset "github.com/k0sproject/k0s/pkg/client/clientset"
+	"github.com/k0sproject/k0s/pkg/constant"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -54,7 +58,7 @@ func (as *AddonsSuite) TestHelmBasedAddons() {
 	fileAddonName := "tgz-addon"
 	as.PutFile(as.ControllerNode(0), "/tmp/k0s.yaml", fmt.Sprintf(k0sConfigWithAddon, addonName))
 	as.pullHelmChart(as.ControllerNode(0))
-	as.Require().NoError(as.InitController(0, "--config=/tmp/k0s.yaml"))
+	as.Require().NoError(as.InitController(0, "--config=/tmp/k0s.yaml", "--enable-dynamic-config"))
 	as.NoError(as.RunWorkers())
 	kc, err := as.KubeClient(as.ControllerNode(0))
 	as.Require().NoError(err)
@@ -66,6 +70,8 @@ func (as *AddonsSuite) TestHelmBasedAddons() {
 
 	as.AssertSomeKubeSystemPods(kc)
 
+	as.Run("Rename chart in Helm extension", func() { as.renameChart(ctx) })
+
 	values := map[string]interface{}{
 		"replicaCount": 2,
 		"image": map[string]interface{}{
@@ -93,7 +99,47 @@ func (as *AddonsSuite) pullHelmChart(node string) {
 	as.Require().NoError(err)
 }
 
-func (as *AddonsSuite) deleteRelease(chart *v1beta1.Chart) {
+func (as *AddonsSuite) renameChart(ctx context.Context) {
+	restConfig, err := as.GetKubeConfig(as.ControllerNode(0))
+	as.Require().NoError(err)
+	k0sClients, err := k0sclientset.NewForConfig(restConfig)
+	as.Require().NoError(err)
+
+	configs := k0sClients.K0sV1beta1().ClusterConfigs(constant.ClusterConfigNamespace)
+	cfg, err := configs.Get(ctx, constant.ClusterConfigObjectName, metav1.GetOptions{})
+	as.Require().NoError(err)
+
+	i := slices.IndexFunc(cfg.Spec.Extensions.Helm.Charts, func(c k0sv1beta1.Chart) bool {
+		return c.Name == "tgz-addon"
+	})
+	as.Require().GreaterOrEqual(i, 0, "Didn't find tgz-addon in %v", cfg.Spec.Extensions.Helm.Charts)
+	cfg.Spec.Extensions.Helm.Charts[i].Name = "tgz-renamed-addon"
+
+	cfg, err = configs.Update(ctx, cfg, metav1.UpdateOptions{FieldManager: as.T().Name()})
+	as.Require().NoError(err)
+	if data, err := yaml.Marshal(cfg); as.NoError(err) {
+		as.T().Logf("%s", data)
+	}
+
+	as.Require().NoError(wait.PollUntilContextCancel(ctx, 350*time.Millisecond, true, func(ctx context.Context) (bool, error) {
+		charts, err := k0sClients.HelmV1beta1().Charts(constant.ClusterConfigNamespace).List(ctx, metav1.ListOptions{})
+		if err != nil {
+			return false, nil
+		}
+
+		hasChart := func(name string) bool {
+			return slices.IndexFunc(charts.Items, func(c helmv1beta1.Chart) bool {
+				return c.Name == name
+			}) >= 0
+		}
+
+		return !hasChart("k0s-addon-chart-tgz-addon") && hasChart("k0s-addon-chart-tgz-renamed-addon"), nil
+	}), "While waiting for Chart resource to be swapped")
+
+	as.waitForTestRelease("tgz-renamed-addon", "0.6.0", "kube-system", 1)
+}
+
+func (as *AddonsSuite) deleteRelease(chart *helmv1beta1.Chart) {
 	ctx := as.Context()
 	as.T().Logf("Deleting chart %s/%s", chart.Namespace, chart.Name)
 	ssh, err := as.SSH(ctx, as.ControllerNode(0))
@@ -124,15 +170,15 @@ func (as *AddonsSuite) deleteRelease(chart *v1beta1.Chart) {
 		return true, nil
 	}))
 
-	helmScheme, err := v1beta1.SchemeBuilder.Build()
+	helmScheme, err := helmv1beta1.SchemeBuilder.Build()
 	as.Require().NoError(err)
 	chartClient, err := client.New(cfg, client.Options{Scheme: helmScheme})
 	as.Require().NoError(err)
 
 	as.T().Logf("Expecting chart %s/%s to be deleted", chart.Namespace, chart.Name)
 	var lastResourceVersion string
 	as.Require().NoError(wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(ctx context.Context) (bool, error) {
-		var found v1beta1.Chart
+		var found helmv1beta1.Chart
 		err := chartClient.Get(ctx, client.ObjectKey{Namespace: chart.Namespace, Name: chart.Name}, &found)
 		switch {
 		case err == nil:
@@ -154,21 +200,21 @@ func (as *AddonsSuite) deleteRelease(chart *v1beta1.Chart) {
 }
 
 func (as *AddonsSuite) deleteUninstalledChart(ctx context.Context) {
-	spec := v1beta1.ChartSpec{
+	spec := helmv1beta1.ChartSpec{
 		ChartName:   "whatever",
 		ReleaseName: "nonexistent",
 		Namespace:   "default",
 		Version:     "1",
 	}
-	status := v1beta1.ChartStatus{
+	status := helmv1beta1.ChartStatus{
 		ReleaseName: spec.ReleaseName,
 		Namespace:   spec.Namespace,
 		Version:     spec.Version,
 		AppVersion:  "1",
 		Revision:    1,
 		ValuesHash:  spec.HashValues(),
 	}
-	chart := &v1beta1.Chart{
+	chart := &helmv1beta1.Chart{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:       "bogus",
 			Namespace:  "kube-system",
@@ -179,7 +225,7 @@ func (as *AddonsSuite) deleteUninstalledChart(ctx context.Context) {
 	cfg, err := as.GetKubeConfig(as.ControllerNode(0))
 	as.Require().NoError(err)
 
-	scheme, err := v1beta1.SchemeBuilder.Build()
+	scheme, err := helmv1beta1.SchemeBuilder.Build()
 	as.Require().NoError(err)
 	crClient, err := client.New(cfg, client.Options{Scheme: scheme})
 	as.Require().NoError(err)
@@ -215,7 +261,7 @@ func (as *AddonsSuite) deleteUninstalledChart(ctx context.Context) {
 	as.T().Logf("Expecting bogus chart %s/%s to be deleted", chart.Namespace, chart.Name)
 	var lastResourceVersion string
 	as.Require().NoError(wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(ctx context.Context) (bool, error) {
-		var found v1beta1.Chart
+		var found helmv1beta1.Chart
 		err := crClient.Get(ctx, client.ObjectKey{Namespace: chart.Namespace, Name: chart.Name}, &found)
 		switch {
 		case err == nil:
@@ -236,18 +282,18 @@ func (as *AddonsSuite) deleteUninstalledChart(ctx context.Context) {
 	}))
 }
 
-func (as *AddonsSuite) waitForTestRelease(addonName, appVersion string, namespace string, rev int64) *v1beta1.Chart {
+func (as *AddonsSuite) waitForTestRelease(addonName, appVersion string, namespace string, rev int64) *helmv1beta1.Chart {
 	ctx := as.Context()
 	as.T().Logf("waiting to see %s release ready in kube API, generation %d", addonName, rev)
 
 	cfg, err := as.GetKubeConfig(as.ControllerNode(0))
 	as.Require().NoError(err)
 
-	helmScheme, err := v1beta1.SchemeBuilder.Build()
+	helmScheme, err := helmv1beta1.SchemeBuilder.Build()
 	as.Require().NoError(err)
 	chartClient, err := client.New(cfg, client.Options{Scheme: helmScheme})
 	as.Require().NoError(err)
-	var chart v1beta1.Chart
+	var chart helmv1beta1.Chart
 	var lastResourceVersion string
 	as.Require().NoError(wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(pollCtx context.Context) (done bool, err error) {
 		err = chartClient.Get(pollCtx, client.ObjectKey{

pkg/component/controller/extensions_controller.go

@@ -17,11 +17,14 @@ limitations under the License.
 package controller
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"regexp"
 	"slices"
 	"time"
 
@@ -60,24 +63,24 @@ import (
 
 // Helm watch for Chart crd
 type ExtensionsController struct {
-	saver         manifestsSaver
 	L             *logrus.Entry
 	helm          *helm.Commands
 	kubeConfig    string
 	leaderElector leaderelector.Interface
+	manifestsDir  string
 }
 
 var _ manager.Component = (*ExtensionsController)(nil)
 var _ manager.Reconciler = (*ExtensionsController)(nil)
 
 // NewExtensionsController builds new HelmAddons
-func NewExtensionsController(s manifestsSaver, k0sVars *config.CfgVars, kubeClientFactory kubeutil.ClientFactoryInterface, leaderElector leaderelector.Interface) *ExtensionsController {
+func NewExtensionsController(k0sVars *config.CfgVars, kubeClientFactory kubeutil.ClientFactoryInterface, leaderElector leaderelector.Interface) *ExtensionsController {
 	return &ExtensionsController{
-		saver:         s,
 		L:             logrus.WithFields(logrus.Fields{"component": "extensions_controller"}),
 		helm:          helm.NewCommands(k0sVars),
 		kubeConfig:    k0sVars.AdminKubeConfigPath,
 		leaderElector: leaderElector,
+		manifestsDir:  filepath.Join(k0sVars.ManifestsDir, "helm"),
 	}
 }
 
@@ -179,8 +182,13 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
 		}
 	}
 
+	var fileNamesToKeep []string
 	for _, chart := range helmSpec.Charts {
+		fileName := chartManifestFileName(&chart)
+		fileNamesToKeep = append(fileNamesToKeep, fileName)
+
 		tw := templatewriter.TemplateWriter{
+			Path:     filepath.Join(ec.manifestsDir, fileName),
 			Name:     "addon_crd_manifest",
 			Template: chartCrdTemplate,
 			Data: struct {
@@ -191,15 +199,35 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
 				Finalizer: finalizerName,
 			},
 		}
-		buf := bytes.NewBuffer([]byte{})
-		if err := tw.WriteToBuffer(buf); err != nil {
-			errs = append(errs, fmt.Errorf("can't create chart CR instance %q: %w", chart.ChartName, err))
+		if err := tw.Write(); err != nil {
+			errs = append(errs, fmt.Errorf("can't write file for Helm chart manifest %q: %w", chart.ChartName, err))
 			continue
 		}
-		if err := ec.saver.Save(chartManifestFileName(&chart), buf.Bytes()); err != nil {
-			errs = append(errs, fmt.Errorf("can't save addon CRD manifest for chart CR instance %q: %w", chart.ChartName, err))
-			continue
+
+		ec.L.Infof("Wrote Helm chart manifest file %q", tw.Path)
+	}
+
+	if err := filepath.WalkDir(ec.manifestsDir, func(path string, entry fs.DirEntry, err error) error {
+		switch {
+		case !entry.Type().IsRegular():
+			ec.L.Debugf("Keeping %v as it is not a regular file", entry)
+		case slices.Contains(fileNamesToKeep, entry.Name()):
+			ec.L.Debugf("Keeping %v as it belongs to a known Helm extension", entry)
+		case !isChartManifestFileName(entry.Name()):
+			ec.L.Debugf("Keeping %v as it is not a Helm chart manifest file", entry)
+		default:
+			if err := os.Remove(path); err != nil {
+				if !errors.Is(err, os.ErrNotExist) {
+					errs = append(errs, fmt.Errorf("failed to remove Helm chart manifest file, the Chart resource will remain in the cluster: %w", err))
+				}
+			} else {
+				ec.L.Infof("Removed Helm chart manifest file %q", path)
+			}
 		}
+
+		return nil
+	}); err != nil {
+		errs = append(errs, fmt.Errorf("failed to walk Helm chart manifest directory: %w", err))
 	}
 
 	return errors.Join(errs...)
@@ -210,6 +238,11 @@ func chartManifestFileName(c *k0sv1beta1.Chart) string {
 	return fmt.Sprintf("%d_helm_extension_%s.yaml", c.Order, c.Name)
 }
 
+// Determines if the given file name is in the format for chart manifest file names.
+func isChartManifestFileName(fileName string) bool {
+	return regexp.MustCompile(`^-?[0-9]+_helm_extension_.+\.yaml$`).MatchString(fileName)
+}
+
 type ChartReconciler struct {
 	client.Client
 	helm          *helm.Commands

pkg/component/controller/extensions_controller_test.go

@@ -223,4 +223,5 @@ func TestChartManifestFileName(t *testing.T) {
 	assert.Equal(t, chartManifestFileName(&chart), "0_helm_extension_release.yaml")
 	assert.Equal(t, chartManifestFileName(&chart1), "1_helm_extension_release.yaml")
 	assert.Equal(t, chartManifestFileName(&chart2), "2_helm_extension_release.yaml")
+	assert.True(t, isChartManifestFileName("0_helm_extension_release.yaml"))
 }