Skip to content

Fix p4tenant remove to support deleting individual VMs#4

Merged
alessandrocornacchia merged 1 commit into
masterfrom
fix/issue-1
Jan 29, 2026
Merged

Fix p4tenant remove to support deleting individual VMs#4
alessandrocornacchia merged 1 commit into
masterfrom
fix/issue-1

Conversation

@alessandrocornacchia

@alessandrocornacchia alessandrocornacchia commented Jan 29, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #1 - The p4tenant remove command now correctly supports deleting individual VMs instead of removing all VMs or the wrong VM.

Problem

When trying to remove restvm-mspina-02, the tool would:

  • Remove the entire user mspina from restart_users
  • Only remove restvm-mspina-01 from configuration files
  • The CLI's VM selection feature was ignored

Solution

  1. Updated TenantManager.remove_tenant() to accept an optional vm_names parameter

    • Defaults to removing all VMs if not specified (backward compatible)
    • Only removes user from restart_users when ALL their VMs are being deleted
    • Processes each VM in the provided list
  2. Updated CLI remove command to pass selected VMs to the method

    • The VM selection prompt now actually controls which VMs are removed
    • Admin inventories are updated for each selected VM

Changes to apply

The "Changes to apply" UI now correctly shows:

  • Individual VMs being removed (not just VM-01)
  • Whether the user will be removed from restart_users (only if all VMs deleted)
  • Separate entries for each selected VM in admin inventories

Test plan

  • Test removing a single VM from a user with multiple VMs (e.g., remove restvm-mspina-02 while keeping -01)
  • Verify user remains in restart_users when some VMs remain
  • Test removing all VMs for a user - verify user is removed from restart_users
  • Test removing the only VM for a user - verify complete cleanup
  • Verify "Changes to apply" displays correct VMs being removed

🤖 Generated with Claude Code

The p4tenant remove command incorrectly removed all VMs or the wrong VM when
trying to remove a specific VM (e.g., restvm-mspina-02).

Changes:
- Updated TenantManager.remove_tenant() to accept optional vm_names parameter
- The method now removes only the specified VMs instead of defaulting to VM-01
- Only removes user from restart_users when ALL their VMs are being deleted
- Updated CLI to pass selected VMs to remove_tenant()
- Fixed admin inventory updates to process each selected VM

This allows users to selectively delete individual VMs while keeping others,
matching the behavior of the "add vm" command.

Fixes #1

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 29, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@alessandrocornacchia alessandrocornacchia merged commit 5efa7ba into master Jan 29, 2026
1 check passed
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.

p4-tenant remove

1 participant