feat: Allow leases to have custom labels set when a new holder takes the lease #131632
Conversation
|
Welcome @DerekFrank! |
|
Hi @DerekFrank. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/ok-to-test |
9e6551b to
b63166c
Compare
|
/test pull-kubernetes-e2e-kind |
|
/cc @Jefftree |
|
/triage accepted |
b63166c to
501d207
Compare
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/leaselock.go
Show resolved
Hide resolved
501d207 to
8597aba
Compare
|
e2e failing due to a 502 response from google.com. I'm going to guess thats a flake? |
|
/retest |
84c947b to
9ca18a2
Compare
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/leaselock_test.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/leaselock_test.go
Show resolved
Hide resolved
9ca18a2 to
a89b7b7
Compare
@DerekFrank why is this complicated? The holder usually is a pod. What other information would you want to put in there? |
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/interface.go
Show resolved
Hide resolved
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/interface.go
Show resolved
Hide resolved
Our main usecase is to understand which cloud provider zone a given holder is in for zonal outage fail over mechanisms. Currently to find out what zone the holder is in one would need to:
Given the nature of zonal outages, this makes any failover mechanism susceptible to all sorts of dependency failures. |
Could you elaborate a bit as to why you need failure domain info to do failover? Another replica will automatically take over or not? Furthermore the failure domain really is a correlation, how would you be able to tell the failure was in any way caused by the replica being in a given failure domain? |
Apologies, I should have been more clear. I am attempting to setup a mechanism that, when manually activated, will force failover to happen for all controllers within a specific zone. This is useful for maintaining availability when there are known partial zonal outages for underlying cloud provider services. For an example, if a cloud provider is having a partial networking outages within a specific zone such that some calls from a controller to the control plane fail but some do not, the controllers in that zone will most likely be able to maintain leadership status. This is not ideal as they will be failing to perform optimally due to the dropped calls. We would be able to see that the controllers are impacted through routine monitoring, and could then leverage the fail over mechanism. The goal of the mechanism would be to force replicas in another zone to take leadership of the lease, mitigating impact until the partial networking outage can be resolved. Ideally the mechanism does this without relying on being able to communicate with the controllers in the affected zone as we know it will be impaired. For an example of how the label could be used, if the lease holders added their zonal information to the lease renewal requests, the api server will be able to reject lease renewal requests that originate from the impaired zone. I'm not suggesting adding that functionality specifically |
a89b7b7 to
792f229
Compare
staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/interface.go
Outdated
Show resolved
Hide resolved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DerekFrank, jpbetz 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 |
792f229 to
109ae1b
Compare
|
/lgtm |
|
LGTM label has been added. Git tree hash: 54ca0d347b91701cd999e12e2b39a041f43307d8
|
…adata feat: Allow leases to have custom labels set when a new holder takes the lease Kubernetes-commit: 3eebe67
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds unit tests for the leaselock class, and slightly modifies the functionality to allow users to set custom labels that are updated when a lease gets a new leader.
The intended use of this feature is to allow graceful fail away from leaders. Right now understanding which replica holds a lease is complicated as the holder identity is merely the name of the pod. This would simplify the process to understand who holds the lease without having to backtrace that information through the pod's host names. It also prevents race conditions from other workarounds such as custom controllers that reconcile on leases and update labels.
Which issue(s) this PR fixes:
Fixes kubernetes/client-go#1413
Special notes for your reviewer:
Does this PR introduce a user-facing change?
This change is technically user facing, as users can now leverage the functionality, but there is no action necessary. If a release note is required, I would say a note such as
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: