-
Notifications
You must be signed in to change notification settings - Fork 108
Support enterprise feature in node pool resource and improve its update logic #1832
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
Conversation
| sr.Plan.GetAttribute(ctx, path.Root("update_strategy"), &strategy) | ||
| rrifr.RequiresReplace = strategy == "rolling_update" | ||
| }, | ||
| "Triggers replacement when `k8s_version` is changed and `update_strategy` is rolling_update", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the design, Rolling Update strategy will immediately trigger a recycle of the node pool when the k8s version is updated. Unfortunately we don't have enough valid k8s version for testing. Can validate it once possible.
ezilber-akamai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests passing locally. Nice work!
jriddle-linode
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
📝 Description
This PR supports new fields
k8s_version,rolling_updatefor LKE-enterprise node pool. Also improves the Update logic in the resource to better match our code standard and reduce sending unnecessary update calls.✔️ How to Test
The linodego change hasn't merged yet. May need to pull this PR locally and replace the linodego dependency.
Run the whole
lkenodepoolintegration test to make sure the new feature and improvement both works as expected.Manual test:
update_strategy. For example, update theupdate_strategyto berolling_updatein the above config.rolling_update, try to update k8s_version to be any value (since there is not another valid value for us to test). For example, update thek8s_versionto belke-version, and thenmake planortf plan. Observe that the plan requires replace, i.e.Feel free to play around with updating the resource to ensure the correctness.