fix(pro): always check for available template updates and wait for

changed parameters to be applied

Changes the template behaviour for pro instances like so:
1. If template is not versioned _always_ keep the workspace in sync
   with template, even if that means rescheduling due to workspace
   changes
2. If the template is versioned and the workspace uses an explicit
   version we keep the workspace up to date with the template unless a
   new version is available. We expect versioned templates to introduce
   changes through a new version, not by updating an existing version

In addition to that we now wait until the server applied parameter
changes instead of assuming it did in time. This fixes a race condition
where the parameter changes wouldn't be applied if the controller took
more than a couple ms

Author: pascalbreuninger

cmd/pro/provider/up.go

@@ -2,19 +2,23 @@ package provider
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 
 	"github.com/loft-sh/devpod/cmd/pro/flags"
+	"github.com/loft-sh/devpod/pkg/client/clientimplementation"
 	"github.com/loft-sh/devpod/pkg/platform"
 	"github.com/loft-sh/devpod/pkg/platform/client"
 	"github.com/loft-sh/devpod/pkg/platform/remotecommand"
 	"github.com/loft-sh/log"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
 	managementv1 "github.com/loft-sh/api/v4/pkg/apis/management/v1"
 	storagev1 "github.com/loft-sh/api/v4/pkg/apis/storage/v1"
+	corev1 "k8s.io/api/core/v1"
 )
 
 // UpCmd holds the cmd flags:
@@ -33,9 +37,15 @@ type streams struct {
 
 // NewUpCmd creates a new command
 func NewUpCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
+	logLevel := logrus.InfoLevel
+	if os.Getenv(clientimplementation.DevPodDebug) == "true" || globalFlags.Debug {
+		logLevel = logrus.DebugLevel
+	}
+
 	cmd := &UpCmd{
 		GlobalFlags: globalFlags,
-		Log:         log.GetInstance(),
+		Log: log.NewStreamLoggerWithFormat( /* we don't use stdout */ nil,
+			os.Stderr, logLevel, log.JSONFormat).ErrorStreamOnly(),
 		streams: streams{
 			Stdin:  os.Stdin,
 			Stdout: os.Stdout,
@@ -69,6 +79,24 @@ func (cmd *UpCmd) Run(ctx context.Context) error {
 	instance, err := platform.FindInstanceInProject(ctx, baseClient, info.UID, info.ProjectName)
 	if err != nil {
 		return err
+	} else if instance == nil {
+		return fmt.Errorf("workspace %s not found in project %s. Looks like it does not exist anymore and you can delete it", info.ID, info.ProjectName)
+	}
+
+	// Log current workspace information. This is both useful to the user to understand the workspace configuration
+	// and to us when we receive troubleshooting logs
+	printInstanceInfo(instance, cmd.Log)
+
+	if instance.Spec.TemplateRef != nil && templateUpdateRequired(instance) {
+		cmd.Log.Info("Template update required")
+		oldInstance := instance.DeepCopy()
+		instance.Spec.TemplateRef.SyncOnce = true
+
+		instance, err = platform.UpdateInstance(ctx, baseClient, oldInstance, instance, cmd.Log)
+		if err != nil {
+			return fmt.Errorf("update instance: %w", err)
+		}
+		cmd.Log.Info("Successfully updated template")
 	}
 
 	return cmd.up(ctx, instance, baseClient)
@@ -85,10 +113,41 @@ func (cmd *UpCmd) up(ctx context.Context, workspace *managementv1.DevPodWorkspac
 		return err
 	}
 
-	_, err = remotecommand.ExecuteConn(ctx, conn, cmd.streams.Stdin, cmd.streams.Stdout, cmd.streams.Stderr, cmd.Log.ErrorStreamOnly())
+	_, err = remotecommand.ExecuteConn(ctx, conn, cmd.streams.Stdin, cmd.streams.Stdout, cmd.streams.Stderr, cmd.Log)
 	if err != nil {
 		return fmt.Errorf("error executing: %w", err)
 	}
 
 	return nil
 }
+
+func templateUpdateRequired(instance *managementv1.DevPodWorkspaceInstance) bool {
+	var templateResolved, templateChangesAvailable bool
+	for _, condition := range instance.Status.Conditions {
+		if condition.Type == storagev1.InstanceTemplateResolved {
+			templateResolved = condition.Status == corev1.ConditionTrue
+			continue
+		}
+
+		if condition.Type == storagev1.InstanceTemplateSynced {
+			templateChangesAvailable = condition.Status == corev1.ConditionFalse &&
+				condition.Reason == "TemplateChangesAvailable"
+			continue
+		}
+	}
+
+	return !templateResolved || templateChangesAvailable
+}
+
+func printInstanceInfo(instance *managementv1.DevPodWorkspaceInstance, log log.Logger) {
+	workspaceConfig, _ := json.Marshal(struct {
+		Runner     storagev1.RunnerRef
+		Template   *storagev1.TemplateRef
+		Parameters string
+	}{
+		Runner:     instance.Spec.RunnerRef,
+		Template:   instance.Spec.TemplateRef,
+		Parameters: instance.Spec.Parameters,
+	})
+	log.Info("Starting pro workspace with configuration", string(workspaceConfig))
+}

cmd/pro/provider/update/workspace.go

@@ -17,7 +17,6 @@ import (
 	"github.com/loft-sh/log/terminal"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // WorkspaceCmd holds the cmd flags
@@ -112,25 +111,10 @@ func (cmd *WorkspaceCmd) Run(ctx context.Context, stdin io.Reader, stdout io.Wri
 }
 
 func updateInstance(ctx context.Context, client client.Client, oldInstance *managementv1.DevPodWorkspaceInstance, newInstance *managementv1.DevPodWorkspaceInstance, log log.Logger) (*managementv1.DevPodWorkspaceInstance, error) {
-	managementClient, err := client.Management()
-	if err != nil {
-		return nil, err
-	}
-
-	patch := ctrlclient.MergeFrom(oldInstance)
-	data, err := patch.Data(newInstance)
-	if err != nil {
-		return nil, err
-	} else if len(data) == 0 || string(data) == "{}" {
-		return newInstance, nil
-	}
-
-	res, err := managementClient.Loft().ManagementV1().
-		DevPodWorkspaceInstances(oldInstance.GetNamespace()).
-		Patch(ctx, oldInstance.GetName(), patch.Type(), data, metav1.PatchOptions{})
-	if err != nil {
-		return nil, err
+	// This ensures the template is kept up to date with configuration changes
+	if newInstance.Spec.TemplateRef != nil {
+		newInstance.Spec.TemplateRef.SyncOnce = true
 	}
 
-	return platform.WaitForInstance(ctx, client, res, log)
+	return platform.UpdateInstance(ctx, client, oldInstance, newInstance, log)
 }

pkg/ide/vscode/open.go

@@ -109,7 +109,7 @@ func openViaCLI(ctx context.Context, workspace, folder string, newWindow bool, f
 	// Needs to be separated by `=` because of windows
 	folderUriArg := fmt.Sprintf("--folder-uri=vscode-remote://ssh-remote+%s.devpod/%s", workspace, folder)
 	args = append(args, folderUriArg)
-	log.Debugf("Run %s command %s %s", flavor, codePath, strings.Join(args, " "))
+	log.Debugf("Run %s command %s %s", flavor.DisplayName(), codePath, strings.Join(args, " "))
 	out, err = exec.CommandContext(ctx, codePath, args...).CombinedOutput()
 	if err != nil {
 		return command.WrapCommandError(out, err)

pkg/platform/instance.go

@@ -17,8 +17,10 @@ import (
 	"github.com/loft-sh/devpod/pkg/platform/client"
 	"github.com/loft-sh/devpod/pkg/platform/project"
 	"github.com/loft-sh/log"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type WorkspaceInfo struct {
@@ -159,6 +161,32 @@ func DialInstance(baseClient client.Client, workspace *managementv1.DevPodWorksp
 	return conn, nil
 }
 
+// UpdateInstance diffs two versions of a DevPodWorkspaceInstance, applies changes via a patch to reduce conflicts.
+// Afterwards it waits until the instance is ready to be used.
+func UpdateInstance(ctx context.Context, client client.Client, oldInstance *managementv1.DevPodWorkspaceInstance, newInstance *managementv1.DevPodWorkspaceInstance, log log.Logger) (*managementv1.DevPodWorkspaceInstance, error) {
+	managementClient, err := client.Management()
+	if err != nil {
+		return nil, err
+	}
+
+	patch := ctrlclient.MergeFrom(oldInstance)
+	data, err := patch.Data(newInstance)
+	if err != nil {
+		return nil, err
+	} else if len(data) == 0 || string(data) == "{}" {
+		return newInstance, nil
+	}
+
+	res, err := managementClient.Loft().ManagementV1().
+		DevPodWorkspaceInstances(oldInstance.GetNamespace()).
+		Patch(ctx, oldInstance.GetName(), patch.Type(), data, metav1.PatchOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	return WaitForInstance(ctx, client, res, log)
+}
+
 func WaitForInstance(ctx context.Context, client client.Client, instance *managementv1.DevPodWorkspaceInstance, log log.Logger) (*managementv1.DevPodWorkspaceInstance, error) {
 	managementClient, err := client.Management()
 	if err != nil {
@@ -182,8 +210,18 @@ func WaitForInstance(ctx context.Context, client client.Client, instance *manage
 			return false, nil
 		}
 
+		if !isTemplateSynced(updatedInstance) {
+			log.Debugf("Workspace template is not ready yet")
+			for _, cond := range updatedInstance.Status.Conditions {
+				if cond.Status != corev1.ConditionTrue {
+					log.Debugf("%s is %s (%s): %s", cond.Type, cond.Status, cond.Reason, cond.Message)
+				}
+			}
+			return false, nil
+		}
+
 		if !isRunnerReady(updatedInstance, storagev1.BuiltinRunnerName) {
-			log.Debugf("Runner is not ready yet, waiting until its ready", name, status.Phase)
+			log.Debugf("Runner is not ready yet", name, status.Phase)
 			return false, nil
 		}
 
@@ -218,3 +256,19 @@ func isRunnerReady(workspace *managementv1.DevPodWorkspaceInstance, builtinRunne
 	return workspace.GetAnnotations() != nil &&
 		workspace.GetAnnotations()[storagev1.DevPodWorkspaceRunnerEndpointAnnotation] != ""
 }
+
+func isTemplateSynced(workspace *managementv1.DevPodWorkspaceInstance) bool {
+	// We're still waiting for the sync to happen
+	// The controller will remove this field once it's done syncing
+	if workspace.Spec.TemplateRef != nil && workspace.Spec.TemplateRef.SyncOnce {
+		return false
+	}
+
+	for _, condition := range workspace.Status.Conditions {
+		if condition.Type == storagev1.InstanceTemplateResolved {
+			return condition.Status == corev1.ConditionTrue
+		}
+	}
+
+	return false
+}