-
Notifications
You must be signed in to change notification settings - Fork 280
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
feat: Lock updates on azure resources when other component is doing t… #7193
feat: Lock updates on azure resources when other component is doing t… #7193
Conversation
5995635
to
858a34e
Compare
I’m curious about which component would access the same Azure resource simultaneously, and exactly which resource is it? It seems like the lock covers the entire |
AKS RP may have conflicts with cloud provider. |
858a34e
to
fab4e84
Compare
fab4e84
to
22fc064
Compare
pkg/provider/azure_lock.go
Outdated
lease.Annotations[consts.AzureResourceLockPreviousHolderNameAnnotation], | ||
consts.AzureResourceLockHolderNameCloudControllerManager, | ||
) { | ||
l.Cloud.lbCache, err = l.Cloud.newLBCache() |
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.
Invalidates cache if the previous holder is another component. We can invalidate more caches here if needed.
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.
we'd need clear caches for vmss, vmssvm and vm
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.
Added.
22fc064
to
a8a619b
Compare
f7fbaa8
to
c949880
Compare
c949880
to
cfcd0ef
Compare
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: feiskyer, nilo19 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
1 similar comment
/retest |
E1012 01:41:09.094776 1 controllermanager.go:93] Run: failed to configure cloud controller manager: --cloud-config cannot be empty when --enable-dynamic-reloading is not set to true |
/retest |
/retest |
1 similar comment
/retest |
…he same thing. This PR utilizes a lease in each service reconciliation to prevent race conditions where cloud provider and others are updating the same azure resources.
cfcd0ef
to
30c2743
Compare
/lgtm |
/cherrypick release-1.31 |
@nilo19: #7193 failed to apply on top of branch "release-1.31":
In response to this:
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. |
/cherrypick release-1.30 |
@MartinForReal: #7193 failed to apply on top of branch "release-1.30":
In response to this:
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. |
/cherrypick release-1.30 |
@MartinForReal: #7193 failed to apply on top of branch "release-1.30":
In response to this:
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. |
…he same thing.
This PR utilizes a lease in each service reconciliation to prevent race conditions where cloud provider and others are updating the same azure resources.
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR utilizes a lease in each service reconciliation to prevent race conditions where cloud provider and others are updating the same azure resources. If the lease has not expired yet and is held by other component, the service reconciliation will be aborted and retried exponentially.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: