Skip to content

🐛 Allow setting empty tolerations on provider deployments#1034

Open
tahsinrahman wants to merge 1 commit into
kubernetes-sigs:mainfrom
tahsinrahman:fix-1018-empty-tolerations-cache
Open

🐛 Allow setting empty tolerations on provider deployments#1034
tahsinrahman wants to merge 1 commit into
kubernetes-sigs:mainfrom
tahsinrahman:fix-1018-empty-tolerations-cache

Conversation

@tahsinrahman
Copy link
Copy Markdown

@tahsinrahman tahsinrahman commented Apr 10, 2026 •
edited
Loading

What this PR does / why we need it:
Changes DeploymentSpec.Tolerations from []corev1.Toleration to *[]corev1.Toleration so the three states — unset (nil), explicitly empty (&[]), and non-empty — are distinguishable.

Previously, setting spec.deployment.tolerations: [] produced the same json.Marshal output as not setting tolerations at all (null vs [] both marshal identically for a bare slice). This meant the provider cache hash did not change, stale cached manifests were reused, and upstream tolerations (e.g. node-role.kubernetes.io/control-plane) remained on the provider Deployment.

With the pointer, json.Marshal emits "tolerations":null vs "tolerations":[], producing different hashes and triggering a proper re-reconciliation.

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 #1018

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 10, 2026 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-cluster-api-operator ready!

Name Link
🔨 Latest commit a15ed35
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-cluster-api-operator/deploys/69f838fba2cf250008484e14
😎 Deploy Preview https://deploy-preview-1034--kubernetes-sigs-cluster-api-operator.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 10, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Apr 10, 2026

Welcome @tahsinrahman!

It looks like this is your first PR to kubernetes-sigs/cluster-api-operator 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-operator has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Apr 10, 2026

Hi @tahsinrahman. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 10, 2026
@tahsinrahman tahsinrahman force-pushed the fix-1018-empty-tolerations-cache branch from 5787bae to e4c9a7b Compare April 10, 2026 02:08
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 10, 2026
@tahsinrahman tahsinrahman changed the title 🐛 Fix explicit empty tolerations cache invalidation 🐛 Allow setting empty tolerations on provider deployments Apr 10, 2026
@furkatgofurov7
Copy link
Copy Markdown
Member

furkatgofurov7 commented Apr 10, 2026

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 10, 2026
@furkatgofurov7
Copy link
Copy Markdown
Member

furkatgofurov7 commented Apr 10, 2026

Hey, this needs a commit message to be fixed (remove mentioning issues, i.e closes #1018)

Danil-Grigorev
Danil-Grigorev reviewed Apr 12, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Danil-Grigorev Danil-Grigorev left a comment

Choose a reason for hiding this comment

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

Looks good in general, though ptr.To should be used instead.

Comment thread internal/controller/component_customizer_test.go Outdated
Comment thread internal/controller/component_customizer_test.go Outdated
@tahsinrahman tahsinrahman force-pushed the fix-1018-empty-tolerations-cache branch from e4c9a7b to b8974a4 Compare April 13, 2026 03:01
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 13, 2026
@tahsinrahman
Copy link
Copy Markdown
Author

tahsinrahman commented Apr 13, 2026

thanks for your review @Danil-Grigorev @furkatgofurov7 , i've updated

@furkatgofurov7
Copy link
Copy Markdown
Member

furkatgofurov7 commented Apr 13, 2026

/retest

4 similar comments
@tahsinrahman
Copy link
Copy Markdown
Author

tahsinrahman commented Apr 14, 2026

/retest

@Danil-Grigorev
Copy link
Copy Markdown
Member

Danil-Grigorev commented Apr 17, 2026

/retest

@furkatgofurov7
Copy link
Copy Markdown
Member

furkatgofurov7 commented Apr 17, 2026

/retest

@Danil-Grigorev
Copy link
Copy Markdown
Member

Danil-Grigorev commented Apr 30, 2026

/retest

@tahsinrahman
Copy link
Copy Markdown
Author

tahsinrahman commented May 4, 2026

CI is failing due to the api changes. not sure what is the way forward here

sigs.k8s.io/cluster-api-operator/api/v1alpha2
  Incompatible changes:
  - DeploymentSpec.Tolerations: changed from []k8s.io/api/core/v1.Toleration to *[]k8s.io/api/core/v1.Toleration

@tahsinrahman tahsinrahman force-pushed the fix-1018-empty-tolerations-cache branch from b8974a4 to a15ed35 Compare May 4, 2026 06:13
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 4, 2026

@tahsinrahman: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-operator-apidiff-main a15ed35 link false /test pull-cluster-api-operator-apidiff-main

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@Danil-Grigorev
Copy link
Copy Markdown
Member

Danil-Grigorev commented May 8, 2026

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 8, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev

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

The pull request process is described 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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 8, 2026
@alexander-demicev
Copy link
Copy Markdown
Contributor

alexander-demicev commented May 11, 2026

@tahsinrahman hi, can you rebase?

@alexander-demicev
Copy link
Copy Markdown
Contributor

alexander-demicev commented May 11, 2026

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 11, 2026

LGTM label has been added.

DetailsGit tree hash: 33de1e59d9794cc61eb3b59d4f37922a92e832f3

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

Reviewers

@Danil-Grigorev Danil-Grigorev Danil-Grigorev left review comments

@alexander-demicev alexander-demicev Awaiting requested review from alexander-demicev

@fabriziopandini fabriziopandini Awaiting requested review from fabriziopandini

Assignees

@alexander-demicev alexander-demicev

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

spec.deployment.tolerations: [] does not invalidate provider cache, so upstream tolerations remain

5 participants

@tahsinrahman @k8s-ci-robot @furkatgofurov7 @Danil-Grigorev @alexander-demicev