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): only set initialization.upgrade attribute when not using custom cloud-init file #1253

Merged
merged 3 commits into from
May 6, 2024

Conversation

nankeen
Copy link
Contributor

@nankeen nankeen commented May 1, 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

Ran this with the situation that caused me to report #1252

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

Closes #1252
Closes #1263

Signed-off-by: Kai <nankeen@users.noreply.github.com>
@nankeen nankeen changed the title fix(vm): ignore upgrade parameter when custom cloud-init file is used fix(vm): only set initialization.upgrade attribute when not using custom cloud-init file May 2, 2024
@bpg
Copy link
Owner

bpg commented May 2, 2024

Thanks for the PR @nankeen!

I'm curious tho, why this bug is not triggered by the existing example resource that has custom files defined.

I may need to take a deeper look at this, and try to reproduce the use case to make sure the fix addresses the issue.

EDIT: just realized this is another cloning issue 🤦🏼

@nankeen
Copy link
Contributor Author

nankeen commented May 3, 2024

I think it is because the example resource isn't cloning an existing VM.

@bpg
Copy link
Owner

bpg commented May 3, 2024

Not that I'm against this fix by any means, but I have troubles reproducing this issue as described in #1252

I'm getting an OK response to the similar request:

Screenshot 2024-05-03 at 4 43 26 PM

@nankeen could you possible shed some light on p.2 from your scenario:

  1. Create a 'proxmox_virtual_environment_vm' resource that is cloud init capable

What exactly was configured in the source VM? Would you mind sharing a redacted template?

Also, what is your Proxmox VE version?

@bpg bpg added the ⌛ pending author's response Requested additional information from the reporter label May 4, 2024
@bpg
Copy link
Owner

bpg commented May 6, 2024

I guess this is PVE 7.x -specific, see another case in #1263

@bpg bpg removed the ⌛ pending author's response Requested additional information from the reporter label May 6, 2024
Copy link
Owner

@bpg bpg left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@bpg bpg merged commit 9762405 into bpg:main May 6, 2024
7 checks passed
@bpg
Copy link
Owner

bpg commented May 6, 2024

@all-contributors please add @nankeen for code

Copy link
Contributor

@bpg

I've put up a pull request to add @nankeen! 🎉

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