Skip to content

A few kt VM tweaks#66

Open
bmastbergen wants to merge 3 commits intomainlinefrom
{bmastbergen}_kt-tweaks
Open

A few kt VM tweaks#66
bmastbergen wants to merge 3 commits intomainlinefrom
{bmastbergen}_kt-tweaks

Conversation

@bmastbergen
Copy link
Copy Markdown
Collaborator

These came to light as I've been working on getting a kt based content release workflow running on jenkins.

  • Using the 'cloud-init status --wait' ensures all the dependency installs are actually done before continuing.
  • In the case of a content release the vm isn't building anything, just running kselftests, and using up all those cpus and ram seems like overkill
  • This also removes ,vcpu.cpuset=0-11,vcpu.placement=static from the vcpu config as that seems very host specific for a general use tool like this.

Copilot AI review requested due to automatic review settings May 7, 2026 13:29
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts VM provisioning and startup/test sequencing to be more configurable and better aligned with cloud-init completion, supporting Jenkins-driven workflows.

Changes:

  • Add vcpus/memory parameters through the VM creation path and expose them via kt vm --vcpus/--memory.
  • Remove host-specific vCPU pinning/placement from the virt-install invocation.
  • Replace fixed “deps install” sleeps with cloud-init status --wait before running tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
kt/ktlib/vm.py Plumbs vCPU/memory sizing parameters into VM create/spin-up flow.
kt/ktlib/virt.py Updates virt-install args to use configurable vCPU/memory and drops cpuset/placement.
kt/commands/vm/impl.py Uses cloud-init completion check before running --test.
kt/commands/vm/command.py Adds CLI flags for vCPU/memory sizing.
kt/commands/content_release/impl.py Switches content release VM dependency wait from sleep to cloud-init wait.
Comments suppressed due to low confidence (1)

kt/ktlib/vm.py:224

  • vcpus/memory are only applied when the VM is created. If the VM already exists (even stopped), spin_up() ignores the requested sizing and just starts the domain, which can make the new CLI flags appear to “do nothing”. Consider either documenting/enforcing that changing sizing requires override=True, or updating the existing domain’s resources before starting it.
    def spin_up(self, config: Config, vcpus: int = 12, memory: int = 32768) -> VmInstance:
        if not VirtHelper.exists(vm_name=self.name):
            logging.info(f"VM {self.name} does not exist, creating from scratch...")

            self._create_image(config=config, vcpus=vcpus, memory=memory)
            return VmInstance(name=self.name, kernel_workspace=self.kernel_workspace)

        logging.info(f"Vm {self.name} already exists")

        if VirtHelper.is_running(vm_name=self.name):
            logging.info(f"Vm {self.name} is running, nothing to do")
            return VmInstance(name=self.name, kernel_workspace=self.kernel_workspace)

        logging.info(f"Vm {self.name} is not running, starting it")
        VmCommand.start(vm_name=self.name)
        time.sleep(Constants.VM_STARTUP_WAIT_SECONDS)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kt/commands/vm/impl.py
logging.info("Waiting for the deps to be installed")
time.sleep(Constants.VM_DEPS_INSTALL_WAIT_SECONDS)
logging.info("Waiting for cloud-init to finish...")
SshCommand.run(domain=vm_instance.domain, command=["sudo cloud-init status --wait || true"])
# Wait for dependencies to be installed if VM was just created
logging.info("Waiting for VM dependencies to be installed...")
time.sleep(Constants.VM_DEPS_INSTALL_WAIT_SECONDS)
SshCommand.run(domain=vm_instance.domain, command=["sudo cloud-init status --wait || true"])
Comment on lines 308 to 309
# Wait for dependencies to be installed if VM was just created
logging.info("Waiting for VM dependencies to be installed...")
Comment on lines 304 to 307

# Setup and spin up the VM (reuses common code from vm command)
vm_instance = Vm.setup_and_spinup(kernel_workspace_name=kernel_workspace)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't landed on the ideal vcpu/memory setup for the content release yet. This PR gets the hooks in place. A future PR will set these values when they've been nailed down.

Comment thread kt/commands/vm/command.py
Comment on lines +60 to +61
@click.option("--vcpus", type=int, default=12, help="Number of virtual CPUs (default: 12)")
@click.option("--memory", type=int, default=32768, help="Memory in MiB (default: 32768)")
roxanan1996
roxanan1996 previously approved these changes May 7, 2026
Copy link
Copy Markdown
Contributor

@roxanan1996 roxanan1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement. The sleep was indeed very annoying.
Can we also remove VM_DEPS_INSTALL_WAIT_SECONDS from util.py? Since it is not used anymore?

@bmastbergen bmastbergen force-pushed the {bmastbergen}_kt-tweaks branch from aa5f4e1 to 46077fd Compare May 7, 2026 13:52
Copilot AI review requested due to automatic review settings May 7, 2026 13:55
@bmastbergen bmastbergen force-pushed the {bmastbergen}_kt-tweaks branch from 46077fd to 804e598 Compare May 7, 2026 13:55
@bmastbergen
Copy link
Copy Markdown
Collaborator Author

Great improvement. The sleep was indeed very annoying. Can we also remove VM_DEPS_INSTALL_WAIT_SECONDS from util.py? Since it is not used anymore?

Good call. Removed VM_DEPS_INSTALL_WAIT_SECONDS

Replace fixed sleeps with 'sudo cloud-init status --wait' to wait
for cloud-init to finish, rather than hoping a hardcoded timeout
is long enough.
Drop the old cpuset/static placement that pinned VMs to specific
cores. Defaults to 12 if not specified.
Defaults to 32768 MiB if not specified.
@bmastbergen bmastbergen force-pushed the {bmastbergen}_kt-tweaks branch from 804e598 to 95afb97 Compare May 7, 2026 13:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread kt/ktlib/vm.py
Comment on lines +209 to 214
def spin_up(self, config: Config, vcpus: int = 12, memory: int = 32768) -> VmInstance:
if not VirtHelper.exists(vm_name=self.name):
logging.info(f"VM {self.name} does not exist, creating from scratch...")

self._create_image(config=config)
self._create_image(config=config, vcpus=vcpus, memory=memory)
return VmInstance(name=self.name, kernel_workspace=self.kernel_workspace)
Comment thread kt/commands/vm/command.py
Comment on lines +60 to +61
@click.option("--vcpus", type=int, default=12, help="Number of virtual CPUs (default: 12)")
@click.option("--memory", type=int, default=32768, help="Memory in MiB (default: 32768)")
@PlaidCat
Copy link
Copy Markdown
Collaborator

PlaidCat commented May 7, 2026

This also removes ,vcpu.cpuset=0-11,vcpu.placement=static from the vcpu config as that seems very host specific for a general use tool like this.

This is very much my alderlake processor in my laptop were core 0-11 are performance cores and the other 8 are efficiency cores

@bmastbergen bmastbergen requested a review from roxanan1996 May 7, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants