Add upgrade path for new/not-installed charts

Signed-off-by: Daniel Weiße <dw@edgeless.systems>

cli/internal/helm/README.md

@@ -1,4 +1,30 @@
-# Chart upgrades
+# Helm
+
+Constellation uses [helm](https://helm.sh/) to install and upgrade deployments to the Kubernetes cluster.
+Helm wraps deployments into charts. One chart should contain all the configuration needed to run a deployment.
+
+## Charts used by Constellation
+
+To make installation and lifecycle management easier, Constellation groups multiple related charts into sub-charts.
+The following "parent" charts are used by Constellation:
+
+* [cert-manager](./charts/cert-manager/)
+
+* [Cilium](./charts/cilium/)
+
+* [constellation-services](./charts/edgeless/constellation-services/)
+
+    Cluster services (mostly) written by us, providing basic functionality of the cluster
+
+* [csi](./charts/edgeless/csi/)
+
+    Our modified Kubernetes CSI drivers and Snapshot controller/CRDs
+
+* [operators](./charts/edgeless/operators/)
+
+    Kubernetes operators we use to control and manage the lifecycle of a Constellation cluster
+
+## Chart upgrades
 
 All services that are installed via helm-install are upgraded via helm-upgrade.
 Two aspects are not full covered by running helm-upgrade: CRDs and values.
@@ -9,19 +35,24 @@ Because upgrades should be a CLI-only operation and we want to avoid the behavio
 
 Here is how we manage CRD upgrades for each chart.
 
-## Cilium
+### Cilium
 
-- CRDs are updated by cilium-operator.
+* CRDs are updated by cilium-operator.
 
-## cert-manager
+### cert-manager
 
-- installCRDs flag is set during upgrade. This flag is managed by cert-manager. cert-manager is in charge of correctly upgrading the CRDs.
+* installCRDs flag is set during upgrade. This flag is managed by cert-manager. cert-manager is in charge of correctly upgrading the CRDs.
-- WARNING: upgrading cert-manager might break other installations of cert-manager in the cluster, if those other installation are not on the same version as the Constellation-manager installation. This is due to the cluster-wide CRDs.
+* WARNING: upgrading cert-manager might break other installations of cert-manager in the cluster, if those other installation are not on the same version as the Constellation-manager installation. This is due to the cluster-wide CRDs.
 
-## Operators
+### Operators
 
-- Manually update CRDs before upgrading the chart. Update by running applying the CRDs found in the `operators/crds/` folder.
+* Manually update CRDs before upgrading the chart. Update by running applying the CRDs found in the `operators/crds/` folder.
 
-## Constellation-services
+### Constellation-services
 
-- There currently are no CRDs in this chart.
+* There currently are no CRDs in this chart.
+
+### CSI
+
+* CRDs are required for enabling snapshot support
+* CRDs are provided as their own helm chart and may be updated using helm

cli/internal/helm/client.go

@@ -40,6 +40,8 @@ const (
 // ErrConfirmationMissing signals that an action requires user confirmation.
 var ErrConfirmationMissing = errors.New("action requires user confirmation")
 
+var errReleaseNotFound = errors.New("release not found")
+
 // Client handles interaction with helm and the cluster.
 type Client struct {
 	config  *action.Configuration
@@ -105,8 +107,10 @@ func (c *Client) shouldUpgrade(releaseName, newVersion string, force bool) error
 func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive, force bool, upgradeID string) error {
 	upgradeErrs := []error{}
 	upgradeReleases := []*chart.Chart{}
+	newReleases := []*chart.Chart{}
 
-	for _, info := range []chartInfo{ciliumInfo, certManagerInfo, constellationOperatorsInfo, constellationServicesInfo} {
+	for _, info := range []chartInfo{ciliumInfo, certManagerInfo, constellationOperatorsInfo, constellationServicesInfo, csiInfo} {
+		c.log.Debugf("Checking release %s", info.releaseName)
 		chart, err := loadChartsDir(helmFS, info.path)
 		if err != nil {
 			return fmt.Errorf("loading chart: %w", err)
@@ -125,9 +129,14 @@ func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout tim
 		var invalidUpgrade *compatibility.InvalidUpgradeError
 		err = c.shouldUpgrade(info.releaseName, upgradeVersion, force)
 		switch {
+		case errors.Is(err, errReleaseNotFound):
+			// if the release is not found, we need to install it
+			c.log.Debugf("Release %s not found, adding to new releases...", info.releaseName)
+			newReleases = append(newReleases, chart)
 		case errors.As(err, &invalidUpgrade):
 			upgradeErrs = append(upgradeErrs, fmt.Errorf("skipping %s upgrade: %w", info.releaseName, err))
 		case err != nil:
+			c.log.Debugf("Adding %s to upgrade releases...", info.releaseName)
 			return fmt.Errorf("should upgrade %s: %w", info.releaseName, err)
 		case err == nil:
 			upgradeReleases = append(upgradeReleases, chart)
@@ -142,10 +151,9 @@ func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout tim
 		}
 	}
 
-	if len(upgradeReleases) == 0 {
+	// Backup CRDs and CRs if we are upgrading anything.
-		return errors.Join(upgradeErrs...)
+	if len(upgradeReleases) != 0 {
-	}
+		c.log.Debugf("Creating backup of CRDs and CRs")
-
 		crds, err := c.backupCRDs(ctx, upgradeID)
 		if err != nil {
 			return fmt.Errorf("creating CRD backup: %w", err)
@@ -153,10 +161,24 @@ func (c *Client) Upgrade(ctx context.Context, config *config.Config, timeout tim
 		if err := c.backupCRs(ctx, crds, upgradeID); err != nil {
 			return fmt.Errorf("creating CR backup: %w", err)
 		}
+	}
 
 	for _, chart := range upgradeReleases {
-		err = c.upgradeRelease(ctx, timeout, config, chart)
+		c.log.Debugf("Upgrading release %s", chart.Metadata.Name)
-		if err != nil {
+		if err := c.upgradeRelease(ctx, timeout, config, chart); err != nil {
+			return fmt.Errorf("upgrading %s: %w", chart.Metadata.Name, err)
+		}
+	}
+
+	// Install new releases after upgrading existing ones.
+	// This makes sure if a release was removed as a dependency from one chart,
+	// and then added as a new standalone chart (or as a dependency of another chart),
+	// that the new release is installed without creating naming conflicts.
+	// If in the future, we require to install a new release before upgrading existing ones,
+	// it should be done in a separate loop, instead of moving this one up.
+	for _, chart := range newReleases {
+		c.log.Debugf("Installing new release %s", chart.Metadata.Name)
+		if err := c.installNewRelease(ctx, timeout, config, chart); err != nil {
 			return fmt.Errorf("upgrading %s: %w", chart.Metadata.Name, err)
 		}
 	}
@@ -199,7 +221,7 @@ func (c *Client) currentVersion(release string) (string, error) {
 	}
 
 	if len(rel) == 0 {
-		return "", fmt.Errorf("release %s not found", release)
+		return "", errReleaseNotFound
 	}
 	if len(rel) > 1 {
 		return "", fmt.Errorf("multiple releases found for %s", release)
@@ -250,14 +272,42 @@ func (s ServiceVersions) ConstellationServices() string {
 	return s.constellationServices
 }
 
+// installNewRelease installs a previously not installed release on the cluster.
+func (c *Client) installNewRelease(
+	ctx context.Context, timeout time.Duration, conf *config.Config, chart *chart.Chart,
+) error {
+	releaseName, values, err := c.loadUpgradeValues(ctx, conf, chart)
+	if err != nil {
+		return fmt.Errorf("loading values: %w", err)
+	}
+	return c.actions.installAction(ctx, releaseName, chart, values, timeout)
+}
+
+// upgradeRelease upgrades a release running on the cluster.
 func (c *Client) upgradeRelease(
 	ctx context.Context, timeout time.Duration, conf *config.Config, chart *chart.Chart,
 ) error {
+	releaseName, values, err := c.loadUpgradeValues(ctx, conf, chart)
+	if err != nil {
+		return fmt.Errorf("loading values: %w", err)
+	}
+
+	values, err = c.mergeClusterValues(values, releaseName)
+	if err != nil {
+		return fmt.Errorf("preparing values: %w", err)
+	}
+
+	return c.actions.upgradeAction(ctx, releaseName, chart, values, timeout)
+}
+
+// loadUpgradeValues loads values for a chart required for running an upgrade.
+func (c *Client) loadUpgradeValues(ctx context.Context, conf *config.Config, chart *chart.Chart,
+) (string, map[string]any, error) {
 	// We need to load all values that can be statically loaded before merging them with the cluster
 	// values. Otherwise the templates are not rendered correctly.
 	k8sVersion, err := versions.NewValidK8sVersion(conf.KubernetesVersion, false)
 	if err != nil {
-		return fmt.Errorf("validating k8s version: %s", conf.KubernetesVersion)
+		return "", nil, fmt.Errorf("validating k8s version: %s", conf.KubernetesVersion)
 	}
 	loader := NewLoader(conf.GetProvider(), k8sVersion)
 
@@ -276,30 +326,23 @@ func (c *Client) upgradeRelease(
 		values = loader.loadOperatorsValues()
 
 		if err := c.updateCRDs(ctx, chart); err != nil {
-			return fmt.Errorf("updating CRDs: %w", err)
+			return "", nil, fmt.Errorf("updating CRDs: %w", err)
 		}
 	case constellationServicesInfo.chartName:
 		releaseName = constellationServicesInfo.releaseName
 		values = loader.loadConstellationServicesValues()
 
 		if err := c.applyMigrations(ctx, releaseName, values, conf); err != nil {
-			return fmt.Errorf("applying migrations: %w", err)
+			return "", nil, fmt.Errorf("applying migrations: %w", err)
 		}
+	case csiInfo.chartName:
+		releaseName = csiInfo.releaseName
+		values = loader.loadCSIValues()
 	default:
-		return fmt.Errorf("unknown chart name: %s", chart.Metadata.Name)
+		return "", nil, fmt.Errorf("unknown chart name: %s", chart.Metadata.Name)
 	}
 
-	values, err = c.prepareValues(values, releaseName)
+	return releaseName, values, nil
-	if err != nil {
-		return fmt.Errorf("preparing values: %w", err)
-	}
-
-	err = c.actions.upgradeAction(ctx, releaseName, chart, values, timeout)
-	if err != nil {
-		return err
-	}
-
-	return nil
 }
 
 // applyMigrations checks the from version and applies the necessary migrations.
@@ -332,12 +375,12 @@ func migrateFrom2_8(_ context.Context, _ map[string]any, _ *config.Config, _ crd
 	return nil
 }
 
-// prepareValues returns a values map as required for helm-upgrade.
+// mergeClusterValues returns a values map as required for helm-upgrade.
 // It imitates the behaviour of helm's reuse-values flag by fetching the current values from the cluster
 // and merging the fetched values with the locally found values.
 // This is done to ensure that new values (from upgrades of the local files) end up in the cluster.
 // reuse-values does not ensure this.
-func (c *Client) prepareValues(localValues map[string]any, releaseName string) (map[string]any, error) {
+func (c *Client) mergeClusterValues(localValues map[string]any, releaseName string) (map[string]any, error) {
 	// Ensure installCRDs is set for cert-manager chart.
 	if releaseName == certManagerInfo.releaseName {
 		localValues["installCRDs"] = true
@@ -395,6 +438,7 @@ type crdClient interface {
 type actionWrapper interface {
 	listAction(release string) ([]*release.Release, error)
 	getValues(release string) (map[string]any, error)
+	installAction(ctx context.Context, releaseName string, chart *chart.Chart, values map[string]any, timeout time.Duration) error
 	upgradeAction(ctx context.Context, releaseName string, chart *chart.Chart, values map[string]any, timeout time.Duration) error
 }
 
@@ -428,3 +472,15 @@ func (a actions) upgradeAction(ctx context.Context, releaseName string, chart *c
 	}
 	return nil
 }
+
+func (a actions) installAction(ctx context.Context, releaseName string, chart *chart.Chart, values map[string]any, timeout time.Duration) error {
+	action := action.NewInstall(a.config)
+	action.Atomic = true
+	action.Namespace = constants.HelmNamespace
+	action.ReleaseName = releaseName
+	action.Timeout = timeout
+	if _, err := action.RunWithContext(ctx, chart, values); err != nil {
+		return fmt.Errorf("installing previously not installed chart %s: %w", chart.Name(), err)
+	}
+	return nil
+}

cli/internal/helm/client_test.go

@@ -100,6 +100,10 @@ func (a *stubActionWrapper) getValues(_ string) (map[string]any, error) {
 	return nil, nil
 }
 
+func (a *stubActionWrapper) installAction(_ context.Context, _ string, _ *chart.Chart, _ map[string]any, _ time.Duration) error {
+	return nil
+}
+
 func (a *stubActionWrapper) upgradeAction(_ context.Context, _ string, _ *chart.Chart, _ map[string]any, _ time.Duration) error {
 	return nil
 }

cli/internal/helm/loader.go

@@ -151,7 +151,7 @@ func (i *ChartLoader) loadRelease(info chartInfo, helmWaitMode helm.WaitMode) (h
 		return helm.Release{}, fmt.Errorf("loading %s chart: %w", info.releaseName, err)
 	}
 
-	values := map[string]any{}
+	var values map[string]any
 
 	switch info.releaseName {
 	case ciliumInfo.releaseName:
@@ -166,10 +166,7 @@ func (i *ChartLoader) loadRelease(info chartInfo, helmWaitMode helm.WaitMode) (h
 		values = i.loadConstellationServicesValues()
 	case csiInfo.releaseName:
 		updateVersions(chart, compatibility.EnsurePrefixV(constants.VersionInfo()))
-	}
+		values = i.loadCSIValues()
-
-	values["tags"] = map[string]any{
-		i.csp.String(): true,
 	}
 
 	chartRaw, err := i.marshalChart(chart)
@@ -240,6 +237,7 @@ func (i *ChartLoader) loadOperatorsValues() map[string]any {
 				},
 			},
 		},
+		"tags": i.cspTags(),
 	}
 }
 
@@ -284,6 +282,19 @@ func (i *ChartLoader) loadConstellationServicesValues() map[string]any {
 		"konnectivity": map[string]any{
 			"image": i.konnectivityImage,
 		},
+		"tags": i.cspTags(),
+	}
+}
+
+func (i *ChartLoader) loadCSIValues() map[string]any {
+	return map[string]any{
+		"tags": i.cspTags(),
+	}
+}
+
+func (i *ChartLoader) cspTags() map[string]any {
+	return map[string]any{
+		i.csp.String(): true,
 	}
 }