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

Support named placements in GKE node pools #2969

Merged
merged 17 commits into from
Aug 30, 2024

Conversation

arajmane-g
Copy link
Contributor

@arajmane-g arajmane-g commented Aug 22, 2024

About the Module Improvement

Toolkit users will be able to utilise named compact placement policy in their node pools when deploying GKE clusters.

Implementation

Introduce a new setting placement_policy by deprecating compact_placement. type and name are two nested settings. The latter is optional.

Placement policy is assumed to exist.

Testing

Integration test will be added as a follow-up.

Following things were tested from the dev environment:

  • Not setting either of the settings should not include the relevant block in the generated plan
  • Setting compact_placement should fail at the Terraform plan stage with a message indicating deprecation
  • It was verified that named, unnamed compact placement policies work using the new setting
  • Terraform apply should fail for an unknown policy
  • Terraform apply should succeed for a known compact placement policy. It was verified that the policy is attached to the VMs in the provisioned node pool

Submission Checklist

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes: not applicable
  • Ensure that unit test coverage remains above 80%: not applicable
  • Update all applicable documentation
  • Follow Cluster Toolkit Contribution guidelines #

@arajmane-g arajmane-g added the release-module-improvements Added to release notes under the "Module Improvements" heading. label Aug 22, 2024
@ankitkinra ankitkinra assigned arajmane-g and unassigned ankitkinra Aug 22, 2024
@arajmane-g arajmane-g changed the base branch from develop to main August 23, 2024 12:50
@arajmane-g arajmane-g changed the base branch from main to develop August 23, 2024 12:50
@arajmane-g arajmane-g assigned ankitkinra and unassigned arajmane-g Aug 23, 2024
@arajmane-g arajmane-g changed the title Support named compact placement in GKE node pools Support named placements in GKE node pools Aug 26, 2024
modules/compute/gke-node-pool/main.tf Outdated Show resolved Hide resolved
modules/compute/gke-node-pool/main.tf Outdated Show resolved Hide resolved
modules/compute/gke-node-pool/main.tf Outdated Show resolved Hide resolved
modules/compute/gke-node-pool/variables.tf Show resolved Hide resolved
@ankitkinra ankitkinra assigned arajmane-g and unassigned ankitkinra Aug 26, 2024
@arajmane-g arajmane-g requested a review from ankitkinra August 27, 2024 12:36
@arajmane-g arajmane-g assigned ankitkinra and unassigned arajmane-g Aug 27, 2024
Copy link
Contributor

@ankitkinra ankitkinra left a comment

Choose a reason for hiding this comment

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

Approved with some nits, please fix them before merging!

modules/compute/gke-node-pool/variables.tf Outdated Show resolved Hide resolved
modules/compute/gke-node-pool/variables.tf Show resolved Hide resolved
@ankitkinra ankitkinra assigned arajmane-g and unassigned ankitkinra Aug 29, 2024
@arajmane-g arajmane-g merged commit f9e7173 into GoogleCloudPlatform:develop Aug 30, 2024
9 of 51 checks passed
@arajmane-g arajmane-g deleted the placement branch August 30, 2024 07:56
@rohitramu rohitramu mentioned this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-module-improvements Added to release notes under the "Module Improvements" heading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants