Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent vm naming #524

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

jasperf
Copy link

@jasperf jasperf commented Mar 8, 2025

This pull request introduces several significant changes to the VM management and configuration handling within the Trellis CLI. The most notable updates include the addition of a new instance name management system, the centralization of VM manager creation logic, and the enhancement of testability for VM-related commands.

Key changes:

VM Instance Management:

  • Added InstanceName field to VmConfig struct in cli_config/cli_config.go to allow custom naming of VM instances.
  • Implemented GetVMInstanceName and SaveVMInstanceName methods in trellis/vm_instance.go to manage VM instance names with a priority system.
  • Updated VmStartCommand to use GetVMInstanceName for setting the VM name and saving it for future reference. [1] [2]

Centralization and Testability:

  • Centralized VM manager creation logic in cmd/main.go to avoid duplicate implementations and enable easier mocking in tests.
  • Removed redundant newVmManager function from cmd/vm.go and updated references to use the centralized version.

Testing Enhancements:

  • Added tests for the new instance name management methods in trellis/vm_instance_test.go.
  • Enhanced VmDeleteCommand and VmStartCommand tests to verify instance file handling and VM name resolution. [1] [2]

These changes improve the flexibility, maintainability, and testability of the Trellis CLI's VM management functionality.

VM Instance Name Resolution

The VM creation and start processes now follow a structured approach to determine which instance name to use. This is a significant improvement over the previous implementation which always defaulted to the first site alphabetically.

How VM Name Resolution Works Now

Priority Order for VM Name Resolution
The new code in trellis/vm_instance.go implements a priority-based approach:

  1. First, check for a saved instance name in .trellis/lima/instance file
  2. If not found, check for a configured instance name in CLI config
  3. Only if both above methods fail, fall back to using the first WordPress site alphabetically

VM Start Process

When running trellis vm start:

  1. The command calls GetVMInstanceName() instead of directly using MainSiteFromEnvironment()
  2. GetVMInstanceName() first tries to read from the instance file
  3. If that file exists and contains a name (like imagewize.com), it will use that name
  4. The VM start command will then use that name to start the VM
    This ensures that once you've created a VM for a specific site, subsequent vm start commands will consistently use the same VM, rather than creating or starting a different one based on alphabetical order.

VM Creation Process

When creating a new VM:

  1. If no VM exists, the code follows the same name resolution process
  2. After creating the VM, it saves the instance name to .trellis/lima/instance file using SaveVMInstanceName()
  3. This file persists the association between your project and a specific VM name

Benefits
This solves your specific issue where vm start was always using "blocks.imagewize.com" instead of "imagewize.com":

  1. If you've previously created a VM with imagewize.com, that name is saved in the instance file
  2. Future vm start commands will read this file first rather than defaulting to alphabetical order
  3. Even if blocks.imagewize.com comes first alphabetically, the saved instance name will be preferred

This provides consistency and predictability when managing VMs across multiple sites in a Trellis project.

@jasperf jasperf closed this Mar 11, 2025
@jasperf jasperf reopened this Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant