Skip to content

feat: SkipMachinePoolModelReconciliation#6325

Open
jackfrancis wants to merge 3 commits into
kubernetes-sigs:mainfrom
jackfrancis:skip-machinepool-model-reconciliation
Open

feat: SkipMachinePoolModelReconciliation#6325
jackfrancis wants to merge 3 commits into
kubernetes-sigs:mainfrom
jackfrancis:skip-machinepool-model-reconciliation

Conversation

@jackfrancis
Copy link
Copy Markdown
Contributor

@jackfrancis jackfrancis commented May 21, 2026

What type of PR is this?

What this PR does / why we need it:

This PR adds a SkipMachinePoolModelReconciliation feature gate to disable AzureMachinePool reconciliation and replacement of VMSS VMs that are running a non-latest VMSS model.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests
  • cherry-pick candidate

Release note:

feat: SkipMachinePoolModelReconciliation

@jackfrancis
feat: SkipMachinePoolModelReconciliation
89b4468 
Signed-off-by: Jack Francis <jackfrancis@gmail.com>
@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 21, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 21, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign neolit123 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from marosset and nojnhuh May 21, 2026 16:40
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.82%. Comparing base (84c7570) to head (8c2214c).
⚠️ Report is 112 commits behind head on main.

Files with missing lines Patch % Lines
...ool_deployments/machinepool_deployment_strategy.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6325      +/-   ##
==========================================
+ Coverage   43.74%   43.82%   +0.08%     
==========================================
  Files         289      291       +2     
  Lines       25475    25331     -144     
==========================================
- Hits        11143    11102      -41     
+ Misses      13529    13456      -73     
+ Partials      803      773      -30

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

willie-yao
willie-yao reviewed May 21, 2026
View reviewed changes
Comment thread feature/feature.go
Comment thread azure/scope/machinepool.go Outdated
Comment thread azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go Outdated
Comment thread azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go Outdated
Comment thread azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go Outdated
Comment thread azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go
@jackfrancis
review feedback
96d9e1b 
Signed-off-by: Jack Francis <jackfrancis@gmail.com>
@willie-yao
Copy link
Copy Markdown
Contributor

willie-yao commented May 21, 2026

/retest

willie-yao
willie-yao reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@willie-yao willie-yao left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you also add some documentation to the book for this feature flag? Otherwise just one more comment and it looks good.

Comment thread azure/scope/machinepool.go
Copy link
Copy Markdown
Contributor

@willie-yao willie-yao May 21, 2026

Choose a reason for hiding this comment

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

Can you also add a test case to TestMachinePoolScope_NeedsRequeue to teset the new path as well? Something like "should not requeue if an instance VM image does not match the VMSS when SkipMachinePoolModelReconciliation is enabled". Sorry I missed this in the initial review.

@jackfrancis
additional test scenario and documentation
8c2214c 
Signed-off-by: Jack Francis <jackfrancis@gmail.com>
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 22, 2026
willie-yao
willie-yao reviewed May 22, 2026
View reviewed changes
Comment thread docs/book/src/self-managed/machinepools.md
proactively replace instances running an older model with new instances (running the latest model).

This is useful for testing scenarios and for operators who want to manage instance refresh on their own schedule
without disabling other reconciliation behavior. To enable it, set `EXP_SKIP_MACHINE_POOL_MODEL_RECONCILIATION=true`
Copy link
Copy Markdown
Contributor

@willie-yao willie-yao May 22, 2026

Choose a reason for hiding this comment

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

Where is this environment variable wired? It doesn't look like it's being read in manager.yaml like the other feature gates.

Comment thread docs/book/src/self-managed/machinepools.md

### Skipping Model Reconciliation
- **Feature status:** Experimental (Alpha)
- **Feature gate:** SkipMachinePoolModelReconciliation=false
Copy link
Copy Markdown
Contributor

@willie-yao willie-yao May 22, 2026

Choose a reason for hiding this comment

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

Should this be SkipMachinePoolModelReconciliation=true or is it indicating the default is false?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willie-yao willie-yao willie-yao left review comments

@marosset marosset Awaiting requested review from marosset

@nojnhuh nojnhuh Awaiting requested review from nojnhuh

Assignees

No one assigned

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

CAPZ Planning
Status: Todo

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jackfrancis @k8s-ci-robot @willie-yao