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

Disk: bool is required #702

Closed
ickebinberliner opened this issue Feb 23, 2023 · 19 comments · Fixed by #923
Closed

Disk: bool is required #702

ickebinberliner opened this issue Feb 23, 2023 · 19 comments · Fixed by #923

Comments

@ickebinberliner
Copy link

I upgrade from Version v2.9.11 -> v2.9.13 and now get an error

Error: bool is required
disk {

with TF_Log=DEBUG get his error message. looks like "AttributeName("backup") " becomes mandatory.

[ERROR] provider.terraform-provider-proxmox_v2.9.13: Response contains error diagnostic: @caller=github.com/hashicorp/[email protected]/tfprotov5/internal/diag/diagnostics.go:55 tf_req_id=8a1111f2-24fa-086b-11cc-a35d385b9ccb @module=sdk.proto diagnostic_detail= diagnostic_summary="bool is required" tf_provider_addr=registry.terraform.io/telmate/proxmox tf_resource_type=proxmox_vm_qemu diagnostic_attribute=AttributeName("disk").ElementKeyInt(0).AttributeName("backup") tf_proto_version=5.3 tf_rpc=UpgradeResourceState diagnostic_severity=ERROR timestamp=2023-02-23T23:16:04.584+0100

Rollback to version v2.9.11 works fine

@memoryleak
Copy link

Version: 2.9.13

locals {
  vm_qemu_automatic_reboot = true
  vm_qemu_clone_rhel8      = "template-rhel8-server"
  vm_qemu_clone_rhel9      = "template-rhel9-server"
  vm_qemu_clone_f37        = "template-fedora37-server"
  vm_qemu_clone_centos8    = "template-centos8-server"
  vm_qemu_clone_centos9    = "template-centos9-server"
  vm_qemu_clone_ubuntu2004 = "template-ubuntu2004-server"
  vm_qemu_cores            = 4
  vm_qemu_cpu              = "host"
  vm_qemu_memory16         = 16384
  vm_qemu_memory8          = 8192
  vm_qemu_memory4          = 4096
  vm_qemu_onboot           = true
  vm_qemu_sockets          = 1
  vm_qemu_target_node      = "pve0"

  vm_qemu_disk_backup  = true
  vm_qemu_disk_size    = "100G"
  vm_qemu_disk_ssd     = 1
  vm_qemu_disk_storage = "local-zfs"
  vm_qemu_disk_type    = "virtio"
  vm_qemu_disk_discard = "on"

  vm_qemu_network_bridge = "vmbr0"
  vm_qemu_network_model  = "virtio"
  vm_qemu_agent          = 1
}
resource "proxmox_vm_qemu" "apps" {
  vmid = 200
  name = "apps"

  automatic_reboot = local.vm_qemu_automatic_reboot
  clone            = local.vm_qemu_clone_rhel9
  cores            = local.vm_qemu_cores
  cpu              = local.vm_qemu_cpu
  memory           = local.vm_qemu_memory8
  onboot           = local.vm_qemu_onboot
  sockets          = local.vm_qemu_sockets
  target_node      = local.vm_qemu_target_node
  agent            = local.vm_qemu_agent

  disk {
    discard = local.vm_qemu_disk_discard
    backup  = local.vm_qemu_disk_backup
    size    = local.vm_qemu_disk_size
    storage = local.vm_qemu_disk_storage
    type    = local.vm_qemu_disk_type
  }

  network {
    bridge  = local.vm_qemu_network_bridge
    model   = local.vm_qemu_network_model
    macaddr = "4E:52:21:67:C0:B1"
  }
}

Output:

│ Error: bool is required
│ 
│   with proxmox_vm_qemu.apps,
│   on machine-apps.tf line 17, in resource "proxmox_vm_qemu" "apps":
│   17:     backup  = local.vm_qemu_disk_backup
│ 

This used to work.

@vilhelmprytz
Copy link
Contributor

I'm having the same problem. Anything new regarding this?

@n8rade
Copy link

n8rade commented Mar 1, 2023

The backup option was changed to a bool with this PR included in 2.9.13: #669

@memoryleak
Copy link

Hi @n8rade: I did try changing the value to "true" and I still get the same error. It seems to not accept any kind of value - at least for me.

@n8rade
Copy link

n8rade commented Mar 2, 2023

Since it doesn't seem to be recognizing "true" as a bool for whatever reason can you try defining the variable as so?

variable "vm_qemu_disk_backup" {
  type = bool
  description = "Whether the VM will backup the disk or not"
  default = true
}

And see if that might work better?

I would expect it to convert the type automatically but for whatever reason that doesn't seem to be working in you case: https://developer.hashicorp.com/terraform/language/expressions/types#type-conversion

@lkubb
Copy link

lkubb commented Mar 3, 2023

#681 (comment)

The issue is that the saved state contains the integer still. This breaking change could have been handled better tbh.

@angelbarrera92
Copy link

Then, do we have to modify the state file to modify this flag somehow?
@lkubb Did you tried?

@marksie1988
Copy link

@angelbarrera92 I had this issue, breaking change should have taken the version up to 2.x.x so people were aware but hey ho not much we can do about that :)

I updated the state file and it worked for me.

@angelbarrera92
Copy link

@angelbarrera92 I had this issue, breaking change should have taken the version up to 2.x.x so people were aware but hey ho not much we can do about that :)

I updated the state file and it worked for me.

How you did it?

@marksie1988
Copy link

@angelbarrera92 I had this issue, breaking change should have taken the version up to 2.x.x so people were aware but hey ho not much we can do about that :)
I updated the state file and it worked for me.

How you did it?

Just edited the terraform.state file directly changed the 0 to false.

@retoo
Copy link

retoo commented May 10, 2023

I downgraded to 2.9.11, changing the state is not really a good option for us right now.

@angelbarrera92
Copy link

angelbarrera92 commented May 29, 2023

@angelbarrera92 I had this issue, breaking change should have taken the version up to 2.x.x so people were aware but hey ho not much we can do about that :)
I updated the state file and it worked for me.

How you did it?

Just edited the terraform.state file directly changed the 0 to false.

I can confirm changing in the terraform state file worked for me:

Steps I`ve followed:

$ terraform state pull > backup.state
$ cp backup.state new.state

Then, modify new.state file by:

  • Increase the serial with +1.
  • Replace "backup": 0 with "backup": false.
    • First, write down the number of errors by running terraform plan.
    • Then check the number of appearances looking for "backup": 0are the same as errors reported byterraform plan`.
$ terraform state push new.state

Check everything is working fine by re-running: terraform plan.

@hestiahacker
Copy link
Contributor

As a user, I would like version v2.9.13 and later to be able to use terraform.tfstate files created with v2.9.11 and earlier.

I'm not really a Go programmer, but it looks like there's a StateFun attribute that can be added to the backup parameter which could convert from a number to a boolean if necessary.

If I understand everything correctly, that should fix this issue. The code should accept integers or booleans for the input, and only store and output boolean values. We just need someone who can write Go (or at least someone who is willing to hack it out and has a test environment to verify things work as expected) to make the change. My test environment is currently called "production", so I do not qualify for this task at this time.

@github-actions
Copy link

This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs

Copy link

This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs

@hestiahacker
Copy link
Contributor

Dear @github-actions: please do not remove issues from the backlog just because nobody has gotten to them yet.

@hestiahacker
Copy link
Contributor

This is in my queue to investigate after #687.

My plan is to:

  1. destroy a test VM
  2. downgrade to provider v2.9.11 in my terraform file
  3. terraform get -update && terraform init -upgrade && terraform version to make sure everything look good
  4. deploy (should work fine)
  5. upgrade to the latest version of the provider
  6. attempt to deploy again (should give me this error)

At that point, I should be in that wedged state. From there, I'll branch off of the latest working commit (currently 4c0c8fc) and see if I can create a patch to automatically convert from a zero to a "false". If my patch works, I should be able to deploy the VM without making any manual changes to terraform state files.

@hestiahacker
Copy link
Contributor

I deployed this small example with v2.9.11 of the provider (I did have to change the disks block back to a disk block, but other than that it's the same).

Then I updated the terrafor to use v3.0.1-rc1, and did terraform get -update && terraform init -upgrade && terraform version to make sure everything updated as I expected. I also updated the disk block in the terraform to be a disks block to be compatible with the new version of the provider.

When I then ran terraform get -update && terraform init -upgrade && terraform version I was able to reproduce this issue. This means I should be able to use this terraform.state file to work on this ticket. I'll try to use the StateFun that I mentioned eariler to get this fixed up.

@hestiahacker
Copy link
Contributor

I've submitted a patch that fixes this error without having to manually change any tfstate files. I do not expect that it will make the 3.0.1 release, as that already has a release candidate that is in testing.

Hopefully this will be included in the following release, which will probably be 3.0.2.

mleone87 pushed a commit that referenced this issue Feb 21, 2024
#923)

* fix: enable new versions of providers to run on old tfstate files #702

* feat: re-applying patch to restore backward compatibility with tfstate files

---------

Co-authored-by: hestia <[email protected]>
hestiahacker pushed a commit to hestiahacker/terraform-provider-proxmox that referenced this issue Feb 23, 2024
spettinichi pushed a commit to spettinichi/terraform-provider-proxmox that referenced this issue Mar 12, 2024
…mate#702 (Telmate#923)

* fix: enable new versions of providers to run on old tfstate files Telmate#702

* feat: re-applying patch to restore backward compatibility with tfstate files

---------

Co-authored-by: hestia <[email protected]>
spettinichi pushed a commit to spettinichi/terraform-provider-proxmox that referenced this issue Mar 12, 2024
…mate#702 (Telmate#923)

* fix: enable new versions of providers to run on old tfstate files Telmate#702

* feat: re-applying patch to restore backward compatibility with tfstate files

---------

Co-authored-by: hestia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants