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 #96: During a resource adjustment the VM is not shut down using ACPI #124

Closed
wants to merge 3 commits into from

Conversation

PacoDw
Copy link
Contributor

@PacoDw PacoDw commented May 13, 2020

Added:

  • The new power_state_mechanism_config attribute was added, and we can use it as a following:
resource "nutanix_virtual_machine" "vm1" {
  name         = "my-vm"
  cluster_uuid = local.cluster1

  num_vcpus_per_socket = 1
  num_sockets          = 1
  memory_size_mib      = 186

  power_state_mechanism_config {
    guest_transition_config {
      should_fail_on_script_failure = true
      enable_script_exec            = true
    }
    mechanism = "ACPI"
  }

}

Also, the following Terraform configuration can work with the old attributes that represented the above new configuration:

resource "nutanix_virtual_machine" "vm1" {
  name         = "my-vm"
  cluster_uuid = local.cluster1

  num_vcpus_per_socket = 1
  num_sockets          = 1
  memory_size_mib      = 186

  should_fail_on_script_failure = true
  enable_script_exec            = true
  power_state_mechanism         = "ACPI"
}

But now they are deprecated, and we don't recomend using them due to inconsistency and bugs that were fixed with this new change.

Let me know if you have another comment or concern, thank you so much

closes #96

@PacoDw PacoDw requested review from marinsalinas and coderGo93 May 13, 2020 19:49
@ghost ghost added the size/XXL label May 13, 2020
@PacoDw PacoDw changed the title Fix #96 Fix #96: During a resource adjustment the VM is not shut down using ACPI May 13, 2020
@marinsalinas
Copy link
Contributor

marinsalinas commented May 19, 2020

@PacoDw could you please fix the conflicts?

PacoDw added 3 commits June 19, 2020 10:56
…ad of:

- should_fail_on_script_failure
- enable_script_exec
- power_state_mechanism
this change represents a better coherency with the API so the above attributes were deprecated, they can be used yet but we recommend
to use the new object power_state_mechanism_config
@@ -257,7 +257,7 @@ func TestAccNutanixVirtualMachine_PowerStateMechanism(t *testing.T) {
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"disk_list"},
ImportStateVerifyIgnore: []string{"power_state_mechanism_config", "power_state_mechanism"},
Copy link
Contributor

Choose a reason for hiding this comment

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

?Can't we add those values in the read function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes let change and check them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are in the read function but they depend on a config to be set and they have the same attribute but with different structure, the power_state_mechanism will be deprecated, so I put them in ImportStateVerifyIgnore array due to a state error

@PacoDw PacoDw requested a review from marinsalinas June 19, 2020 17:58
Copy link
Contributor

@marinsalinas marinsalinas left a comment

Choose a reason for hiding this comment

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

Just fix the conflicts otherwise LGTM

@marinsalinas
Copy link
Contributor

@yannickstruyf3 can you confirm if this is still needed since #96 is fixed with the timeout?

@siddharth-nutanix
Copy link
Collaborator

As the corresponding issue is closed, we are closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

During a resource adjustment the VM is not shut down using ACPI
3 participants