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

fix(vm): cpu.architecture showed as new attribute at re-apply after creation #1524

Merged
merged 1 commit into from
Sep 8, 2024

Conversation

bpg
Copy link
Owner

@bpg bpg commented Sep 8, 2024

Contributor's Note

  • I have added / updated documentation in /docs for any user-facing features or additions.
  • I have added / updated acceptance tests in /fwprovider/tests for any new or updated resources / data sources.
  • I have ran make example to verify that the change works as expected.

Proof of Work

With this template

resource "proxmox_virtual_environment_vm" "test_cdrom" {
  node_name = "pve"
  vm_id     = 999
  started   = false
  name      = "test-cpu-arch"

  cpu {
    cores = 2
    sockets = 1
  }
}

Before the fix:

❯ tofu apply -auto-approve

OpenTofu used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

OpenTofu will perform the following actions:

  # proxmox_virtual_environment_vm.test_cdrom will be created
  + resource "proxmox_virtual_environment_vm" "test_cdrom" {
      + acpi                    = true
      + bios                    = "seabios"
      + id                      = (known after apply)
      + ipv4_addresses          = (known after apply)
      + ipv6_addresses          = (known after apply)
      + keyboard_layout         = "en-us"
      + mac_addresses           = (known after apply)
      + migrate                 = false
      + name                    = "test-cpu-arch"
      + network_interface_names = (known after apply)
      + node_name               = "pve"
      + on_boot                 = true
      + protection              = false
      + reboot                  = false
      + scsi_hardware           = "virtio-scsi-pci"
      + started                 = false
      + stop_on_destroy         = false
      + tablet_device           = true
      + template                = false
      + timeout_clone           = 1800
      + timeout_create          = 1800
      + timeout_migrate         = 1800
      + timeout_move_disk       = 1800
      + timeout_reboot          = 1800
      + timeout_shutdown_vm     = 1800
      + timeout_start_vm        = 1800
      + timeout_stop_vm         = 300
      + vm_id                   = 999

      + cpu {
          + architecture = "x86_64"
          + cores        = 2
          + hotplugged   = 0
          + limit        = 0
          + numa         = false
          + sockets      = 1
          + type         = "qemu64"
          + units        = 1024
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
proxmox_virtual_environment_vm.test_cdrom: Creating...
proxmox_virtual_environment_vm.test_cdrom: Creation complete after 1s [id=999]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
❯ tofu apply -auto-approve
proxmox_virtual_environment_vm.test_cdrom: Refreshing state... [id=999]

OpenTofu used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

OpenTofu will perform the following actions:

  # proxmox_virtual_environment_vm.test_cdrom will be updated in-place
  ~ resource "proxmox_virtual_environment_vm" "test_cdrom" {
        id                      = "999"
        name                    = "test-cpu-arch"
        tags                    = []
        # (26 unchanged attributes hidden)

      ~ cpu {
          + architecture = "x86_64"
            # (8 unchanged attributes hidden)
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.
proxmox_virtual_environment_vm.test_cdrom: Modifying... [id=999]
proxmox_virtual_environment_vm.test_cdrom: Modifications complete after 0s [id=999]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

After the fix:

❯ tofu apply -auto-approve

OpenTofu used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

OpenTofu will perform the following actions:

  # proxmox_virtual_environment_vm.test_cdrom will be created
  + resource "proxmox_virtual_environment_vm" "test_cdrom" {
      + acpi                    = true
      + bios                    = "seabios"
      + id                      = (known after apply)
      + ipv4_addresses          = (known after apply)
      + ipv6_addresses          = (known after apply)
      + keyboard_layout         = "en-us"
      + mac_addresses           = (known after apply)
      + migrate                 = false
      + name                    = "test-cpu-arch"
      + network_interface_names = (known after apply)
      + node_name               = "pve"
      + on_boot                 = true
      + protection              = false
      + reboot                  = false
      + scsi_hardware           = "virtio-scsi-pci"
      + started                 = false
      + stop_on_destroy         = false
      + tablet_device           = true
      + template                = false
      + timeout_clone           = 1800
      + timeout_create          = 1800
      + timeout_migrate         = 1800
      + timeout_move_disk       = 1800
      + timeout_reboot          = 1800
      + timeout_shutdown_vm     = 1800
      + timeout_start_vm        = 1800
      + timeout_stop_vm         = 300
      + vm_id                   = 999

      + cpu {
          + cores      = 2
          + hotplugged = 0
          + limit      = 0
          + numa       = false
          + sockets    = 1
          + type       = "qemu64"
          + units      = 1024
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
proxmox_virtual_environment_vm.test_cdrom: Creating...
proxmox_virtual_environment_vm.test_cdrom: Creation complete after 1s [id=999]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
❯ tofu apply -auto-approve
proxmox_virtual_environment_vm.test_cdrom: Refreshing state... [id=999]

No changes. Your infrastructure matches the configuration.

OpenTofu has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates: #1494 (comment)

@bpg bpg force-pushed the fix-default-cpu-architecture branch from fce13cd to ffdaefc Compare September 8, 2024 23:00
@pull-request-size pull-request-size bot added size/XS and removed size/S labels Sep 8, 2024
@bpg bpg changed the title Fix default cpu architecture fix(vm): cpu.architecture showed as new attribute at re-apply after creation Sep 8, 2024
… creation

Fix for another use case of mismanaged default value. This one was a bit trickier to spot as it triggered only when provider is authenticated using root@pam, as architecture change is allowed only for root.

Removing default value altogether, as the PVE API default for this attribute is an empty string.

VM2 resource will have no such issue, related: #1310, #1311

Signed-off-by: Pavel Boldyrev <[email protected]>
@bpg bpg force-pushed the fix-default-cpu-architecture branch from ffdaefc to c587b3d Compare September 8, 2024 23:21
@pull-request-size pull-request-size bot added size/S and removed size/XS labels Sep 8, 2024
@bpg bpg added the autoapprove label Sep 8, 2024
Copy link

@mergify mergify bot left a comment

Choose a reason for hiding this comment

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

🤖 LGTM! beep boop

@bpg bpg changed the title fix(vm): cpu.architecture showed as new attribute at re-apply after creation fix(vm): cpu.architecture showed as new attribute at re-apply after creation Sep 8, 2024
@bpg bpg merged commit c20d79d into main Sep 8, 2024
9 checks passed
@bpg bpg deleted the fix-default-cpu-architecture branch September 8, 2024 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant