Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added support for node affinity labels #94

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

rosmo
Copy link
Contributor

@rosmo rosmo commented May 24, 2022

Adds support for node_affinity field in googlecompute source. Example:

  node_affinity { 
    key = "workload"
    operator = "IN"
    values = ["packer"]
  }
  node_affinity { 
    key = "workload"
    operator = "NOT_IN"
    values = ["notpacker"]
  }

I put WIP in the title because the test are giving me a panic which seems to be unrelated to my changes. However, I tested building images with and without node_affinity configs. Also I'm not sure what parts get autogenerated and I shouldn't be including in my PR.

The PR also touches the waitForState mechanism, because if a sole-tenant node is not available the operation state becomes DONE but there is a precondition failed error emitted, which is flagged as a retryable error and will keep the function spinning until timeout. I'm also fine with taking that out, in case you think it's too much.

Closes #93

@rosmo rosmo requested a review from a team as a code owner May 24, 2022 09:26
@rosmo
Copy link
Contributor Author

rosmo commented May 27, 2022

Removing WIP, since make testacc gives a full PASS. make test works too - figured out it really needs to be run on GCE (looks to be OS Login related).

@rosmo rosmo changed the title [WIP] Added support for node affinity labels Added support for node affinity labels May 27, 2022
@nywilken nywilken self-assigned this Jun 9, 2022
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

@rosmo thanks for the contribution. Change wise this is looking good to me.

I'm a bit curious about your comment " figured out it really needs to be run on GCE (looks to be OS Login related)." Are you implying there is an issue with how OS Login works. Or does node affinity only work when Packer is executed on GCE nodes?

I would like to see a bit more documentation around the node affinity config attribute. Specifically what are the requirements for using affinity labels and an example of what an actual config may look like.

@rosmo rosmo force-pushed the gcp-node-affinities branch from 2795e5e to 027d33d Compare June 13, 2022 10:05
@rosmo rosmo force-pushed the gcp-node-affinities branch from 027d33d to a6141b9 Compare June 13, 2022 10:07
@rosmo
Copy link
Contributor Author

rosmo commented Jun 13, 2022

re: OS Login - I think the test suite was making an assumption about some username (testing@packer.io) where mine was different. Not a big deal, as it's completely unrelated to this.

I added a link to the Sole Tenancy (aka node affinity labels) to the documentation - the feature is mainly for a) compliance purposes, and/or b) building Windows images using BYOL (as per licensing terms BYOL licenses are not allowed to run on shared tenancy infrastructure).

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Thanks for making the updates. The changes look good to me.

@nywilken nywilken merged commit 2923193 into hashicorp:main Jun 22, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SoleTenant node support during build process
2 participants