From 92d43b79218d14f9595f26cc4918171df448c819 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Sat, 8 Mar 2025 09:05:12 +0700 Subject: [PATCH 1/9] Persistent VM Naming --- cli_config/cli_config.go | 8 ++--- cmd/vm_delete.go | 8 ++++- cmd/vm_start.go | 7 ++++- trellis/trellis.go | 28 ++++++++++++----- trellis/vm_instance.go | 68 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 trellis/vm_instance.go diff --git a/cli_config/cli_config.go b/cli_config/cli_config.go index 11b9048a..ace80009 100644 --- a/cli_config/cli_config.go +++ b/cli_config/cli_config.go @@ -17,10 +17,10 @@ type VmImage struct { } type VmConfig struct { - Manager string `yaml:"manager"` - HostsResolver string `yaml:"hosts_resolver"` - Images []VmImage `yaml:"images"` - Ubuntu string `yaml:"ubuntu"` + Manager string `yaml:"manager"` + HostsResolver string `yaml:"hosts_resolver"` + Ubuntu string `yaml:"ubuntu"` + InstanceName string `yaml:"instance_name"` // Custom name for the Lima VM instance } type Config struct { diff --git a/cmd/vm_delete.go b/cmd/vm_delete.go index fa00db84..44155212 100644 --- a/cmd/vm_delete.go +++ b/cmd/vm_delete.go @@ -3,6 +3,8 @@ package cmd import ( "flag" "strings" + "os" + "path/filepath" "github.com/manifoldco/promptui" "github.com/mitchellh/cli" @@ -67,7 +69,11 @@ func (c *VmDeleteCommand) Run(args []string) int { if err := manager.DeleteInstance(siteName); err != nil { c.UI.Error("Error: " + err.Error()) return 1 - } + } + + // Remove instance file if it exists + instancePath := filepath.Join(c.Trellis.ConfigPath(), "lima", "instance") + os.Remove(instancePath) // Ignore errors as file may not exist } return 0 diff --git a/cmd/vm_start.go b/cmd/vm_start.go index 3c2128eb..9fb1b7d6 100644 --- a/cmd/vm_start.go +++ b/cmd/vm_start.go @@ -67,7 +67,7 @@ func (c *VmStartCommand) Run(args []string) int { return 0 } - if !errors.Is(err, vm.VmNotFoundErr) { + if (!errors.Is(err, vm.VmNotFoundErr)) { c.UI.Error("Error starting VM.") c.UI.Error(err.Error()) return 1 @@ -80,6 +80,11 @@ func (c *VmStartCommand) Run(args []string) int { return 1 } + // Save the instance name for future reference + if err = c.Trellis.SaveVMInstanceName(siteName); err != nil { + c.UI.Warn("Warning: Failed to save VM instance name. VM was created successfully, but future commands may not recognize it.") + } + if err = manager.StartInstance(siteName); err != nil { c.UI.Error("Error starting VM.") c.UI.Error(err.Error()) diff --git a/trellis/trellis.go b/trellis/trellis.go index 83e229cc..eaf5cebb 100644 --- a/trellis/trellis.go +++ b/trellis/trellis.go @@ -263,16 +263,28 @@ func (t *Trellis) FindSiteNameFromEnvironment(environment string, siteNameArg st return "", fmt.Errorf("Error: %s is not a valid site. Valid options are %s", siteNameArg, siteNames) } -func (t *Trellis) MainSiteFromEnvironment(environment string) (string, *Site, error) { - sites := t.SiteNamesFromEnvironment(environment) - - if len(sites) == 0 { - return "", nil, fmt.Errorf("Error: No sites found in %s environment", environment) +func (t *Trellis) MainSiteFromEnvironment(env string) (string, *Site, error) { + if _, ok := t.Environments[env]; !ok { + return "", nil, fmt.Errorf("environment %s not found", env) } - name := sites[0] - - return name, t.Environments[environment].WordPressSites[name], nil + // Get the instance name using our new priority system + siteName, err := t.GetVMInstanceName() + if err != nil || siteName == "" { + // Fall back to using the first site name if there's an error or empty result + siteNames := t.SiteNamesFromEnvironment(env) + if len(siteNames) == 0 { + return "", nil, fmt.Errorf("no sites found in environment %s", env) + } + siteName = siteNames[0] + } + + site, ok := t.Environments[env].WordPressSites[siteName] + if !ok { + return "", nil, fmt.Errorf("site %s not found in environment %s", siteName, env) + } + + return siteName, site, nil } func (t *Trellis) getDefaultSiteNameFromEnvironment(environment string) (siteName string, err error) { diff --git a/trellis/vm_instance.go b/trellis/vm_instance.go new file mode 100644 index 00000000..bb6f9663 --- /dev/null +++ b/trellis/vm_instance.go @@ -0,0 +1,68 @@ +package trellis + +import ( + "os" + "path/filepath" + "strings" +) + +const ( + LimaDirName = "lima" + InstanceFile = "instance" +) + +// GetVMInstanceName returns the VM instance name based on the following priority: +// 1. Instance file in .trellis/lima/instance +// 2. CliConfig instance_name setting +// 3. First site in development environment's wordpress_sites.yml +func (t *Trellis) GetVMInstanceName() (string, error) { + // 1. Check for instance file + instanceName, err := t.readInstanceNameFromFile() + if err == nil && instanceName != "" { + return instanceName, nil + } + + // 2. Check CLI config for instance_name + if t.CliConfig.Vm.InstanceName != "" { + return t.CliConfig.Vm.InstanceName, nil + } + + // 3. Simply use the first site in the development environment + config := t.Environments["development"] + if config == nil || len(config.WordPressSites) == 0 { + return "", nil + } + + // Get the first site name alphabetically (which is the default behavior) + siteNames := t.SiteNamesFromEnvironment("development") + if len(siteNames) > 0 { + return siteNames[0], nil + } + + return "", nil +} + +// SaveVMInstanceName writes the VM instance name to the instance file +func (t *Trellis) SaveVMInstanceName(instanceName string) error { + limaDir := filepath.Join(t.ConfigPath(), LimaDirName) + + // Create the lima directory if it doesn't exist + if err := os.MkdirAll(limaDir, 0755); err != nil && !os.IsExist(err) { + return err + } + + instancePath := filepath.Join(limaDir, InstanceFile) + return os.WriteFile(instancePath, []byte(instanceName), 0644) +} + +// readInstanceNameFromFile reads the VM instance name from the instance file +func (t *Trellis) readInstanceNameFromFile() (string, error) { + instancePath := filepath.Join(t.ConfigPath(), LimaDirName, InstanceFile) + + data, err := os.ReadFile(instancePath) + if err != nil { + return "", err + } + + return strings.TrimSpace(string(data)), nil +} From cde5567592b3d295ecfc01832dea9cfef70a01e9 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Sat, 8 Mar 2025 09:11:07 +0700 Subject: [PATCH 2/9] Tests Addition --- cmd/vm_delete_test.go | 49 ++++++++++ cmd/vm_start_test.go | 92 ++++++++++++++++++ trellis/trellis_test.go | 83 +++++++++++++++++ trellis/vm_instance_test.go | 181 ++++++++++++++++++++++++++++++++++++ 4 files changed, 405 insertions(+) create mode 100644 trellis/vm_instance_test.go diff --git a/cmd/vm_delete_test.go b/cmd/vm_delete_test.go index 581a1b12..974c7b02 100644 --- a/cmd/vm_delete_test.go +++ b/cmd/vm_delete_test.go @@ -1,6 +1,8 @@ package cmd import ( + "os" + "path/filepath" "strings" "testing" @@ -54,3 +56,50 @@ func TestVmDeleteRunValidations(t *testing.T) { }) } } + +func TestVmDeleteRemovesInstanceFile(t *testing.T) { + cleanup := trellis.LoadFixtureProject(t) + defer cleanup() + + // Setup test environment + ui := cli.NewMockUi() + mockTrellis := trellis.NewTrellis() + mockTrellis.LoadProject() + + // Create the lima directory and instance file + limaDir := filepath.Join(mockTrellis.ConfigPath(), "lima") + os.MkdirAll(limaDir, 0755) + instancePath := filepath.Join(limaDir, "instance") + os.WriteFile(instancePath, []byte("example.com"), 0644) + + // Verify file exists before test + if _, err := os.Stat(instancePath); os.IsNotExist(err) { + t.Fatalf("failed to create test instance file") + } + + // Create command + vmDeleteCommand := NewVmDeleteCommand(ui, mockTrellis) + vmDeleteCommand.force = true // Skip confirmation prompt + + // Replace VM manager with mock + originalNewVmManager := newVmManager + mockManager := &MockVmManager{} + newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { + return mockManager, nil + } + defer func() { newVmManager = originalNewVmManager }() + + // Run command + code := vmDeleteCommand.Run([]string{}) + + // Check command succeeded + if code != 0 { + t.Errorf("expected exit code 0, got %d", code) + } + + // Check instance file was removed + _, err := os.Stat(instancePath) + if !os.IsNotExist(err) { + t.Error("expected instance file to be deleted") + } +} diff --git a/cmd/vm_start_test.go b/cmd/vm_start_test.go index 143ac68e..087a9d3a 100644 --- a/cmd/vm_start_test.go +++ b/cmd/vm_start_test.go @@ -1,10 +1,13 @@ package cmd import ( + "os" + "path/filepath" "strings" "testing" "github.com/mitchellh/cli" + "github.com/roots/trellis-cli/pkg/vm" "github.com/roots/trellis-cli/trellis" ) @@ -54,3 +57,92 @@ func TestVmStartRunValidations(t *testing.T) { }) } } + +// MockVmManager for testing +type MockVmManager struct { + createCalled bool + startCalled bool + siteName string +} + +func (m *MockVmManager) CreateInstance(name string) error { + m.createCalled = true + m.siteName = name + return nil +} + +func (m *MockVmManager) StartInstance(name string) error { + m.startCalled = true + m.siteName = name + // First call returns VmNotFoundErr to trigger creation + if !m.createCalled { + return vm.VmNotFoundErr + } + return nil +} + +func (m *MockVmManager) StopInstance(name string) error { + return nil +} + +func (m *MockVmManager) DeleteInstance(name string) error { + return nil +} + +func TestVmStartSavesInstanceName(t *testing.T) { + cleanup := trellis.LoadFixtureProject(t) + defer cleanup() + + // Setup test environment + ui := cli.NewMockUi() + mockTrellis := trellis.NewTrellis() + mockTrellis.LoadProject() + + // Create command + vmStartCommand := NewVmStartCommand(ui, mockTrellis) + + // Replace VM manager with mock + originalNewVmManager := newVmManager + mockManager := &MockVmManager{} + newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { + return mockManager, nil + } + defer func() { newVmManager = originalNewVmManager }() + + // Mock provision command to return success + originalNewProvisionCommand := NewProvisionCommand + NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { + cmd := &ProvisionCommand{UI: ui, Trellis: trellis} + cmd.Run = func(args []string) int { + return 0 + } + return cmd + } + defer func() { NewProvisionCommand = originalNewProvisionCommand }() + + // Run command + code := vmStartCommand.Run([]string{}) + + // Check VM was created and started + if code != 0 { + t.Errorf("expected exit code 0, got %d", code) + } + if !mockManager.createCalled { + t.Error("expected CreateInstance to be called") + } + if !mockManager.startCalled { + t.Error("expected StartInstance to be called") + } + + // Check instance file was created + instancePath := filepath.Join(mockTrellis.ConfigPath(), "lima", "instance") + data, err := os.ReadFile(instancePath) + if err != nil { + t.Errorf("expected instance file to exist: %v", err) + } + + instanceName := strings.TrimSpace(string(data)) + if instanceName != mockManager.siteName { + t.Errorf("expected instance name %q, got %q", mockManager.siteName, instanceName) + } +} diff --git a/trellis/trellis_test.go b/trellis/trellis_test.go index b51a62e7..2dd8f96c 100644 --- a/trellis/trellis_test.go +++ b/trellis/trellis_test.go @@ -444,3 +444,86 @@ ask_vault_pass: true t.Errorf("expected load project to load project CLI config file") } } + +func TestGetVMInstanceName(t *testing.T) { + defer LoadFixtureProject(t)() + + tp := NewTrellis() + err := tp.LoadProject() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Test case 1: No instance file and no config setting - should use first site + instanceName, err := tp.GetVMInstanceName() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + expectedName := tp.SiteNamesFromEnvironment("development")[0] + if instanceName != expectedName { + t.Errorf("expected instance name %q, got %q", expectedName, instanceName) + } + + // Test case 2: With config setting - should use config value + tp.CliConfig.Vm.InstanceName = "configured-name" + instanceName, err = tp.GetVMInstanceName() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if instanceName != "configured-name" { + t.Errorf("expected instance name %q, got %q", "configured-name", instanceName) + } + + // Test case 3: With instance file - should use file value (highest priority) + // Create the instance file + limaDir := filepath.Join(tp.ConfigPath(), "lima") + if err := os.MkdirAll(limaDir, 0755); err != nil { + t.Fatalf("failed to create lima directory: %v", err) + } + instancePath := filepath.Join(limaDir, "instance") + if err := os.WriteFile(instancePath, []byte("file-instance-name"), 0644); err != nil { + t.Fatalf("failed to write instance file: %v", err) + } + + instanceName, err = tp.GetVMInstanceName() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if instanceName != "file-instance-name" { + t.Errorf("expected instance name %q, got %q", "file-instance-name", instanceName) + } + + // Clean up + os.Remove(instancePath) + tp.CliConfig.Vm.InstanceName = "" +} + +func TestSaveVMInstanceName(t *testing.T) { + defer LoadFixtureProject(t)() + + tp := NewTrellis() + err := tp.LoadProject() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Test saving the instance name + err = tp.SaveVMInstanceName("test-instance") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify file was created and contains correct content + instancePath := filepath.Join(tp.ConfigPath(), "lima", "instance") + data, err := os.ReadFile(instancePath) + if err != nil { + t.Fatalf("failed to read instance file: %v", err) + } + + if string(data) != "test-instance" { + t.Errorf("expected instance file to contain %q, got %q", "test-instance", string(data)) + } + + // Clean up + os.Remove(instancePath) +} diff --git a/trellis/vm_instance_test.go b/trellis/vm_instance_test.go new file mode 100644 index 00000000..25039a2b --- /dev/null +++ b/trellis/vm_instance_test.go @@ -0,0 +1,181 @@ +package trellis + +import ( + "os" + "path/filepath" + "testing" +) + +func TestGetVMInstanceName(t *testing.T) { + tempDir := t.TempDir() + defer TestChdir(t, tempDir)() + + // Create a mock Trellis structure + tp := &Trellis{ + ConfigDir: ".trellis", + Path: tempDir, + Environments: map[string]*Config{ + "development": { + WordPressSites: map[string]*Site{ + "example.com": {}, + "another-site.com": {}, + }, + }, + }, + } + + // Create config directory + if err := tp.CreateConfigDir(); err != nil { + t.Fatalf("Failed to create config directory: %v", err) + } + + // Test case 1: No instance file, no config setting + // Should return the first site alphabetically (another-site.com) + name, err := tp.GetVMInstanceName() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if name != "another-site.com" { + t.Errorf("Expected 'another-site.com', got '%s'", name) + } + + // Test case 2: With config setting + tp.CliConfig.Vm.InstanceName = "custom-name" + name, err = tp.GetVMInstanceName() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if name != "custom-name" { + t.Errorf("Expected 'custom-name', got '%s'", name) + } + + // Test case 3: With instance file (highest priority) + limaDir := filepath.Join(tp.ConfigPath(), LimaDirName) + if err := os.MkdirAll(limaDir, 0755); err != nil { + t.Fatalf("Failed to create lima directory: %v", err) + } + instancePath := filepath.Join(limaDir, InstanceFile) + if err := os.WriteFile(instancePath, []byte("instance-file-name"), 0644); err != nil { + t.Fatalf("Failed to write instance file: %v", err) + } + + name, err = tp.GetVMInstanceName() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if name != "instance-file-name" { + t.Errorf("Expected 'instance-file-name', got '%s'", name) + } + + // Clean up + tp.CliConfig.Vm.InstanceName = "" +} + +func TestSaveVMInstanceName(t *testing.T) { + tempDir := t.TempDir() + defer TestChdir(t, tempDir)() + + // Create a mock Trellis structure + tp := &Trellis{ + ConfigDir: ".trellis", + Path: tempDir, + } + + // Create config directory + if err := tp.CreateConfigDir(); err != nil { + t.Fatalf("Failed to create config directory: %v", err) + } + + // Save instance name + instanceName := "test-vm-instance" + if err := tp.SaveVMInstanceName(instanceName); err != nil { + t.Fatalf("Failed to save instance name: %v", err) + } + + // Verify file was created + instancePath := filepath.Join(tp.ConfigPath(), LimaDirName, InstanceFile) + data, err := os.ReadFile(instancePath) + if err != nil { + t.Fatalf("Failed to read instance file: %v", err) + } + + // Verify content + if string(data) != instanceName { + t.Errorf("Expected '%s', got '%s'", instanceName, string(data)) + } + + // Test updating existing file + newInstanceName := "updated-name" + if err := tp.SaveVMInstanceName(newInstanceName); err != nil { + t.Fatalf("Failed to update instance name: %v", err) + } + + // Verify update + data, err = os.ReadFile(instancePath) + if err != nil { + t.Fatalf("Failed to read instance file: %v", err) + } + + if string(data) != newInstanceName { + t.Errorf("Expected '%s', got '%s'", newInstanceName, string(data)) + } +} + +func TestReadInstanceNameFromFile(t *testing.T) { + tempDir := t.TempDir() + defer TestChdir(t, tempDir)() + + // Create a mock Trellis structure + tp := &Trellis{ + ConfigDir: ".trellis", + Path: tempDir, + } + + // Create config directory + if err := tp.CreateConfigDir(); err != nil { + t.Fatalf("Failed to create config directory: %v", err) + } + + // Test reading non-existent file + name, err := tp.readInstanceNameFromFile() + if err == nil { + t.Error("Expected error when reading non-existent file") + } + if name != "" { + t.Errorf("Expected empty string, got '%s'", name) + } + + // Create instance file + limaDir := filepath.Join(tp.ConfigPath(), LimaDirName) + if err := os.MkdirAll(limaDir, 0755); err != nil { + t.Fatalf("Failed to create lima directory: %v", err) + } + instancePath := filepath.Join(limaDir, InstanceFile) + expectedName := "instance-file-name" + if err := os.WriteFile(instancePath, []byte(expectedName), 0644); err != nil { + t.Fatalf("Failed to write instance file: %v", err) + } + + // Test reading existing file + name, err = tp.readInstanceNameFromFile() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if name != expectedName { + t.Errorf("Expected '%s', got '%s'", expectedName, name) + } + + // Test with trailing whitespace + expectedName = "trimmed-name" + if err := os.WriteFile(instancePath, []byte(expectedName+"\n"), 0644); err != nil { + t.Fatalf("Failed to write instance file: %v", err) + } + + name, err = tp.readInstanceNameFromFile() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if name != expectedName { + t.Errorf("Expected '%s', got '%s'", expectedName, name) + } +} From 8eac282c50cd106f7c6e31ca53550c8d7c8511b7 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Sat, 8 Mar 2025 12:26:36 +0700 Subject: [PATCH 3/9] test fixes --- cli_config/cli_config.go | 9 +-- cmd/main.go | 37 ++++++++++++ cmd/provision.go | 6 -- cmd/vm.go | 23 +------- cmd/vm_delete_test.go | 21 +++++-- cmd/vm_start_test.go | 109 +++++++++++++++++++++++------------- trellis/trellis_test.go | 83 --------------------------- trellis/vm_instance_test.go | 2 +- 8 files changed, 130 insertions(+), 160 deletions(-) create mode 100644 cmd/main.go diff --git a/cli_config/cli_config.go b/cli_config/cli_config.go index ace80009..641fe2e1 100644 --- a/cli_config/cli_config.go +++ b/cli_config/cli_config.go @@ -17,10 +17,11 @@ type VmImage struct { } type VmConfig struct { - Manager string `yaml:"manager"` - HostsResolver string `yaml:"hosts_resolver"` - Ubuntu string `yaml:"ubuntu"` - InstanceName string `yaml:"instance_name"` // Custom name for the Lima VM instance + Manager string `yaml:"manager"` + HostsResolver string `yaml:"hosts_resolver"` + Ubuntu string `yaml:"ubuntu"` + InstanceName string `yaml:"instance_name"` // Custom name for the Lima VM instance + Images []VmImage `yaml:"images"` // VM image configuration (as a slice) } type Config struct { diff --git a/cmd/main.go b/cmd/main.go new file mode 100644 index 00000000..1889133b --- /dev/null +++ b/cmd/main.go @@ -0,0 +1,37 @@ +package cmd + +import ( + "fmt" + "runtime" + + "github.com/mitchellh/cli" + "github.com/roots/trellis-cli/pkg/lima" + "github.com/roots/trellis-cli/pkg/vm" + "github.com/roots/trellis-cli/trellis" +) + +// Declare these as variables so they can be overridden in tests +var ( + newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { + // Incorporate the logic from the previous vm.go implementation + switch t.CliConfig.Vm.Manager { + case "auto": + switch runtime.GOOS { + case "darwin": + return lima.NewManager(t, ui) + default: + return nil, fmt.Errorf("No VM managers are supported on %s yet.", runtime.GOOS) + } + case "lima": + return lima.NewManager(t, ui) + case "mock": + return vm.NewMockManager(t, ui) + } + + return nil, fmt.Errorf("VM manager not found") + } + + NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { + return &ProvisionCommand{UI: ui, Trellis: trellis} + } +) diff --git a/cmd/provision.go b/cmd/provision.go index e0f01d7f..e9a921cd 100644 --- a/cmd/provision.go +++ b/cmd/provision.go @@ -12,12 +12,6 @@ import ( "github.com/roots/trellis-cli/trellis" ) -func NewProvisionCommand(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { - c := &ProvisionCommand{UI: ui, Trellis: trellis} - c.init() - return c -} - type ProvisionCommand struct { UI cli.Ui flags *flag.FlagSet diff --git a/cmd/vm.go b/cmd/vm.go index 6a3a17fc..1945c601 100644 --- a/cmd/vm.go +++ b/cmd/vm.go @@ -1,38 +1,17 @@ package cmd import ( - "fmt" "os" "path/filepath" - "runtime" "github.com/mitchellh/cli" - "github.com/roots/trellis-cli/pkg/lima" - "github.com/roots/trellis-cli/pkg/vm" "github.com/roots/trellis-cli/trellis" ) const VagrantInventoryFilePath string = ".vagrant/provisioners/ansible/inventory/vagrant_ansible_inventory" -func newVmManager(trellis *trellis.Trellis, ui cli.Ui) (manager vm.Manager, err error) { - switch trellis.CliConfig.Vm.Manager { - case "auto": - switch runtime.GOOS { - case "darwin": - return lima.NewManager(trellis, ui) - default: - return nil, fmt.Errorf("No VM managers are supported on %s yet.", runtime.GOOS) - } - case "lima": - return lima.NewManager(trellis, ui) - case "mock": - return vm.NewMockManager(trellis, ui) - } - - return nil, fmt.Errorf("VM manager not found") -} - func findDevInventory(trellis *trellis.Trellis, ui cli.Ui) string { + // Use the newVmManager variable from main.go manager, managerErr := newVmManager(trellis, ui) if managerErr == nil { diff --git a/cmd/vm_delete_test.go b/cmd/vm_delete_test.go index 974c7b02..07a58646 100644 --- a/cmd/vm_delete_test.go +++ b/cmd/vm_delete_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/mitchellh/cli" + "github.com/roots/trellis-cli/pkg/vm" "github.com/roots/trellis-cli/trellis" ) @@ -68,9 +69,16 @@ func TestVmDeleteRemovesInstanceFile(t *testing.T) { // Create the lima directory and instance file limaDir := filepath.Join(mockTrellis.ConfigPath(), "lima") - os.MkdirAll(limaDir, 0755) + err := os.MkdirAll(limaDir, 0755) + if err != nil { + t.Fatalf("failed to create lima directory: %v", err) + } + instancePath := filepath.Join(limaDir, "instance") - os.WriteFile(instancePath, []byte("example.com"), 0644) + err = os.WriteFile(instancePath, []byte("example.com"), 0644) + if err != nil { + t.Fatalf("failed to write instance file: %v", err) + } // Verify file exists before test if _, err := os.Stat(instancePath); os.IsNotExist(err) { @@ -82,12 +90,15 @@ func TestVmDeleteRemovesInstanceFile(t *testing.T) { vmDeleteCommand.force = true // Skip confirmation prompt // Replace VM manager with mock - originalNewVmManager := newVmManager mockManager := &MockVmManager{} + + // Save original function and restore after test + originalManagerFunc := newVmManager + defer func() { newVmManager = originalManagerFunc }() + newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { return mockManager, nil } - defer func() { newVmManager = originalNewVmManager }() // Run command code := vmDeleteCommand.Run([]string{}) @@ -98,7 +109,7 @@ func TestVmDeleteRemovesInstanceFile(t *testing.T) { } // Check instance file was removed - _, err := os.Stat(instancePath) + _, err = os.Stat(instancePath) if !os.IsNotExist(err) { t.Error("expected instance file to be deleted") } diff --git a/cmd/vm_start_test.go b/cmd/vm_start_test.go index 087a9d3a..943a2432 100644 --- a/cmd/vm_start_test.go +++ b/cmd/vm_start_test.go @@ -11,6 +11,64 @@ import ( "github.com/roots/trellis-cli/trellis" ) +// Store original functions to restore later +var ( + originalNewVmManager = newVmManager + originalNewProvisionCmd = NewProvisionCommand +) + +// MockVmManager for testing +type MockVmManager struct { + createCalled bool + startCalled bool + siteName string +} + +func (m *MockVmManager) CreateInstance(name string) error { + m.createCalled = true + m.siteName = name + return nil +} + +func (m *MockVmManager) StartInstance(name string) error { + m.startCalled = true + m.siteName = name + // First call returns VmNotFoundErr to trigger creation + if !m.createCalled { + return vm.VmNotFoundErr + } + return nil +} + +func (m *MockVmManager) StopInstance(name string) error { + return nil +} + +func (m *MockVmManager) DeleteInstance(name string) error { + return nil +} + +// Add the missing InventoryPath method required by the vm.Manager interface +func (m *MockVmManager) InventoryPath() string { + return "/mock/inventory/path" +} + +// Update the OpenShell method with the correct signature +func (m *MockVmManager) OpenShell(sshUser string, hostName string, additionalArgs []string) error { + // Mock implementation + return nil +} + +// Mock version of NewProvisionCommand for testing +type MockProvisionCommand struct { + UI cli.Ui + Trellis *trellis.Trellis + Run func(args []string) int +} + +func (c *MockProvisionCommand) Synopsis() string { return "" } +func (c *MockProvisionCommand) Help() string { return "" } + func TestVmStartRunValidations(t *testing.T) { defer trellis.LoadFixtureProject(t)() @@ -58,37 +116,6 @@ func TestVmStartRunValidations(t *testing.T) { } } -// MockVmManager for testing -type MockVmManager struct { - createCalled bool - startCalled bool - siteName string -} - -func (m *MockVmManager) CreateInstance(name string) error { - m.createCalled = true - m.siteName = name - return nil -} - -func (m *MockVmManager) StartInstance(name string) error { - m.startCalled = true - m.siteName = name - // First call returns VmNotFoundErr to trigger creation - if !m.createCalled { - return vm.VmNotFoundErr - } - return nil -} - -func (m *MockVmManager) StopInstance(name string) error { - return nil -} - -func (m *MockVmManager) DeleteInstance(name string) error { - return nil -} - func TestVmStartSavesInstanceName(t *testing.T) { cleanup := trellis.LoadFixtureProject(t) defer cleanup() @@ -102,23 +129,26 @@ func TestVmStartSavesInstanceName(t *testing.T) { vmStartCommand := NewVmStartCommand(ui, mockTrellis) // Replace VM manager with mock - originalNewVmManager := newVmManager mockManager := &MockVmManager{} + + // Save original function and replace with test double + defer func() { newVmManager = originalNewVmManager }() newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { return mockManager, nil } - defer func() { newVmManager = originalNewVmManager }() - // Mock provision command to return success - originalNewProvisionCommand := NewProvisionCommand + // Mock provision command + defer func() { NewProvisionCommand = originalNewProvisionCmd }() NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { - cmd := &ProvisionCommand{UI: ui, Trellis: trellis} - cmd.Run = func(args []string) int { - return 0 + // Create an actual ProvisionCommand instead of trying to cast from MockProvisionCommand + cmd := &ProvisionCommand{ + UI: ui, + Trellis: trellis, } + + // No need for type casting, return the real command return cmd } - defer func() { NewProvisionCommand = originalNewProvisionCommand }() // Run command code := vmStartCommand.Run([]string{}) @@ -139,6 +169,7 @@ func TestVmStartSavesInstanceName(t *testing.T) { data, err := os.ReadFile(instancePath) if err != nil { t.Errorf("expected instance file to exist: %v", err) + return } instanceName := strings.TrimSpace(string(data)) diff --git a/trellis/trellis_test.go b/trellis/trellis_test.go index 2dd8f96c..b51a62e7 100644 --- a/trellis/trellis_test.go +++ b/trellis/trellis_test.go @@ -444,86 +444,3 @@ ask_vault_pass: true t.Errorf("expected load project to load project CLI config file") } } - -func TestGetVMInstanceName(t *testing.T) { - defer LoadFixtureProject(t)() - - tp := NewTrellis() - err := tp.LoadProject() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - // Test case 1: No instance file and no config setting - should use first site - instanceName, err := tp.GetVMInstanceName() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - expectedName := tp.SiteNamesFromEnvironment("development")[0] - if instanceName != expectedName { - t.Errorf("expected instance name %q, got %q", expectedName, instanceName) - } - - // Test case 2: With config setting - should use config value - tp.CliConfig.Vm.InstanceName = "configured-name" - instanceName, err = tp.GetVMInstanceName() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if instanceName != "configured-name" { - t.Errorf("expected instance name %q, got %q", "configured-name", instanceName) - } - - // Test case 3: With instance file - should use file value (highest priority) - // Create the instance file - limaDir := filepath.Join(tp.ConfigPath(), "lima") - if err := os.MkdirAll(limaDir, 0755); err != nil { - t.Fatalf("failed to create lima directory: %v", err) - } - instancePath := filepath.Join(limaDir, "instance") - if err := os.WriteFile(instancePath, []byte("file-instance-name"), 0644); err != nil { - t.Fatalf("failed to write instance file: %v", err) - } - - instanceName, err = tp.GetVMInstanceName() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if instanceName != "file-instance-name" { - t.Errorf("expected instance name %q, got %q", "file-instance-name", instanceName) - } - - // Clean up - os.Remove(instancePath) - tp.CliConfig.Vm.InstanceName = "" -} - -func TestSaveVMInstanceName(t *testing.T) { - defer LoadFixtureProject(t)() - - tp := NewTrellis() - err := tp.LoadProject() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - // Test saving the instance name - err = tp.SaveVMInstanceName("test-instance") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - // Verify file was created and contains correct content - instancePath := filepath.Join(tp.ConfigPath(), "lima", "instance") - data, err := os.ReadFile(instancePath) - if err != nil { - t.Fatalf("failed to read instance file: %v", err) - } - - if string(data) != "test-instance" { - t.Errorf("expected instance file to contain %q, got %q", "test-instance", string(data)) - } - - // Clean up - os.Remove(instancePath) -} diff --git a/trellis/vm_instance_test.go b/trellis/vm_instance_test.go index 25039a2b..4ee51fb1 100644 --- a/trellis/vm_instance_test.go +++ b/trellis/vm_instance_test.go @@ -17,7 +17,7 @@ func TestGetVMInstanceName(t *testing.T) { Environments: map[string]*Config{ "development": { WordPressSites: map[string]*Site{ - "example.com": {}, + "example.com": {}, "another-site.com": {}, }, }, From 3d49591601cbee998e452b6ba6b73419b1f4db24 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Sun, 9 Mar 2025 15:04:33 +0700 Subject: [PATCH 4/9] use the proper instance name resolution method --- cmd/vm_start.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/vm_start.go b/cmd/vm_start.go index 9fb1b7d6..0f9a1e3a 100644 --- a/cmd/vm_start.go +++ b/cmd/vm_start.go @@ -49,9 +49,15 @@ func (c *VmStartCommand) Run(args []string) int { return 1 } - siteName, _, err := c.Trellis.MainSiteFromEnvironment("development") + // CHANGE: Use GetVMInstanceName instead of MainSiteFromEnvironment + siteName, err := c.Trellis.GetVMInstanceName() if err != nil { - c.UI.Error("Error: could not automatically set VM name: " + err.Error()) + c.UI.Error("Error: could not get VM name: " + err.Error()) + return 1 + } + + if siteName == "" { + c.UI.Error("Error: could not automatically set VM name. No site found in development environment.") return 1 } @@ -67,7 +73,7 @@ func (c *VmStartCommand) Run(args []string) int { return 0 } - if (!errors.Is(err, vm.VmNotFoundErr)) { + if !errors.Is(err, vm.VmNotFoundErr) { c.UI.Error("Error starting VM.") c.UI.Error(err.Error()) return 1 From 5d9e5f12f31a6bebd97679e4abe69126e9b109ad Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Sun, 9 Mar 2025 15:08:03 +0700 Subject: [PATCH 5/9] vm start test update --- cmd/vm_start_test.go | 106 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 93 insertions(+), 13 deletions(-) diff --git a/cmd/vm_start_test.go b/cmd/vm_start_test.go index 943a2432..ea58c21d 100644 --- a/cmd/vm_start_test.go +++ b/cmd/vm_start_test.go @@ -13,8 +13,8 @@ import ( // Store original functions to restore later var ( - originalNewVmManager = newVmManager - originalNewProvisionCmd = NewProvisionCommand + originalNewVmManager = newVmManager + originalNewProvisionCmd = NewProvisionCommand ) // MockVmManager for testing @@ -69,6 +69,17 @@ type MockProvisionCommand struct { func (c *MockProvisionCommand) Synopsis() string { return "" } func (c *MockProvisionCommand) Help() string { return "" } +// Add a MockTrellis type that implements the GetVMInstanceName method +type MockTrellisWithVMName struct { + *trellis.Trellis + instanceName string +} + +// Override the GetVMInstanceName method for testing +func (m *MockTrellisWithVMName) GetVMInstanceName() (string, error) { + return m.instanceName, nil +} + func TestVmStartRunValidations(t *testing.T) { defer trellis.LoadFixtureProject(t)() @@ -119,40 +130,40 @@ func TestVmStartRunValidations(t *testing.T) { func TestVmStartSavesInstanceName(t *testing.T) { cleanup := trellis.LoadFixtureProject(t) defer cleanup() - + // Setup test environment ui := cli.NewMockUi() mockTrellis := trellis.NewTrellis() mockTrellis.LoadProject() - + // Create command vmStartCommand := NewVmStartCommand(ui, mockTrellis) - + // Replace VM manager with mock mockManager := &MockVmManager{} - + // Save original function and replace with test double defer func() { newVmManager = originalNewVmManager }() newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { return mockManager, nil } - + // Mock provision command defer func() { NewProvisionCommand = originalNewProvisionCmd }() NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { // Create an actual ProvisionCommand instead of trying to cast from MockProvisionCommand cmd := &ProvisionCommand{ - UI: ui, + UI: ui, Trellis: trellis, } - + // No need for type casting, return the real command return cmd } - + // Run command code := vmStartCommand.Run([]string{}) - + // Check VM was created and started if code != 0 { t.Errorf("expected exit code 0, got %d", code) @@ -163,7 +174,7 @@ func TestVmStartSavesInstanceName(t *testing.T) { if !mockManager.startCalled { t.Error("expected StartInstance to be called") } - + // Check instance file was created instancePath := filepath.Join(mockTrellis.ConfigPath(), "lima", "instance") data, err := os.ReadFile(instancePath) @@ -171,9 +182,78 @@ func TestVmStartSavesInstanceName(t *testing.T) { t.Errorf("expected instance file to exist: %v", err) return } - + instanceName := strings.TrimSpace(string(data)) if instanceName != mockManager.siteName { t.Errorf("expected instance name %q, got %q", mockManager.siteName, instanceName) } } + +// Add this test to verify the VM name resolution +func TestVmStartUsesGetVMInstanceName(t *testing.T) { + cleanup := trellis.LoadFixtureProject(t) + defer cleanup() + + // Setup test environment with our custom mock + ui := cli.NewMockUi() + mockTrellis := trellis.NewTrellis() + mockTrellis.LoadProject() + + // Create a custom mock Trellis that returns a specific instance name + mockTrellisWithVMName := &MockTrellisWithVMName{ + Trellis: mockTrellis, + instanceName: "custom-instance-name", + } + + // Create command with our custom mock + vmStartCommand := NewVmStartCommand(ui, mockTrellisWithVMName) + + // Replace VM manager with mock + mockManager := &MockVmManager{} + + // Save original function and replace with test double + defer func() { newVmManager = originalNewVmManager }() + newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { + return mockManager, nil + } + + // Mock provision command + defer func() { NewProvisionCommand = originalNewProvisionCmd }() + NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { + cmd := &ProvisionCommand{ + UI: ui, + Trellis: trellis, + } + return cmd + } + + // Run command + code := vmStartCommand.Run([]string{}) + + // Check VM was created and started with the correct instance name + if code != 0 { + t.Errorf("expected exit code 0, got %d", code) + } + if !mockManager.createCalled { + t.Error("expected CreateInstance to be called") + } + if !mockManager.startCalled { + t.Error("expected StartInstance to be called") + } + if mockManager.siteName != "custom-instance-name" { + t.Errorf("expected site name to be 'custom-instance-name', got %s", mockManager.siteName) + } + + // Check instance file was created with correct name + instancePath := filepath.Join(mockTrellis.ConfigPath(), "lima", "instance") + data, err := os.ReadFile(instancePath) + if err != nil { + t.Errorf("expected instance file to exist: %v", err) + return + } + + instanceName := strings.TrimSpace(string(data)) + if instanceName != "custom-instance-name" { + t.Errorf("expected instance name %q, got %q", "custom-instance-name", instanceName) + } +} From e17565149b030a54d8301584057788d82fa67c33 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Mon, 10 Mar 2025 08:11:34 +0700 Subject: [PATCH 6/9] main.go documentation --- cmd/main.go | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 1889133b..f2cfae91 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,3 +1,4 @@ +// Package cmd contains all command-line interface commands for Trellis CLI. package cmd import ( @@ -10,27 +11,38 @@ import ( "github.com/roots/trellis-cli/trellis" ) +// This file provides centralized declarations of functions as variables +// to enable mocking in tests and prevent duplicate implementations. +// Previously, these functions were defined in multiple places, causing +// "redeclared in this block" errors during compilation. + // Declare these as variables so they can be overridden in tests var ( + // newVmManager creates a VM manager instance based on configuration. + // Using a function variable allows for mocking in tests and centralizes VM manager creation logic. + // This approach eliminates duplicate implementations that previously existed across files. newVmManager = func(t *trellis.Trellis, ui cli.Ui) (vm.Manager, error) { - // Incorporate the logic from the previous vm.go implementation - switch t.CliConfig.Vm.Manager { - case "auto": - switch runtime.GOOS { - case "darwin": - return lima.NewManager(t, ui) - default: - return nil, fmt.Errorf("No VM managers are supported on %s yet.", runtime.GOOS) - } - case "lima": + // Select appropriate VM manager based on configuration + switch t.CliConfig.Vm.Manager { + case "auto": + switch runtime.GOOS { + case "darwin": return lima.NewManager(t, ui) - case "mock": - return vm.NewMockManager(t, ui) + default: + return nil, fmt.Errorf("no VM managers are supported on %s yet", runtime.GOOS) } - - return nil, fmt.Errorf("VM manager not found") + case "lima": + return lima.NewManager(t, ui) + case "mock": + return vm.NewMockManager(t, ui) } - + + return nil, fmt.Errorf("vm manager not found") + } + + // NewProvisionCommand creates a new ProvisionCommand instance. + // This was moved here from provision.go to follow the same pattern + // of using function variables for testability. NewProvisionCommand = func(ui cli.Ui, trellis *trellis.Trellis) *ProvisionCommand { return &ProvisionCommand{UI: ui, Trellis: trellis} } From 56fbe3ea6a78aa1022141a0ca81dc584f6f3cee0 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Mon, 10 Mar 2025 08:37:02 +0700 Subject: [PATCH 7/9] vm start test patch --- cmd/vm_start_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/vm_start_test.go b/cmd/vm_start_test.go index ea58c21d..3480fada 100644 --- a/cmd/vm_start_test.go +++ b/cmd/vm_start_test.go @@ -206,7 +206,7 @@ func TestVmStartUsesGetVMInstanceName(t *testing.T) { } // Create command with our custom mock - vmStartCommand := NewVmStartCommand(ui, mockTrellisWithVMName) + vmStartCommand := NewVmStartCommand(ui, mockTrellisWithVMName.Trellis) // Replace VM manager with mock mockManager := &MockVmManager{} From c31fe7f54cd278564d40e33cd2538c7bb128142a Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Mon, 10 Mar 2025 09:23:10 +0700 Subject: [PATCH 8/9] .DS_Store addition to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 943061b9..e9f949ab 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ trellis-cli dist tmp +.DS_Store From de02b9b20cab2219f8925ae3db644e327978af96 Mon Sep 17 00:00:00 2001 From: Jasper Frumau Date: Tue, 11 Mar 2025 09:39:58 +0700 Subject: [PATCH 9/9] handle case where a VM already exists but no instance file available --- trellis/vm_instance.go | 51 +++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/trellis/vm_instance.go b/trellis/vm_instance.go index bb6f9663..b5b83ddf 100644 --- a/trellis/vm_instance.go +++ b/trellis/vm_instance.go @@ -2,19 +2,21 @@ package trellis import ( "os" + "os/exec" "path/filepath" "strings" ) const ( - LimaDirName = "lima" - InstanceFile = "instance" + LimaDirName = "lima" + InstanceFile = "instance" ) // GetVMInstanceName returns the VM instance name based on the following priority: // 1. Instance file in .trellis/lima/instance // 2. CliConfig instance_name setting -// 3. First site in development environment's wordpress_sites.yml +// 3. Check for existing VMs matching any site name +// 4. First site in development environment's wordpress_sites.yml func (t *Trellis) GetVMInstanceName() (string, error) { // 1. Check for instance file instanceName, err := t.readInstanceNameFromFile() @@ -27,30 +29,57 @@ func (t *Trellis) GetVMInstanceName() (string, error) { return t.CliConfig.Vm.InstanceName, nil } - // 3. Simply use the first site in the development environment + // 3. NEW: Check for existing VMs matching site names + // Get all site names from the development environment + siteNames := t.SiteNamesFromEnvironment("development") + + // Check if any of these site names already exists as a VM + for _, siteName := range siteNames { + vmExists, _ := checkVMExists(siteName) + if vmExists { + // Found existing VM - save this for future use + t.SaveVMInstanceName(siteName) + return siteName, nil + } + } + + // 4. Simply use the first site in the development environment config := t.Environments["development"] if config == nil || len(config.WordPressSites) == 0 { return "", nil } - + // Get the first site name alphabetically (which is the default behavior) - siteNames := t.SiteNamesFromEnvironment("development") if len(siteNames) > 0 { return siteNames[0], nil } - + return "", nil } +// checkVMExists checks if a VM with the given name already exists +func checkVMExists(name string) (bool, error) { + cmd := exec.Command("limactl", "list", "--json") + output, err := cmd.Output() + + if err != nil { + return false, err + } + + // Simple string check rather than parsing JSON + return strings.Contains(string(output), `"name":"`+name+`"`) || + strings.Contains(string(output), `"name": "`+name+`"`), nil +} + // SaveVMInstanceName writes the VM instance name to the instance file func (t *Trellis) SaveVMInstanceName(instanceName string) error { limaDir := filepath.Join(t.ConfigPath(), LimaDirName) - + // Create the lima directory if it doesn't exist if err := os.MkdirAll(limaDir, 0755); err != nil && !os.IsExist(err) { return err } - + instancePath := filepath.Join(limaDir, InstanceFile) return os.WriteFile(instancePath, []byte(instanceName), 0644) } @@ -58,11 +87,11 @@ func (t *Trellis) SaveVMInstanceName(instanceName string) error { // readInstanceNameFromFile reads the VM instance name from the instance file func (t *Trellis) readInstanceNameFromFile() (string, error) { instancePath := filepath.Join(t.ConfigPath(), LimaDirName, InstanceFile) - + data, err := os.ReadFile(instancePath) if err != nil { return "", err } - + return strings.TrimSpace(string(data)), nil }