From 867bcf80855aedae041353112abde76eee2d2078 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 21 Nov 2025 17:28:34 -0500 Subject: [PATCH 1/2] fix(virt): Repair colima virtualization mode The Colima virtualization mode was broken in the course of a recent refactor. This PR re-implements it correctly, fixing a number of issues, including: - Not resetting the session => incorrect `DOCKER_HOST` - Apply config faults in Configure in `pkg/project` - Adding `workstation.Prepare` to create sub-components after initial workstation construction Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- cmd/init.go | 4 + cmd/init_test.go | 58 ++++++ pkg/project/project.go | 24 +-- pkg/project/project_test.go | 150 +++++++++----- pkg/runtime/env/docker_env.go | 28 ++- pkg/runtime/env/docker_env_test.go | 308 ++++++++++++++++++++++++++++ pkg/runtime/runtime.go | 10 +- pkg/runtime/runtime_test.go | 181 ++++++++++++++++ pkg/workstation/workstation.go | 91 ++++---- pkg/workstation/workstation_test.go | 140 ++++++++----- 10 files changed, 824 insertions(+), 170 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index 5e7d2cfa8..7825f53d6 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -123,6 +123,10 @@ var initCmd = &cobra.Command{ return err } + if err := rt.HandleSessionReset(); err != nil { + return fmt.Errorf("failed to handle session reset: %w", err) + } + if err := proj.Initialize(initReset); err != nil { return err } diff --git a/cmd/init_test.go b/cmd/init_test.go index 6b9fa81a6..95aae42b1 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -1060,4 +1060,62 @@ func TestInitCmd(t *testing.T) { t.Errorf("Expected error message to contain 'failed to add current directory to trusted file', got: %v", err) } }) + + t.Run("CallsHandleSessionReset", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // And tracking whether Shell.CheckResetFlags is called (which HandleSessionReset calls) + var checkResetFlagsCalled bool + mocks.Shell.Shell.CheckResetFlagsFunc = func() (bool, error) { + checkResetFlagsCalled = true + return false, nil + } + + // When executing the init command + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), runtimeOverridesKey, mocks.Runtime) + cmd.SetArgs([]string{}) + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + + // And CheckResetFlags should have been called (indicating HandleSessionReset was called) + if !checkResetFlagsCalled { + t.Error("Expected CheckResetFlags to be called (via HandleSessionReset), but it was not") + } + }) + + t.Run("HandlesHandleSessionResetError", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // And CheckResetFlags returns an error (which HandleSessionReset will propagate) + expectedError := fmt.Errorf("failed to check reset flags") + mocks.Shell.Shell.CheckResetFlagsFunc = func() (bool, error) { + return false, expectedError + } + + // When executing the init command + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), runtimeOverridesKey, mocks.Runtime) + cmd.SetArgs([]string{}) + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then an error should occur + if err == nil { + t.Error("Expected error when HandleSessionReset fails, got nil") + return + } + + // And the error should contain the expected message + if !strings.Contains(err.Error(), "failed to handle session reset") { + t.Errorf("Expected error message to contain 'failed to handle session reset', got: %v", err) + } + }) } diff --git a/pkg/project/project.go b/pkg/project/project.go index a80b28e74..a34b77237 100644 --- a/pkg/project/project.go +++ b/pkg/project/project.go @@ -61,10 +61,6 @@ func NewProject(contextName string, opts ...*Project) (*Project, error) { rt.ContextName = contextName rt.ConfigRoot = filepath.Join(rt.ProjectRoot, "contexts", contextName) - if err := rt.ApplyConfigDefaults(); err != nil { - return nil, err - } - var comp *composer.Composer if overrides != nil && overrides.Composer != nil { comp = overrides.Composer @@ -80,7 +76,7 @@ func NewProject(contextName string, opts ...*Project) (*Project, error) { } var ws *workstation.Workstation - if rt.ConfigHandler.IsDevMode(rt.ContextName) { + if rt.ConfigHandler.IsDevMode(contextName) { if overrides != nil && overrides.Workstation != nil { ws = overrides.Workstation } else { @@ -119,6 +115,10 @@ func (p *Project) Configure(flagOverrides map[string]any) error { } } + if err := p.Runtime.ApplyConfigDefaults(flagOverrides); err != nil { + return fmt.Errorf("failed to apply config defaults: %w", err) + } + providerOverride := "" if flagOverrides != nil { if prov, ok := flagOverrides["provider"].(string); ok { @@ -148,14 +148,14 @@ func (p *Project) Configure(flagOverrides map[string]any) error { } // Initialize runs the complete initialization sequence for the project. -// It initializes network, prepares context, generates infrastructure, prepares tools, -// and bootstraps the environment. The overwrite parameter controls whether -// infrastructure generation should overwrite existing files. Returns an error -// if any step fails. +// It prepares the workstation (creates services and assigns IPs), prepares context, +// generates infrastructure, prepares tools, and bootstraps the environment. +// The overwrite parameter controls whether infrastructure generation should overwrite +// existing files. Returns an error if any step fails. func (p *Project) Initialize(overwrite bool) error { - if p.Workstation != nil && p.Workstation.NetworkManager != nil { - if err := p.Workstation.NetworkManager.AssignIPs(p.Workstation.Services); err != nil { - return fmt.Errorf("failed to assign IPs to network manager: %w", err) + if p.Workstation != nil { + if err := p.Workstation.Prepare(); err != nil { + return fmt.Errorf("failed to prepare workstation: %w", err) } } diff --git a/pkg/project/project_test.go b/pkg/project/project_test.go index 8a19567ee..87bd32c4d 100644 --- a/pkg/project/project_test.go +++ b/pkg/project/project_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + v1alpha1 "github.com/windsorcli/cli/api/v1alpha1" "github.com/windsorcli/cli/pkg/composer" "github.com/windsorcli/cli/pkg/composer/blueprint" "github.com/windsorcli/cli/pkg/provisioner" @@ -277,35 +278,6 @@ func TestNewProject(t *testing.T) { } }) - t.Run("ErrorOnApplyConfigDefaultsFailure", func(t *testing.T) { - mocks := setupProjectMocks(t) - mockConfig := mocks.ConfigHandler.(*config.MockConfigHandler) - mockConfig.IsLoadedFunc = func() bool { - return false - } - mockConfig.IsDevModeFunc = func(contextName string) bool { - return true - } - mockConfig.GetStringFunc = func(key string, defaultValue ...string) string { - return "" - } - mockConfig.SetFunc = func(key string, value any) error { - if key == "dev" { - return fmt.Errorf("set dev failed") - } - return nil - } - - proj, err := NewProject("test-context", &Project{Runtime: mocks.Runtime}) - - if err == nil { - t.Fatal("Expected error for ApplyConfigDefaults failure") - } - if proj != nil { - t.Error("Expected Project to be nil on error") - } - }) - t.Run("ErrorOnWorkstationCreationFailure", func(t *testing.T) { mocks := setupProjectMocks(t) mockConfig := mocks.ConfigHandler.(*config.MockConfigHandler) @@ -421,29 +393,6 @@ func TestNewProject(t *testing.T) { } }) - t.Run("ErrorOnRuntimeCreationFailure", func(t *testing.T) { - mockShell := shell.NewMockShell() - mockShell.GetProjectRootFunc = func() (string, error) { - return "", fmt.Errorf("failed to get project root") - } - - proj, err := NewProject("test-context", &Project{ - Runtime: &runtime.Runtime{ - Shell: mockShell, - }, - }) - - if err == nil { - t.Fatal("Expected error for runtime creation failure") - } - if proj != nil { - t.Error("Expected Project to be nil on error") - } - if !strings.Contains(err.Error(), "failed to initialize context") && !strings.Contains(err.Error(), "failed to get project root") && !strings.Contains(err.Error(), "config handler not available") { - t.Errorf("Expected error about initializing context, getting project root, or config handler, got: %v", err) - } - }) - } // ============================================================================= @@ -643,6 +592,93 @@ func TestProject_Configure(t *testing.T) { } }) + t.Run("CallsApplyConfigDefaultsWithFlagOverrides", func(t *testing.T) { + // Given a project with flag overrides containing vm.driver=colima + mocks := setupProjectMocks(t) + mockConfig := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfig.IsLoadedFunc = func() bool { + return false + } + mockConfig.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfig.GetStringFunc = func(key string, defaultValue ...string) string { + return "" + } + + var setDefaultConfig v1alpha1.Context + mockConfig.SetDefaultFunc = func(cfg v1alpha1.Context) error { + setDefaultConfig = cfg + return nil + } + mockConfig.SetFunc = func(key string, value any) error { + return nil + } + + proj, err := NewProject("test-context", &Project{Runtime: mocks.Runtime}) + if err != nil { + t.Fatalf("Failed to create project: %v", err) + } + + flagOverrides := map[string]any{ + "vm.driver": "colima", + "provider": "generic", + } + + // When Configure is called with flag overrides + err = proj.Configure(flagOverrides) + + // Then ApplyConfigDefaults should be called with flag overrides, resulting in DefaultConfig_Full + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if setDefaultConfig.Network == nil || setDefaultConfig.Network.LoadBalancerIPs == nil { + t.Error("Expected DefaultConfig_Full with LoadBalancerIPs to be set (colima should use Full config)") + } + + if setDefaultConfig.Cluster != nil && len(setDefaultConfig.Cluster.Workers.HostPorts) > 0 { + t.Error("Expected DefaultConfig_Full without hostports to be set (colima should not have hostports)") + } + }) + + t.Run("ErrorOnApplyConfigDefaultsFailure", func(t *testing.T) { + // Given a project where SetDefault fails + mocks := setupProjectMocks(t) + mockConfig := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfig.IsLoadedFunc = func() bool { + return false + } + mockConfig.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfig.GetStringFunc = func(key string, defaultValue ...string) string { + return "" + } + + mockConfig.SetDefaultFunc = func(cfg v1alpha1.Context) error { + return fmt.Errorf("set default failed") + } + + proj, err := NewProject("test-context", &Project{Runtime: mocks.Runtime}) + if err != nil { + t.Fatalf("Failed to create project: %v", err) + } + + // When Configure is called + err = proj.Configure(nil) + + // Then an error should be returned + if err == nil { + t.Error("Expected error for ApplyConfigDefaults failure") + return + } + + if !strings.Contains(err.Error(), "failed to apply config defaults") { + t.Errorf("Expected error about apply config defaults, got: %v", err) + } + }) + } func TestProject_Initialize(t *testing.T) { @@ -672,6 +708,10 @@ func TestProject_Initialize(t *testing.T) { t.Fatalf("Failed to create project: %v", err) } + if err := proj.Configure(nil); err != nil { + t.Fatalf("Failed to configure project: %v", err) + } + if proj.Workstation == nil { t.Fatal("Expected workstation to be created") } @@ -834,6 +874,10 @@ func TestProject_Initialize(t *testing.T) { t.Fatalf("Failed to create project: %v", err) } + if err := proj.Configure(nil); err != nil { + t.Fatalf("Failed to configure project: %v", err) + } + if proj.Workstation == nil { t.Fatal("Expected workstation to be created") } @@ -851,7 +895,7 @@ func TestProject_Initialize(t *testing.T) { return } - if !strings.Contains(err.Error(), "failed to assign IPs to network manager") { + if !strings.Contains(err.Error(), "failed to assign IPs to services") { t.Errorf("Expected specific error message, got: %v", err) } }) diff --git a/pkg/runtime/env/docker_env.go b/pkg/runtime/env/docker_env.go index 276cf34bb..842fdf5dc 100644 --- a/pkg/runtime/env/docker_env.go +++ b/pkg/runtime/env/docker_env.go @@ -10,6 +10,7 @@ import ( "net" "os" "path/filepath" + "strings" "github.com/windsorcli/cli/pkg/runtime/config" "github.com/windsorcli/cli/pkg/runtime/shell" @@ -46,10 +47,19 @@ func NewDockerEnvPrinter(shell shell.Shell, configHandler config.ConfigHandler) func (e *DockerEnvPrinter) GetEnvVars() (map[string]string, error) { envVars := make(map[string]string) - if dockerHostValue, dockerHostExists := e.shims.LookupEnv("DOCKER_HOST"); dockerHostExists { - envVars["DOCKER_HOST"] = dockerHostValue - } else { - vmDriver := e.configHandler.GetString("vm.driver") + vmDriver := e.configHandler.GetString("vm.driver") + _, dockerHostExists := e.shims.LookupEnv("DOCKER_HOST") + _, managedEnvExists := e.shims.LookupEnv("WINDSOR_MANAGED_ENV") + + isDockerHostManaged := false + if managedEnvExists { + managedEnvStr := e.shims.Getenv("WINDSOR_MANAGED_ENV") + if strings.Contains(managedEnvStr, "DOCKER_HOST") { + isDockerHostManaged = true + } + } + + if vmDriver != "" && (!dockerHostExists || isDockerHostManaged) { homeDir, err := e.shims.UserHomeDir() if err != nil { return nil, fmt.Errorf("error retrieving user home directory: %w", err) @@ -69,21 +79,25 @@ func (e *DockerEnvPrinter) GetEnvVars() (map[string]string, error) { if e.shims.Goos() == "windows" { contextName = "desktop-linux" envVars["DOCKER_HOST"] = "npipe:////./pipe/docker_engine" + e.SetManagedEnv("DOCKER_HOST") } else { switch vmDriver { case "colima": contextName = fmt.Sprintf("colima-windsor-%s", configContext) dockerHostPath := fmt.Sprintf("unix://%s/.colima/windsor-%s/docker.sock", homeDir, configContext) envVars["DOCKER_HOST"] = dockerHostPath + e.SetManagedEnv("DOCKER_HOST") case "docker-desktop": contextName = "desktop-linux" dockerHostPath := fmt.Sprintf("unix://%s/.docker/run/docker.sock", homeDir) envVars["DOCKER_HOST"] = dockerHostPath + e.SetManagedEnv("DOCKER_HOST") case "docker": contextName = "default" envVars["DOCKER_HOST"] = "unix:///var/run/docker.sock" + e.SetManagedEnv("DOCKER_HOST") default: contextName = "default" @@ -109,11 +123,17 @@ func (e *DockerEnvPrinter) GetEnvVars() (map[string]string, error) { } } envVars["DOCKER_CONFIG"] = filepath.ToSlash(dockerConfigDir) + e.SetManagedEnv("DOCKER_CONFIG") + } else if dockerHostExists { + if dockerHostValue, _ := e.shims.LookupEnv("DOCKER_HOST"); dockerHostValue != "" { + envVars["DOCKER_HOST"] = dockerHostValue + } } registryURL, _ := e.getRegistryURL() if registryURL != "" { envVars["REGISTRY_URL"] = registryURL + e.SetManagedEnv("REGISTRY_URL") } return envVars, nil diff --git a/pkg/runtime/env/docker_env_test.go b/pkg/runtime/env/docker_env_test.go index 8dfd24369..51c29f9ee 100644 --- a/pkg/runtime/env/docker_env_test.go +++ b/pkg/runtime/env/docker_env_test.go @@ -541,6 +541,314 @@ contexts: if envVars["DOCKER_HOST"] != expectedDockerHost { t.Errorf("DOCKER_HOST = %v, want %v", envVars["DOCKER_HOST"], expectedDockerHost) } + + // And DOCKER_HOST should NOT be marked as managed (since it was manually set) + managedEnv := printer.GetManagedEnv() + isManaged := false + for _, env := range managedEnv { + if env == "DOCKER_HOST" { + isManaged = true + break + } + } + if isManaged { + t.Error("Expected DOCKER_HOST not to be marked as managed when manually set") + } + }) + + t.Run("DockerHostManagedByWindsorIsOverridden", func(t *testing.T) { + // Given a new DockerEnvPrinter with DOCKER_HOST already managed by Windsor + os.Unsetenv("DOCKER_HOST") + mocks := setupDockerEnvMocks(t) + configStr := ` +version: v1alpha1 +contexts: + test-context: + vm: + driver: colima + docker: + registries: + mock-registry-url: + hostport: 5000 +` + if err := mocks.ConfigHandler.LoadConfigString(configStr); err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + mocks.Shims.LookupEnv = func(key string) (string, bool) { + if key == "DOCKER_HOST" { + return "tcp://old-managed-host:2375", true + } + if key == "WINDSOR_MANAGED_ENV" { + return "DOCKER_HOST DOCKER_CONFIG", true + } + return "", false + } + + mocks.Shims.Getenv = func(key string) string { + if key == "WINDSOR_MANAGED_ENV" { + return "DOCKER_HOST DOCKER_CONFIG" + } + return "" + } + + printer := NewDockerEnvPrinter(mocks.Shell, mocks.ConfigHandler) + printer.shims = mocks.Shims + + // When getting environment variables + envVars, err := printer.GetEnvVars() + + // Then no error should be returned + if err != nil { + t.Fatalf("GetEnvVars returned an error: %v", err) + } + + // And DOCKER_HOST should be set based on vm.driver (not the old managed value) + var expectedDockerHost string + if mocks.Shims.Goos() == "windows" { + expectedDockerHost = "npipe:////./pipe/docker_engine" + } else { + expectedDockerHost = fmt.Sprintf("unix://%s/.colima/windsor-test-context/docker.sock", filepath.ToSlash("/mock/home")) + } + if envVars["DOCKER_HOST"] != expectedDockerHost { + t.Errorf("DOCKER_HOST = %v, want %v", envVars["DOCKER_HOST"], expectedDockerHost) + } + + // And DOCKER_HOST should be marked as managed + managedEnv := printer.GetManagedEnv() + isManaged := false + for _, env := range managedEnv { + if env == "DOCKER_HOST" { + isManaged = true + break + } + } + if !isManaged { + t.Error("Expected DOCKER_HOST to be marked as managed when set by vm.driver") + } + }) + + t.Run("DockerHostUnmanagedIsPreserved", func(t *testing.T) { + // Given a new DockerEnvPrinter with DOCKER_HOST set but not managed + mocks := setupDockerEnvMocks(t) + configStr := ` +version: v1alpha1 +contexts: + test-context: + vm: + driver: colima + docker: + registries: + mock-registry-url: + hostport: 5000 +` + if err := mocks.ConfigHandler.LoadConfigString(configStr); err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + mocks.Shims.LookupEnv = func(key string) (string, bool) { + if key == "DOCKER_HOST" { + return "tcp://custom-unmanaged-host:2375", true + } + if key == "WINDSOR_MANAGED_ENV" { + return "DOCKER_CONFIG", true // DOCKER_HOST not in managed list + } + return "", false + } + + printer := NewDockerEnvPrinter(mocks.Shell, mocks.ConfigHandler) + printer.shims = mocks.Shims + + // When getting environment variables + envVars, err := printer.GetEnvVars() + + // Then no error should be returned + if err != nil { + t.Fatalf("GetEnvVars returned an error: %v", err) + } + + // And DOCKER_HOST should preserve the unmanaged value + expectedDockerHost := "tcp://custom-unmanaged-host:2375" + if envVars["DOCKER_HOST"] != expectedDockerHost { + t.Errorf("DOCKER_HOST = %v, want %v", envVars["DOCKER_HOST"], expectedDockerHost) + } + + // And DOCKER_HOST should NOT be marked as managed + managedEnv := printer.GetManagedEnv() + isManaged := false + for _, env := range managedEnv { + if env == "DOCKER_HOST" { + isManaged = true + break + } + } + if isManaged { + t.Error("Expected DOCKER_HOST not to be marked as managed when unmanaged value is preserved") + } + }) + + t.Run("DockerHostNotSetWithVmDriver", func(t *testing.T) { + // Given a new DockerEnvPrinter with vm.driver set but no DOCKER_HOST + os.Unsetenv("DOCKER_HOST") + mocks := setupDockerEnvMocks(t) + configStr := ` +version: v1alpha1 +contexts: + test-context: + vm: + driver: docker-desktop + docker: + registries: + mock-registry-url: + hostport: 5000 +` + if err := mocks.ConfigHandler.LoadConfigString(configStr); err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + mocks.Shims.LookupEnv = func(key string) (string, bool) { + return "", false // DOCKER_HOST doesn't exist + } + + printer := NewDockerEnvPrinter(mocks.Shell, mocks.ConfigHandler) + printer.shims = mocks.Shims + + // When getting environment variables + envVars, err := printer.GetEnvVars() + + // Then no error should be returned + if err != nil { + t.Fatalf("GetEnvVars returned an error: %v", err) + } + + // And DOCKER_HOST should be set based on vm.driver + expectedDockerHost := fmt.Sprintf("unix://%s/.docker/run/docker.sock", filepath.ToSlash("/mock/home")) + if envVars["DOCKER_HOST"] != expectedDockerHost { + t.Errorf("DOCKER_HOST = %v, want %v", envVars["DOCKER_HOST"], expectedDockerHost) + } + + // And DOCKER_HOST should be marked as managed + managedEnv := printer.GetManagedEnv() + isManaged := false + for _, env := range managedEnv { + if env == "DOCKER_HOST" { + isManaged = true + break + } + } + if !isManaged { + t.Error("Expected DOCKER_HOST to be marked as managed when set by vm.driver") + } + }) + + t.Run("DockerHostNotSetWithoutVmDriver", func(t *testing.T) { + // Given a new DockerEnvPrinter without vm.driver and no DOCKER_HOST + os.Unsetenv("DOCKER_HOST") + baseMocks := setupEnvMocks(t) + mocks := setupDockerEnvMocks(t, &EnvTestMocks{ + ConfigHandler: config.NewConfigHandler(baseMocks.Shell), + }) + configStr := ` +version: v1alpha1 +contexts: + test-context: + docker: + registries: + mock-registry-url: + hostport: 5000 +` + if err := mocks.ConfigHandler.LoadConfigString(configStr); err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + mocks.Shims.LookupEnv = func(key string) (string, bool) { + return "", false // DOCKER_HOST doesn't exist + } + + printer := NewDockerEnvPrinter(mocks.Shell, mocks.ConfigHandler) + printer.shims = mocks.Shims + + // When getting environment variables + envVars, err := printer.GetEnvVars() + + // Then no error should be returned + if err != nil { + t.Fatalf("GetEnvVars returned an error: %v", err) + } + + // And DOCKER_HOST should not be set + if _, exists := envVars["DOCKER_HOST"]; exists { + t.Errorf("Expected DOCKER_HOST not to be set when vm.driver is empty, got %v", envVars["DOCKER_HOST"]) + } + + // And DOCKER_HOST should not be marked as managed + managedEnv := printer.GetManagedEnv() + isManaged := false + for _, env := range managedEnv { + if env == "DOCKER_HOST" { + isManaged = true + break + } + } + if isManaged { + t.Error("Expected DOCKER_HOST not to be marked as managed when not set") + } + }) + + t.Run("DockerHostEmptyStringIsPreserved", func(t *testing.T) { + // Given a new DockerEnvPrinter with DOCKER_HOST set to empty string (not managed) + mocks := setupDockerEnvMocks(t) + configStr := ` +version: v1alpha1 +contexts: + test-context: + vm: + driver: colima + docker: + registries: + mock-registry-url: + hostport: 5000 +` + if err := mocks.ConfigHandler.LoadConfigString(configStr); err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + mocks.Shims.LookupEnv = func(key string) (string, bool) { + if key == "DOCKER_HOST" { + return "", true // Empty string but exists + } + if key == "WINDSOR_MANAGED_ENV" { + return "DOCKER_CONFIG", true // DOCKER_HOST not in managed list + } + return "", false + } + + printer := NewDockerEnvPrinter(mocks.Shell, mocks.ConfigHandler) + printer.shims = mocks.Shims + + // When getting environment variables + envVars, err := printer.GetEnvVars() + + // Then no error should be returned + if err != nil { + t.Fatalf("GetEnvVars returned an error: %v", err) + } + + // And DOCKER_HOST should preserve the empty string value (since it exists and is not managed) + // Note: The code checks if DOCKER_HOST exists, and if it does and is not managed, it preserves it + // However, if the value is empty, it might still be preserved. Let's check the actual behavior. + if dockerHost, exists := envVars["DOCKER_HOST"]; exists { + // If DOCKER_HOST exists but is empty and not managed, it should be preserved + // But the code logic says: if vmDriver != "" && (!dockerHostExists || isDockerHostManaged) + // Since dockerHostExists is true and isDockerHostManaged is false, the condition is false + // So it goes to the else branch which preserves the existing value + if dockerHost != "" { + t.Errorf("DOCKER_HOST = %v, want empty string (preserved)", dockerHost) + } + } else { + // If DOCKER_HOST doesn't exist in envVars, that's also acceptable + // The code might not add it if it's empty + } }) } diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 756c64aee..5d0a1c984 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -599,8 +599,9 @@ func (rt *Runtime) incrementBuildID(existingBuildID, currentDate string) (string // It determines the appropriate default configuration (localhost, full, or standard) based on the VM driver // and dev mode settings. For dev mode, it also sets the provider to "generic" if not already set. // This method should be called before loading configuration from disk to ensure defaults are applied first. -// The context name is read from rt.ContextName. Returns an error if any configuration operation fails. -func (rt *Runtime) ApplyConfigDefaults() error { +// The context name is read from rt.ContextName. Optional flagOverrides can be provided to check vm.driver +// before it's set in config. Returns an error if any configuration operation fails. +func (rt *Runtime) ApplyConfigDefaults(flagOverrides ...map[string]any) error { contextName := rt.ContextName if contextName == "" { contextName = "local" @@ -625,6 +626,11 @@ func (rt *Runtime) ApplyConfigDefaults() error { } vmDriver := rt.ConfigHandler.GetString("vm.driver") + if vmDriver == "" && len(flagOverrides) > 0 && flagOverrides[0] != nil { + if driver, ok := flagOverrides[0]["vm.driver"].(string); ok && driver != "" { + vmDriver = driver + } + } if isDevMode && vmDriver == "" { switch runtime.GOOS { case "darwin", "windows": diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 128bd9025..afb97933c 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -1382,6 +1382,187 @@ func TestRuntime_ApplyConfigDefaults(t *testing.T) { t.Errorf("Expected error about set provider, got: %v", err) } }) + + t.Run("UsesFullConfigWhenColimaInFlagOverrides", func(t *testing.T) { + // Given a runtime in dev mode with colima in flag overrides + mocks := setupRuntimeMocks(t) + rt := mocks.Runtime + rt.ContextName = "local" + + mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfigHandler.IsLoadedFunc = func() bool { + return false + } + mockConfigHandler.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + return "" + } + + var setDefaultConfig v1alpha1.Context + mockConfigHandler.SetDefaultFunc = func(cfg v1alpha1.Context) error { + setDefaultConfig = cfg + return nil + } + + mockConfigHandler.SetFunc = func(key string, value interface{}) error { + return nil + } + + flagOverrides := map[string]any{ + "vm.driver": "colima", + } + + // When ApplyConfigDefaults is called with flag overrides + err := rt.ApplyConfigDefaults(flagOverrides) + + // Then full config should be set (not localhost config) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if setDefaultConfig.Network == nil || setDefaultConfig.Network.LoadBalancerIPs == nil { + t.Error("Expected DefaultConfig_Full with LoadBalancerIPs to be set") + } + + if setDefaultConfig.Cluster != nil && len(setDefaultConfig.Cluster.Workers.HostPorts) > 0 { + t.Error("Expected DefaultConfig_Full without hostports to be set") + } + }) + + t.Run("UsesLocalhostConfigWhenDockerDesktopInFlagOverrides", func(t *testing.T) { + // Given a runtime in dev mode with docker-desktop in flag overrides + mocks := setupRuntimeMocks(t) + rt := mocks.Runtime + rt.ContextName = "local" + + mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfigHandler.IsLoadedFunc = func() bool { + return false + } + mockConfigHandler.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + return "" + } + + var setDefaultConfig v1alpha1.Context + mockConfigHandler.SetDefaultFunc = func(cfg v1alpha1.Context) error { + setDefaultConfig = cfg + return nil + } + + mockConfigHandler.SetFunc = func(key string, value interface{}) error { + return nil + } + + flagOverrides := map[string]any{ + "vm.driver": "docker-desktop", + } + + // When ApplyConfigDefaults is called with flag overrides + err := rt.ApplyConfigDefaults(flagOverrides) + + // Then localhost config should be set (with hostports) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if setDefaultConfig.Cluster == nil || len(setDefaultConfig.Cluster.Workers.HostPorts) == 0 { + t.Error("Expected DefaultConfig_Localhost with hostports to be set") + } + }) + + t.Run("IgnoresFlagOverridesWhenVMDriverAlreadySet", func(t *testing.T) { + // Given a runtime in dev mode with vm.driver already set in config + mocks := setupRuntimeMocks(t) + rt := mocks.Runtime + rt.ContextName = "local" + + mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfigHandler.IsLoadedFunc = func() bool { + return false + } + mockConfigHandler.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + if key == "vm.driver" { + return "colima" + } + return "" + } + + var setDefaultConfig v1alpha1.Context + mockConfigHandler.SetDefaultFunc = func(cfg v1alpha1.Context) error { + setDefaultConfig = cfg + return nil + } + + mockConfigHandler.SetFunc = func(key string, value interface{}) error { + return nil + } + + flagOverrides := map[string]any{ + "vm.driver": "docker-desktop", + } + + // When ApplyConfigDefaults is called with flag overrides + err := rt.ApplyConfigDefaults(flagOverrides) + + // Then config should use colima from config (not docker-desktop from overrides) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if setDefaultConfig.Cluster != nil && len(setDefaultConfig.Cluster.Workers.HostPorts) > 0 { + t.Error("Expected DefaultConfig_Full without hostports to be set (colima), not localhost config") + } + }) + + t.Run("IgnoresEmptyFlagOverrides", func(t *testing.T) { + // Given a runtime in dev mode with empty flag overrides + mocks := setupRuntimeMocks(t) + rt := mocks.Runtime + rt.ContextName = "local" + + mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) + mockConfigHandler.IsLoadedFunc = func() bool { + return false + } + mockConfigHandler.IsDevModeFunc = func(contextName string) bool { + return true + } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + return "" + } + + setDefaultCalled := false + mockConfigHandler.SetDefaultFunc = func(cfg v1alpha1.Context) error { + setDefaultCalled = true + return nil + } + + mockConfigHandler.SetFunc = func(key string, value interface{}) error { + return nil + } + + flagOverrides := map[string]any{} + + // When ApplyConfigDefaults is called with empty flag overrides + err := rt.ApplyConfigDefaults(flagOverrides) + + // Then defaults should still be applied (using OS default) + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if !setDefaultCalled { + t.Error("Expected SetDefault to be called even with empty flag overrides") + } + }) } func TestRuntime_ApplyProviderDefaults(t *testing.T) { diff --git a/pkg/workstation/workstation.go b/pkg/workstation/workstation.go index d41d11fe0..f4d841078 100644 --- a/pkg/workstation/workstation.go +++ b/pkg/workstation/workstation.go @@ -5,6 +5,7 @@ import ( "os" "github.com/windsorcli/cli/pkg/runtime" + "github.com/windsorcli/cli/pkg/runtime/shell" "github.com/windsorcli/cli/pkg/runtime/shell/ssh" "github.com/windsorcli/cli/pkg/workstation/network" "github.com/windsorcli/cli/pkg/workstation/services" @@ -74,63 +75,72 @@ func NewWorkstation(rt *runtime.Runtime, opts ...*Workstation) (*Workstation, er } } - // Create NetworkManager if not already set - if workstation.NetworkManager == nil { - workstation.NetworkManager = network.NewBaseNetworkManager(rt) + if workstation.SSHClient == nil { + workstation.SSHClient = ssh.NewSSHClient() } - // Create Services if not already set - if workstation.Services == nil { - serviceList, err := workstation.createServices() - if err != nil { - return nil, fmt.Errorf("failed to create services: %w", err) + return workstation, nil +} + +// ============================================================================= +// Public Methods +// ============================================================================= + +// Prepare creates all workstation components (network manager, services, virtual machine, +// container runtime) and assigns IP addresses to services. This must be called after +// configuration is loaded but before blueprint loading, as blueprint evaluation depends +// on service addresses being set in config. Returns an error if any component creation +// or IP assignment fails. +func (w *Workstation) Prepare() error { + vmDriver := w.ConfigHandler.GetString("vm.driver") + + if w.NetworkManager == nil { + if vmDriver == "colima" { + secureShell := shell.NewSecureShell(w.SSHClient) + networkInterfaceProvider := network.NewNetworkInterfaceProvider() + w.NetworkManager = network.NewColimaNetworkManager(w.Runtime, w.SSHClient, secureShell, networkInterfaceProvider) + } else { + w.NetworkManager = network.NewBaseNetworkManager(w.Runtime) } - workstation.Services = serviceList } - // Create VirtualMachine if not already set - if workstation.VirtualMachine == nil { - vmDriver := workstation.ConfigHandler.GetString("vm.driver", "") - if vmDriver == "colima" { - workstation.VirtualMachine = virt.NewColimaVirt(rt) + if w.Services == nil { + serviceList, err := w.createServices() + if err != nil { + return fmt.Errorf("failed to create services: %w", err) } + w.Services = serviceList } - // Create ContainerRuntime if not already set - if workstation.ContainerRuntime == nil { - dockerEnabled := workstation.ConfigHandler.GetBool("docker.enabled", false) - if dockerEnabled { - workstation.ContainerRuntime = virt.NewDockerVirt(rt, workstation.Services) + if w.NetworkManager != nil { + if err := w.NetworkManager.AssignIPs(w.Services); err != nil { + return fmt.Errorf("failed to assign IPs to services: %w", err) } } - // Create SSHClient if not already set - if workstation.SSHClient == nil { - workstation.SSHClient = ssh.NewSSHClient() + if vmDriver == "colima" && w.VirtualMachine == nil { + w.VirtualMachine = virt.NewColimaVirt(w.Runtime) } - return workstation, nil -} + containerRuntimeEnabled := w.ConfigHandler.GetBool("docker.enabled") + if containerRuntimeEnabled && w.ContainerRuntime == nil { + w.ContainerRuntime = virt.NewDockerVirt(w.Runtime, w.Services) + } -// ============================================================================= -// Public Methods -// ============================================================================= + return nil +} -// Up starts the workstation environment including VMs, containers, networking, and services. -// It sets the NO_CACHE environment variable, launches the virtual machine if the driver is "colima", -// initializes the network manager for all registered services, re-initializes DNS services, -// writes service configurations, initializes and starts the container runtime if enabled, -// configures networking components, and informs the user of successful environment setup. +// Up initializes the workstation environment: starts VMs, containers, networking, and services. +// Sets NO_CACHE environment variable, starts the virtual machine if configured, writes service +// configs, starts the container runtime if enabled, and configures networking. All components +// must be created via Prepare() before calling Up(). Returns error on any failure. func (w *Workstation) Up() error { if err := os.Setenv("NO_CACHE", "true"); err != nil { return fmt.Errorf("Error setting NO_CACHE environment variable: %w", err) } vmDriver := w.ConfigHandler.GetString("vm.driver") - if vmDriver == "colima" { - if w.VirtualMachine == nil { - return fmt.Errorf("no virtual machine found") - } + if vmDriver == "colima" && w.VirtualMachine != nil { if err := w.VirtualMachine.Up(); err != nil { return fmt.Errorf("error running virtual machine Up command: %w", err) } @@ -142,11 +152,7 @@ func (w *Workstation) Up() error { } } - containerRuntimeEnabled := w.ConfigHandler.GetBool("docker.enabled") - if containerRuntimeEnabled { - if w.ContainerRuntime == nil { - return fmt.Errorf("no container runtime found") - } + if w.ContainerRuntime != nil { if err := w.ContainerRuntime.WriteConfig(); err != nil { return fmt.Errorf("failed to write container runtime config: %w", err) } @@ -156,7 +162,6 @@ func (w *Workstation) Up() error { } if w.NetworkManager != nil { - // Only configure guest and host routes for colima if vmDriver == "colima" { if err := w.NetworkManager.ConfigureGuest(); err != nil { return fmt.Errorf("error configuring guest: %w", err) @@ -165,8 +170,6 @@ func (w *Workstation) Up() error { return fmt.Errorf("error configuring host route: %w", err) } } - - // Configure DNS if enabled if dnsEnabled := w.ConfigHandler.GetBool("dns.enabled"); dnsEnabled { if err := w.NetworkManager.ConfigureDNS(); err != nil { return fmt.Errorf("error configuring DNS: %w", err) diff --git a/pkg/workstation/workstation_test.go b/pkg/workstation/workstation_test.go index 74c95b3d9..70d57ee01 100644 --- a/pkg/workstation/workstation_test.go +++ b/pkg/workstation/workstation_test.go @@ -205,139 +205,155 @@ func setupWorkstationMocks(t *testing.T, opts ...func(*WorkstationTestMocks)) *W func TestNewWorkstation(t *testing.T) { t.Run("Success", func(t *testing.T) { - // Given + // Given a properly configured runtime with all required dependencies mocks := setupWorkstationMocks(t) - // When + // When creating a new workstation with the runtime workstation, err := NewWorkstation(mocks.Runtime) - // Then + // Then the workstation should be created successfully without errors if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And the workstation should not be nil if workstation == nil { t.Error("Expected workstation to be created") } + // And the ConfigHandler should be set if workstation.ConfigHandler == nil { t.Error("Expected ConfigHandler to be set") } + // And the Shell should be set if workstation.Shell == nil { t.Error("Expected Shell to be set") } }) t.Run("NilContext", func(t *testing.T) { - // Given + // Given a nil runtime is provided _ = setupWorkstationMocks(t) - // When + // When creating a new workstation with nil runtime workstation, err := NewWorkstation(nil) - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for nil context") } + // And the workstation should be nil if workstation != nil { t.Error("Expected workstation to be nil") } + // And the error message should indicate runtime is required if err.Error() != "runtime is required" { t.Errorf("Expected specific error message, got: %v", err) } }) t.Run("NilConfigHandler", func(t *testing.T) { - // Given + // Given a runtime with nil ConfigHandler mocks := setupWorkstationMocks(t) rt := &ctxpkg.Runtime{ Shell: mocks.Shell, } - // When + // When creating a new workstation with the incomplete runtime workstation, err := NewWorkstation(rt) - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for nil ConfigHandler") } + // And the workstation should be nil if workstation != nil { t.Error("Expected workstation to be nil") } + // And the error message should indicate ConfigHandler is required if err.Error() != "ConfigHandler is required on runtime" { t.Errorf("Expected specific error message, got: %v", err) } }) t.Run("NilShell", func(t *testing.T) { - // Given + // Given a runtime with nil Shell mocks := setupWorkstationMocks(t) rt := &ctxpkg.Runtime{ ConfigHandler: mocks.ConfigHandler, } - // When + // When creating a new workstation with the incomplete runtime workstation, err := NewWorkstation(rt) - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for nil Shell") } + // And the workstation should be nil if workstation != nil { t.Error("Expected workstation to be nil") } + // And the error message should indicate Shell is required if err.Error() != "Shell is required on runtime" { t.Errorf("Expected specific error message, got: %v", err) } }) t.Run("NilRuntime", func(t *testing.T) { - // Given + // Given a nil runtime is provided _ = setupWorkstationMocks(t) - // When + // When creating a new workstation with nil runtime workstation, err := NewWorkstation(nil) - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for nil injector") } + // And the workstation should be nil if workstation != nil { t.Error("Expected workstation to be nil") } + // And the error message should indicate runtime is required if err.Error() != "runtime is required" { t.Errorf("Expected specific error message, got: %v", err) } }) t.Run("CreatesDependencies", func(t *testing.T) { - // Given + // Given a properly configured runtime mocks := setupWorkstationMocks(t) - // When + // When creating a new workstation workstation, err := NewWorkstation(mocks.Runtime) - // Then + // Then the workstation should be created successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } - if workstation.NetworkManager == nil { - t.Error("Expected NetworkManager to be created") + // And SSHClient should be created + if workstation.SSHClient == nil { + t.Error("Expected SSHClient to be created") } - if workstation.Services == nil { - t.Error("Expected Services to be created") + // And NetworkManager should not be created yet (created in Prepare) + if workstation.NetworkManager != nil { + t.Error("Expected NetworkManager not to be created in NewWorkstation (created in Prepare)") } - if workstation.VirtualMachine == nil { - t.Error("Expected VirtualMachine to be created") + // And Services should not be created yet (created in Prepare) + if workstation.Services != nil { + t.Error("Expected Services not to be created in NewWorkstation (created in Prepare)") } - if workstation.ContainerRuntime == nil { - t.Error("Expected ContainerRuntime to be created") + // And VirtualMachine should not be created yet (created in Prepare) + if workstation.VirtualMachine != nil { + t.Error("Expected VirtualMachine not to be created in NewWorkstation (created in Prepare)") } - if workstation.SSHClient == nil { - t.Error("Expected SSHClient to be created") + // And ContainerRuntime should not be created yet (created in Prepare) + if workstation.ContainerRuntime != nil { + t.Error("Expected ContainerRuntime not to be created in NewWorkstation (created in Prepare)") } }) t.Run("UsesExistingDependencies", func(t *testing.T) { - // Given + // Given a runtime and workstation options with pre-configured dependencies mocks := setupWorkstationMocks(t) opts := &Workstation{ Runtime: mocks.Runtime, @@ -348,25 +364,30 @@ func TestNewWorkstation(t *testing.T) { SSHClient: mocks.SSHClient, } - // When + // When creating a new workstation with the provided options workstation, err := NewWorkstation(mocks.Runtime, opts) - // Then + // Then the workstation should be created successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And the existing NetworkManager should be used if workstation.NetworkManager != mocks.NetworkManager { t.Error("Expected existing NetworkManager to be used") } + // And the existing Services should be used if len(workstation.Services) != 1 { t.Error("Expected existing Services to be used") } + // And the existing VirtualMachine should be used if workstation.VirtualMachine != mocks.VirtualMachine { t.Error("Expected existing VirtualMachine to be used") } + // And the existing ContainerRuntime should be used if workstation.ContainerRuntime != mocks.ContainerRuntime { t.Error("Expected existing ContainerRuntime to be used") } + // And the existing SSHClient should be used if workstation.SSHClient != mocks.SSHClient { t.Error("Expected existing SSHClient to be used") } @@ -379,7 +400,7 @@ func TestNewWorkstation(t *testing.T) { func TestWorkstation_Up(t *testing.T) { t.Run("Success", func(t *testing.T) { - // Given + // Given a workstation with all dependencies configured mocks := setupWorkstationMocks(t) workstation, err := NewWorkstation(mocks.Runtime, &Workstation{ VirtualMachine: mocks.VirtualMachine, @@ -391,17 +412,17 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully without errors if err != nil { t.Errorf("Expected success, got error: %v", err) } }) t.Run("SetsNoCacheEnvironmentVariable", func(t *testing.T) { - // Given + // Given a workstation with all dependencies configured mocks := setupWorkstationMocks(t) workstation, err := NewWorkstation(mocks.Runtime, &Workstation{ VirtualMachine: mocks.VirtualMachine, @@ -413,20 +434,21 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And the NO_CACHE environment variable should be set to "true" if os.Getenv("NO_CACHE") != "true" { t.Error("Expected NO_CACHE environment variable to be set") } }) t.Run("StartsVirtualMachine", func(t *testing.T) { - // Given + // Given a workstation with a virtual machine configured and a tracking flag for Up() calls mocks := setupWorkstationMocks(t) vmUpCalled := false mocks.VirtualMachine.UpFunc = func(verbose ...bool) error { @@ -443,20 +465,21 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And VirtualMachine.Up() should be called if !vmUpCalled { t.Error("Expected VirtualMachine.Up to be called") } }) t.Run("StartsContainerRuntime", func(t *testing.T) { - // Given + // Given a workstation with a container runtime configured and a tracking flag for Up() calls mocks := setupWorkstationMocks(t) containerUpCalled := false mocks.ContainerRuntime.UpFunc = func(verbose ...bool) error { @@ -473,20 +496,21 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And ContainerRuntime.Up() should be called if !containerUpCalled { t.Error("Expected ContainerRuntime.Up to be called") } }) t.Run("ConfiguresNetworking", func(t *testing.T) { - // Given + // Given a workstation with network manager configured and tracking flags for network configuration calls mocks := setupWorkstationMocks(t) hostRouteCalled := false guestCalled := false @@ -514,26 +538,29 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And ConfigureHostRoute should be called if !hostRouteCalled { t.Error("Expected ConfigureHostRoute to be called") } + // And ConfigureGuest should be called if !guestCalled { t.Error("Expected ConfigureGuest to be called") } + // And ConfigureDNS should be called if !dnsCalled { t.Error("Expected ConfigureDNS to be called") } }) t.Run("WritesServiceConfigs", func(t *testing.T) { - // Given + // Given a workstation with services configured and a tracking flag for WriteConfig() calls mocks := setupWorkstationMocks(t) writeConfigCalled := false for _, service := range mocks.Services { @@ -552,20 +579,21 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then the workstation should start successfully if err != nil { t.Errorf("Expected success, got error: %v", err) } + // And WriteConfig() should be called on each service if !writeConfigCalled { t.Error("Expected service WriteConfig to be called") } }) t.Run("VirtualMachineUpError", func(t *testing.T) { - // Given + // Given a workstation with a virtual machine that will fail when starting mocks := setupWorkstationMocks(t) mocks.VirtualMachine.UpFunc = func(verbose ...bool) error { return fmt.Errorf("VM start failed") @@ -580,20 +608,21 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for VM start failure") } + // And the error message should indicate virtual machine Up command failure if !strings.Contains(err.Error(), "error running virtual machine Up command") { t.Errorf("Expected specific error message, got: %v", err) } }) t.Run("ContainerRuntimeUpError", func(t *testing.T) { - // Given + // Given a workstation with a container runtime that will fail when starting mocks := setupWorkstationMocks(t) mocks.ContainerRuntime.UpFunc = func(verbose ...bool) error { return fmt.Errorf("container start failed") @@ -608,13 +637,14 @@ func TestWorkstation_Up(t *testing.T) { t.Fatalf("Failed to create workstation: %v", err) } - // When + // When calling Up() to start the workstation err = workstation.Up() - // Then + // Then an error should be returned if err == nil { t.Error("Expected error for container start failure") } + // And the error message should indicate container runtime Up command failure if !strings.Contains(err.Error(), "error running container runtime Up command") { t.Errorf("Expected specific error message, got: %v", err) } From aa84de58432b1fd2ae1d8f1302ac3b31b7051478 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 21 Nov 2025 17:35:55 -0500 Subject: [PATCH 2/2] Fix Windows test Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/runtime/env/docker_env_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/runtime/env/docker_env_test.go b/pkg/runtime/env/docker_env_test.go index 51c29f9ee..8ec22d3a8 100644 --- a/pkg/runtime/env/docker_env_test.go +++ b/pkg/runtime/env/docker_env_test.go @@ -721,8 +721,13 @@ contexts: t.Fatalf("GetEnvVars returned an error: %v", err) } - // And DOCKER_HOST should be set based on vm.driver - expectedDockerHost := fmt.Sprintf("unix://%s/.docker/run/docker.sock", filepath.ToSlash("/mock/home")) + // And DOCKER_HOST should be set based on vm.driver and OS + var expectedDockerHost string + if mocks.Shims.Goos() == "windows" { + expectedDockerHost = "npipe:////./pipe/docker_engine" + } else { + expectedDockerHost = fmt.Sprintf("unix://%s/.docker/run/docker.sock", filepath.ToSlash("/mock/home")) + } if envVars["DOCKER_HOST"] != expectedDockerHost { t.Errorf("DOCKER_HOST = %v, want %v", envVars["DOCKER_HOST"], expectedDockerHost) }