refactor: better order deployment of init manifests

pkg/controllers/deploy/deploy.go

@@ -62,13 +62,13 @@ type Deployer struct {
 	HelmClient     helm.Client
 }
 
-func (r *Deployer) Apply(ctx context.Context, vConfig *config.VirtualClusterConfig) (result ctrl.Result, err error) {
+func (r *Deployer) apply(ctx context.Context, vConfig *config.VirtualClusterConfig, fn func(context.Context, *config.VirtualClusterConfig, *corev1.ConfigMap) error) (err error) {
 	// get config map
 	configMap := &corev1.ConfigMap{}
 	err = r.VirtualManager.GetClient().Get(ctx, types.NamespacedName{Name: VClusterDeployConfigMap, Namespace: VClusterDeployConfigMapNamespace}, configMap)
 	if kerrors.IsNotFound(err) {
 		if vConfig.Experimental.Deploy.VCluster.Manifests == "" && vConfig.Experimental.Deploy.VCluster.ManifestsTemplate == "" && len(vConfig.Experimental.Deploy.VCluster.Helm) == 0 {
-			return ctrl.Result{}, nil
+			return nil
 		}
 
 		configMap = &corev1.ConfigMap{
@@ -79,42 +79,39 @@ func (r *Deployer) Apply(ctx context.Context, vConfig *config.VirtualClusterConf
 		}
 		err = r.VirtualManager.GetClient().Create(ctx, configMap)
 		if err != nil {
-			return ctrl.Result{}, fmt.Errorf("create deploy status config map: %w", err)
+			return fmt.Errorf("create deploy status config map: %w", err)
 		}
 	} else if err != nil {
-		return ctrl.Result{}, err
+		return err
 	}
 
 	// patch the status to the configmap
 	oldConfigMap := configMap.DeepCopy()
 	defer func() {
-		patchErr := r.UpdateConfigMap(ctx, err, result.Requeue, oldConfigMap, configMap)
+		patchErr := r.UpdateConfigMap(ctx, err, vConfig, oldConfigMap, configMap)
 		if patchErr != nil && err == nil {
 			err = patchErr
 		}
 	}()
 
 	// process the init manifests
-	requeue, err := r.ProcessInitManifests(ctx, vConfig, configMap)
+	err = fn(ctx, vConfig, configMap)
 	if err != nil {
-		return ctrl.Result{}, err
-	} else if requeue {
-		return ctrl.Result{Requeue: true}, nil
+		return err
 	}
 
-	// process the helm charts
-	requeue, err = r.ProcessHelmChart(ctx, vConfig, configMap)
-	if err != nil {
-		return ctrl.Result{}, err
-	} else if requeue {
-		return ctrl.Result{Requeue: true}, nil
-	}
+	return nil
+}
+
+func (r *Deployer) DeployInitManifests(ctx context.Context, vConfig *config.VirtualClusterConfig) error {
+	return r.apply(ctx, vConfig, r.ProcessInitManifests)
+}
 
-	// indicates that we have applied all manifests and charts
-	return ctrl.Result{}, nil
+func (r *Deployer) DeployHelmCharts(ctx context.Context, vConfig *config.VirtualClusterConfig) error {
+	return r.apply(ctx, vConfig, r.ProcessHelmChart)
 }
 
-func (r *Deployer) UpdateConfigMap(ctx context.Context, lastError error, requeue bool, oldConfigMap *corev1.ConfigMap, newConfigMap *corev1.ConfigMap) error {
+func (r *Deployer) UpdateConfigMap(ctx context.Context, lastError error, vConfig *config.VirtualClusterConfig, oldConfigMap *corev1.ConfigMap, newConfigMap *corev1.ConfigMap) error {
 	currentStatus := ParseStatus(newConfigMap)
 
 	// set phase initially to pending
@@ -142,7 +139,7 @@ func (r *Deployer) UpdateConfigMap(ctx context.Context, lastError error, requeue
 	// check if there was an error otherwise set to success
 	if currentStatus.Phase == string(StatusPending) {
 		if lastError == nil {
-			if requeue {
+			if len(currentStatus.Charts) != len(vConfig.Experimental.Deploy.VCluster.Helm) {
 				currentStatus.Phase = string(StatusPending)
 			} else {
 				currentStatus.Phase = string(StatusSuccess)
@@ -170,7 +167,7 @@ func (r *Deployer) UpdateConfigMap(ctx context.Context, lastError error, requeue
 	}
 
 	// try patching the configmap
-	r.Log.Debugf("Patch init config map with: %s", string(rawPatch))
+	r.Log.Debugf("Patch deploy config map with: %s", string(rawPatch))
 	err = r.VirtualManager.GetClient().Patch(ctx, newConfigMap, client.RawPatch(patch.Type(), rawPatch))
 	if err != nil {
 		r.Log.Errorf("error updating configmap status: %v", err)
@@ -180,13 +177,13 @@ func (r *Deployer) UpdateConfigMap(ctx context.Context, lastError error, requeue
 	return nil
 }
 
-func (r *Deployer) ProcessInitManifests(ctx context.Context, vConfig *config.VirtualClusterConfig, configMap *corev1.ConfigMap) (bool, error) {
+func (r *Deployer) ProcessInitManifests(ctx context.Context, vConfig *config.VirtualClusterConfig, configMap *corev1.ConfigMap) error {
 	var err error
 	manifests := vConfig.Experimental.Deploy.VCluster.Manifests
 	if vConfig.Experimental.Deploy.VCluster.ManifestsTemplate != "" {
 		templatedManifests, err := k0s.ExecTemplate(vConfig.Experimental.Deploy.VCluster.ManifestsTemplate, vConfig.Name, vConfig.WorkloadTargetNamespace, &vConfig.Config)
 		if err != nil {
-			return false, fmt.Errorf("exec manifests template: %w", err)
+			return fmt.Errorf("exec manifests template: %w", err)
 		}
 
 		manifests += "\n---\n" + string(templatedManifests)
@@ -207,37 +204,38 @@ func (r *Deployer) ProcessInitManifests(ctx context.Context, vConfig *config.Vir
 
 	// should skip?
 	if manifests == lastAppliedManifests {
-		return false, r.setManifestsStatus(configMap, StatusSuccess, "", "")
+		return r.setManifestsStatus(configMap, StatusSuccess, "", "")
 	}
 
 	// apply manifests
 	err = ApplyGivenInitManifests(ctx, r.VirtualManager.GetClient(), r.VirtualManager.GetConfig(), manifests, lastAppliedManifests)
 	if err != nil {
 		r.Log.Errorf("error applying init manifests: %v", err)
 		_ = r.setManifestsStatus(configMap, StatusFailed, InstallError, err.Error())
-		return false, err
+		return err
 	}
 
 	// apply successful, store in an annotation in the configmap itself
 	compressedManifests, err := compress.Compress(manifests)
 	if err != nil {
 		r.Log.Errorf("error compressing manifests: %v", err)
-		return false, err
+		return err
 	}
 
 	// update annotation
 	status.Manifests.LastAppliedManifests = compressedManifests
 	err = r.encodeStatus(configMap, status)
 	if err != nil {
-		return false, err
+		return err
 	}
-	return true, r.setManifestsStatus(configMap, StatusSuccess, "", "")
+
+	return r.setManifestsStatus(configMap, StatusSuccess, "", "")
 }
 
-func (r *Deployer) ProcessHelmChart(ctx context.Context, vConfig *config.VirtualClusterConfig, configMap *corev1.ConfigMap) (bool, error) {
+func (r *Deployer) ProcessHelmChart(ctx context.Context, vConfig *config.VirtualClusterConfig, configMap *corev1.ConfigMap) error {
 	statusMap, err := r.getStatusMap(configMap)
 	if err != nil {
-		return false, err
+		return err
 	}
 
 	charts := vConfig.Experimental.Deploy.VCluster.Helm
@@ -249,36 +247,34 @@ func (r *Deployer) ProcessHelmChart(ctx context.Context, vConfig *config.Virtual
 		err := r.pullChartArchive(ctx, chart)
 		if err != nil {
 			_ = r.setChartStatus(configMap, &chart, StatusFailed, ChartPullError, err.Error())
-			return false, err
+			return err
 		}
 
 		// check if we should upgrade the helm release
 		exists, err := r.releaseExists(chart)
 		if err != nil {
-			return false, err
+			return err
 		} else if exists {
 			r.Log.Debugf("release %s/%s already exists", releaseNamespace, releaseName)
 
 			// check if upgrade is needed
 			upgradedNeeded, err := r.checkIfUpgradeNeeded(configMap, chart)
 			if err != nil {
-				return false, err
+				return err
 			} else if upgradedNeeded {
 				// initiate upgrade
 				err = r.initiateUpgrade(ctx, chart)
 				if err != nil {
 					_ = r.setChartStatus(configMap, &chart, StatusFailed, UpgradeError, err.Error())
-					return false, err
+					return err
 				}
 
 				// update last applied chart config
 				err = r.setChartStatusLastApplied(configMap, &chart)
 				if err != nil {
 					r.Log.Errorf("error updating config map with last applied chart annotation: %v", err)
-					return false, err
+					return err
 				}
-
-				return true, nil
 			}
 
 			// continue to process next chart
@@ -291,32 +287,28 @@ func (r *Deployer) ProcessHelmChart(ctx context.Context, vConfig *config.Virtual
 		if err != nil {
 			r.Log.Errorf("error installing release %s/%s", releaseNamespace, releaseName)
 			_ = r.setChartStatus(configMap, &chart, StatusFailed, InstallError, err.Error())
-			return false, err
+			return err
 		}
 
 		// update last applied chart config
 		err = r.setChartStatusLastApplied(configMap, &chart)
 		if err != nil {
 			r.Log.Errorf("error updating config map with last applied chart annotation: %v", err)
-			return false, err
+			return err
 		}
-
-		// install only one chart successfully in each reconcile
-		// hence reconcile here without error
-		return true, nil
 	}
 
 	if len(statusMap) > 0 {
 		r.Log.Debugf("following charts left in status map, should be deleted: %v", statusMap)
 		for _, chartStatus := range statusMap {
 			err := r.deleteHelmRelease(configMap, chartStatus)
 			if err != nil {
-				return false, errors.Wrap(err, "delete helm release")
+				return errors.Wrap(err, "delete helm release")
 			}
 		}
 	}
 
-	return false, nil
+	return nil
 }
 
 func (r *Deployer) checkIfUpgradeNeeded(cm *corev1.ConfigMap, chart vclusterconfig.ExperimentalDeployHelm) (bool, error) {

pkg/controllers/deploy/start.go

@@ -1,6 +1,7 @@
 package deploy
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/loft-sh/log"
@@ -28,22 +29,32 @@ func RegisterInitManifestsController(controllerCtx *synccontext.ControllerContex
 		return err
 	}
 
-	controller := &Deployer{
+	deployer := &Deployer{
 		Log:            loghelper.New("init-manifests-controller"),
 		VirtualManager: controllerCtx.VirtualManager,
 
 		HelmClient: helm.NewClient(&vConfigRaw, log.GetInstance(), helmBinaryPath),
 	}
 
+	// deploy manifests
+	err = deployer.DeployInitManifests(controllerCtx, controllerCtx.Config)
+	if err != nil {
+		return fmt.Errorf("error deploying experimental.deploy.vCluster.manifests: %w", err)
+	}
+
+	// deploy helm charts
 	go func() {
 		for {
-			result, err := controller.Apply(controllerCtx, controllerCtx.Config)
+			// deploy helm charts
+			err := deployer.DeployHelmCharts(controllerCtx, controllerCtx.Config)
 			if err != nil {
-				klog.Errorf("Error deploying manifests: %v", err)
+				klog.Errorf("Error deploying experimental.deploy.vCluster.helm: %v", err)
 				time.Sleep(time.Second * 10)
-			} else if !result.Requeue {
-				break
+				continue
 			}
+
+			// exit loop
+			break
 		}
 	}()
 

pkg/controllers/register.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	vclusterconfig "github.com/loft-sh/vcluster/config"
-	"github.com/loft-sh/vcluster/pkg/controllers/deploy"
 	"github.com/loft-sh/vcluster/pkg/controllers/generic"
 	"github.com/loft-sh/vcluster/pkg/controllers/servicesync"
 	"github.com/loft-sh/vcluster/pkg/syncer"
@@ -49,12 +48,6 @@ func RegisterControllers(ctx *synccontext.ControllerContext, syncers []syncertyp
 		return err
 	}
 
-	// register init manifests configmap watcher controller
-	err = deploy.RegisterInitManifestsController(ctx)
-	if err != nil {
-		return err
-	}
-
 	// register service syncer to map services between host and virtual cluster
 	err = registerServiceSyncControllers(ctx)
 	if err != nil {

pkg/setup/controllers.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/loft-sh/vcluster/pkg/config"
 	"github.com/loft-sh/vcluster/pkg/controllers"
+	"github.com/loft-sh/vcluster/pkg/controllers/deploy"
 	"github.com/loft-sh/vcluster/pkg/controllers/resources/services"
 	"github.com/loft-sh/vcluster/pkg/coredns"
 	"github.com/loft-sh/vcluster/pkg/log"
@@ -41,6 +42,12 @@ func StartControllers(controllerContext *synccontext.ControllerContext, syncers
 		return err
 	}
 
+	// register init manifests configmap watcher controller
+	err = deploy.RegisterInitManifestsController(controllerContext)
+	if err != nil {
+		return err
+	}
+
 	// start coredns & create syncers
 	if !controllerContext.Config.Experimental.SyncSettings.DisableSync {
 		// setup CoreDNS according to the manifest file