Create "Lease" API in the new "coordination.k8s.io" api group #64246

wojtek-t · 2018-05-24T07:22:20Z

Part of "Efficient Node heartbeats" KEP:
https://github.com/kubernetes/community/blob/master/keps/0009-node-heartbeat.md

Part of: #14733

Create "coordination.k8s.io" api group with "Lease" api in it.

wojtek-t · 2018-05-24T14:38:28Z

/test pull-kubernetes-integration

wojtek-t · 2018-05-24T15:32:55Z

/retest

wojtek-t · 2018-05-24T15:34:37Z

@bgrant0607 @dchen1107 @derekwaynecarr @liggitt - I'm not sure if we want to merge it with 1.11 timeframe, but if so, the PR adding the "Lease" API is essentially ready for review.

k8s-github-robot · 2018-06-26T18:35:36Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@bgrant0607 @dchen1107 @derekwaynecarr @liggitt @mtaufen @wojtek-t

Pull Request Labels

sig/architecture sig/cluster-lifecycle sig/node sig/scalability sig/scheduling: Pull Request will be escalated to these SIGs if needed.
priority/important-soon: Escalate to the pull request owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
kind/feature: New functionality.

Help

liggitt · 2018-06-26T19:26:31Z

pkg/apis/coordination/validation/validation.go

+
+// ValidateLease validates a Lease.
+func ValidateLease(lease *coordination.Lease) field.ErrorList {
+	allErrs := ValidateLeaseSpec(&lease.Spec, field.NewPath("spec"))


what are the name restrictions around Lease objects? I expected a call to ValidateObjectMeta here with a name validator

We don't have a step that does generic ObjectMeta validation?
Edit: I guess not... good to know.

it does generic objectmeta validation with backstop restrictions on object names (disallows / and % characters, disallows . and .. names), but I'm assuming we want something more restrictive than that here

@liggitt So given that we do generic ObjectMetadata validation in generic code (in staging/src/k8s.io/apiserver/pkg/registry/rest/), what other validation you would like to see here?

I'm not sure we would like to be more restrictive here.

Discussed offline with Jordan and added ValidateObjectMeta call. Hopefully that's all we need.

liggitt · 2018-06-26T19:28:15Z

pkg/registry/coordination/lease/strategy.go

+
+// AllowCreateOnUpdate is true for Lease; this means you may create one with a PUT request.
+func (leaseStrategy) AllowCreateOnUpdate() bool {
+	return false


this seems like an object it would be useful to allow this on... it would let us grant named create/update permissions on specific lease instances. as long as we don't allow unconditional update (which it looks like we don't), create-on-update is something I'd want on this resource

Also the godoc doesn't match the implementation here.

Thanks for pointing on that Jordan - I completely agree we should allow for that (and after change godoc is finally correct :))

liggitt · 2018-06-26T19:28:28Z

pkg/registry/coordination/lease/strategy.go

+
+// AllowUnconditionalUpdate is the default update policy for Lease objects.
+func (leaseStrategy) AllowUnconditionalUpdate() bool {
+	return false


+1 for this

mtaufen · 2018-06-26T19:53:58Z

/lgtm cancel
so we can address @liggitt's comments

wojtek-t · 2018-06-27T11:31:29Z

PTAL

liggitt · 2018-06-27T13:43:56Z

/lgtm

k8s-ci-robot · 2018-06-27T13:44:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, wojtek-t

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:

~~api/OWNERS~~ [liggitt,wojtek-t]
~~cmd/kube-apiserver/OWNERS~~ [liggitt,wojtek-t]
~~cmd/kube-controller-manager/OWNERS~~ [wojtek-t]
~~docs/api-reference/OWNERS~~ [liggitt,wojtek-t]
~~hack/OWNERS~~ [liggitt,wojtek-t]
~~pkg/OWNERS~~ [wojtek-t]
~~staging/OWNERS~~ [wojtek-t]
~~test/OWNERS~~ [liggitt,wojtek-t]

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

k8s-github-robot · 2018-06-27T14:29:55Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-06-27T15:17:09Z

Automatic merge from submit-queue (batch tested with PRs 64246, 65489, 65443). If you want to cherry-pick this change to another branch, please follow the instructions here.

zparnold · 2018-07-16T06:17:11Z

Hey there @wojtek-t, I was reading through this PR and associated KEP (I'm the Docs 1.12 release lead) do you think this is something that would need documentation around it? I was thinking it could be something mentioned in the large cluster section of the docs. https://kubernetes.io/docs/setup/cluster-large/ What do you think?

wojtek-t · 2018-07-16T12:09:16Z

Hey there @wojtek-t, I was reading through this PR and associated KEP (I'm the Docs 1.12 release lead) do you think this is something that would need documentation around it? I was thinking it could be something mentioned in the large cluster section of the docs. https://kubernetes.io/docs/setup/cluster-large/ What do you think?

Yes - it requires release note. Though, it's not large-cluster specific. The change was motivated by large clusters, but it will touch clusters of all sizes. So it should be in some more generic place (though I don't know where exactly).
I'm leaving for paternity leave within days (hours?) so @mtaufen will be able to assist you with that.

mtaufen · 2018-07-16T16:21:27Z

@zparnold, I'm working on the node heartbeats side of this, so let's definitely stay in touch regarding docs and where to put them.

nikhita · 2018-09-12T15:11:15Z

This should have a release note.

@wojtek-t Can you edit the main PR body to include the release note? Thanks!

Make periodic NodeStatus updates cheaper cherry-pick master, create lease api, kubernetes#64246 master, node lifecycle controller, kubernetes#69241 kubelet, kubernetes#66257 See merge request !184866

wojtek-t self-assigned this May 24, 2018

k8s-ci-robot requested review from caesarxuchao and cjcullen May 24, 2018 07:22

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api labels May 24, 2018

wojtek-t changed the title ~~Create "Lease" object type~~ [WIP] Create "Lease" object type May 24, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 24, 2018

wojtek-t force-pushed the lease_object_type branch from 0022e67 to 3702402 Compare May 24, 2018 11:53

wojtek-t added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 24, 2018

wojtek-t force-pushed the lease_object_type branch from 3702402 to 92bd65a Compare May 24, 2018 13:51

wojtek-t removed their assignment May 24, 2018

wojtek-t changed the title ~~[WIP] Create "Lease" object type~~ Create "Lease" object type May 24, 2018

wojtek-t assigned liggitt, derekwaynecarr, bgrant0607 and dchen1107 May 24, 2018

wojtek-t changed the title ~~Create "Lease" object type~~ Create "Lease" API in the new "coordination.k8s.io" api group May 24, 2018

wojtek-t added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 24, 2018

liggitt reviewed Jun 26, 2018

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2018

wojtek-t force-pushed the lease_object_type branch from de84b51 to 134ce7b Compare June 27, 2018 10:51

wojtek-t added 4 commits June 27, 2018 13:30

Add coordination API group with Lease type

f38e952

Create coordination registry

b95db13

Enable coordination api group

c79b54d

Autogenerated stuff

0950084

wojtek-t force-pushed the lease_object_type branch from 134ce7b to 0950084 Compare June 27, 2018 11:31

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 27, 2018

k8s-github-robot merged commit 6d3bba7 into kubernetes:master Jun 27, 2018

wojtek-t deleted the lease_object_type branch July 2, 2018 08:29

mtaufen mentioned this pull request Jul 17, 2018

Move frequent Kubelet heartbeats to Lease API kubernetes/enhancements#589

Closed

6 tasks

liggitt mentioned this pull request Aug 8, 2018

leader election: a records's observedTime is checked instead of an actual timestamp kubernetes/client-go#448

Closed

kdomanski mentioned this pull request Aug 8, 2018

machine-controller always needs to wait a bit for its lock kubermatic/machine-controller#264

Closed

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Sep 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create "Lease" API in the new "coordination.k8s.io" api group #64246

Create "Lease" API in the new "coordination.k8s.io" api group #64246

wojtek-t commented May 24, 2018 •

edited

Loading

wojtek-t commented May 24, 2018

wojtek-t commented May 24, 2018

wojtek-t commented May 24, 2018

k8s-github-robot commented Jun 26, 2018

liggitt Jun 26, 2018

mtaufen Jun 26, 2018 •

edited

Loading

liggitt Jun 26, 2018

wojtek-t Jun 27, 2018

wojtek-t Jun 27, 2018

liggitt Jun 26, 2018 •

edited

Loading

mtaufen Jun 26, 2018

wojtek-t Jun 27, 2018

liggitt Jun 26, 2018

mtaufen commented Jun 26, 2018

wojtek-t commented Jun 27, 2018

liggitt commented Jun 27, 2018

k8s-ci-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

zparnold commented Jul 16, 2018

wojtek-t commented Jul 16, 2018

mtaufen commented Jul 16, 2018

nikhita commented Sep 12, 2018

Create "Lease" API in the new "coordination.k8s.io" api group #64246

Create "Lease" API in the new "coordination.k8s.io" api group #64246

Conversation

wojtek-t commented May 24, 2018 • edited Loading

wojtek-t commented May 24, 2018

wojtek-t commented May 24, 2018

wojtek-t commented May 24, 2018

k8s-github-robot commented Jun 26, 2018

liggitt Jun 26, 2018

Choose a reason for hiding this comment

mtaufen Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

liggitt Jun 26, 2018

Choose a reason for hiding this comment

wojtek-t Jun 27, 2018

Choose a reason for hiding this comment

wojtek-t Jun 27, 2018

Choose a reason for hiding this comment

liggitt Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

mtaufen Jun 26, 2018

Choose a reason for hiding this comment

wojtek-t Jun 27, 2018

Choose a reason for hiding this comment

liggitt Jun 26, 2018

Choose a reason for hiding this comment

mtaufen commented Jun 26, 2018

wojtek-t commented Jun 27, 2018

liggitt commented Jun 27, 2018

k8s-ci-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

zparnold commented Jul 16, 2018

wojtek-t commented Jul 16, 2018

mtaufen commented Jul 16, 2018

nikhita commented Sep 12, 2018

wojtek-t commented May 24, 2018 •

edited

Loading

mtaufen Jun 26, 2018 •

edited

Loading

liggitt Jun 26, 2018 •

edited

Loading