Skip to content

Implement the fourth wave of per-reconciler leaderelection#1304

Merged
knative-prow-robot merged 1 commit intoknative:masterfrom
mattmoor:leader-aware-genreconciler
Jun 22, 2020
Merged

Implement the fourth wave of per-reconciler leaderelection#1304
knative-prow-robot merged 1 commit intoknative:masterfrom
mattmoor:leader-aware-genreconciler

Conversation

@mattmoor
Copy link
Copy Markdown
Member

@mattmoor mattmoor commented May 5, 2020

@knative-prow-robot knative-prow-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 5, 2020
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 5, 2020
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 5, 2020
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 5, 2020
@mattmoor
Copy link
Copy Markdown
Member Author

mattmoor commented May 6, 2020

/retest

@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 700fa72 to f8c8f37 Compare May 8, 2020 19:31
@mattmoor
Copy link
Copy Markdown
Member Author

mattmoor commented May 8, 2020

/retest

@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 1a36e1f to f8c8f37 Compare May 8, 2020 22:02
@mattmoor mattmoor mentioned this pull request May 8, 2020
Closed
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch 2 times, most recently from 1eedded to 853d7f1 Compare May 11, 2020 15:01
@knative-prow-robot knative-prow-robot added area/test-and-release needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 11, 2020
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch 2 times, most recently from e411a81 to 76ec52d Compare May 12, 2020 05:15
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2020
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 76ec52d to 2a191f6 Compare May 14, 2020 20:09
@mattmoor mattmoor mentioned this pull request May 14, 2020
Merged
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2020
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 2a191f6 to d5bd843 Compare May 29, 2020 22:35
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 29, 2020
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch 2 times, most recently from b51e836 to a6fde80 Compare May 29, 2020 23:59
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from a6fde80 to 1a0d577 Compare June 6, 2020 20:50
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Jun 6, 2020

The following is the coverage report on the affected files.
Say /test pull-knative-pkg-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
controller/controller.go 92.9% 91.3% -1.5
leaderelection/config.go 56.5% 72.0% 15.5
leaderelection/context.go Do not exist 81.8%
reconciler/leader.go Do not exist 100.0%
webhook/certificates/certificates.go 86.2% 83.9% -2.3
webhook/certificates/controller.go 100.0% 87.5% -12.5
webhook/configmaps/configmaps.go 94.3% 91.8% -2.5
webhook/configmaps/controller.go 100.0% 91.7% -8.3
webhook/psbinding/controller.go 100.0% 92.3% -7.7
webhook/psbinding/psbinding.go 88.1% 87.4% -0.7
webhook/psbinding/reconciler.go 79.6% 78.7% -0.9
webhook/resourcesemantics/conversion/controller.go 100.0% 80.0% -20.0
webhook/resourcesemantics/conversion/reconciler.go 96.4% 90.3% -6.1
webhook/resourcesemantics/defaulting/controller.go 100.0% 90.9% -9.1
webhook/resourcesemantics/defaulting/defaulting.go 88.5% 87.3% -1.2
webhook/resourcesemantics/validation/controller.go 100.0% 93.8% -6.2
webhook/resourcesemantics/validation/reconcile_config.go 95.5% 91.5% -4.0

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2020
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 1a0d577 to 078e1e3 Compare June 19, 2020 19:03
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 19, 2020
@mattmoor mattmoor changed the title [WIP] Implement the fourth wave of per-reconciler leaderelection Implement the fourth wave of per-reconciler leaderelection Jun 19, 2020
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 19, 2020
@mattmoor
Copy link
Copy Markdown
Member Author

mattmoor commented Jun 19, 2020

/retest

🤔

@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from 078e1e3 to b369c6c Compare June 19, 2020 20:27
@mattmoor
Copy link
Copy Markdown
Member Author

mattmoor commented Jun 19, 2020

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 19, 2020
whaught
whaught approved these changes Jun 19, 2020
View reviewed changes
Comment thread codegen/cmd/injection-gen/generators/reconciler_reconciler.go
@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 19, 2020
@mattmoor
Implement the fourth wave of per-reconciler leaderelection.
6e96f61 
With this, all downstream `// +genreconciler` reconcilers will become leader-aware and expose a model for the typed reconcilers they wrap to themselves become leader-aware.

Detailed design: https://docs.google.com/document/d/1i_QHjQO2T3SNv49xjZLWlivcc0UvZN1Tbw2NKxThkyM/edit#
Issue: knative#1181
@mattmoor mattmoor force-pushed the leader-aware-genreconciler branch from b369c6c to 6e96f61 Compare June 19, 2020 21:49
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 19, 2020
@whaught
Copy link
Copy Markdown
Contributor

whaught commented Jun 19, 2020

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 19, 2020
@dprotaso
Copy link
Copy Markdown
Member

dprotaso commented Jun 22, 2020

/approve

@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Jun 22, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, mattmoor, whaught

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

@dprotaso
Copy link
Copy Markdown
Member

dprotaso commented Jun 22, 2020

FYI - there's a hold - unsure about the context

@mattmoor
Copy link
Copy Markdown
Member Author

mattmoor commented Jun 22, 2020

Context for /hold is that I wanted @n3wscott to TAL, since he's been driving most of the codegen work (and was out Friday).

n3wscott
n3wscott reviewed Jun 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

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

/lgtm
/Unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 22, 2020
@knative-prow-robot knative-prow-robot merged commit 5e60b6f into knative:master Jun 22, 2020
@mattmoor mattmoor deleted the leader-aware-genreconciler branch June 22, 2020 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgerd dgerd Awaiting requested review from dgerd

@evankanderson evankanderson Awaiting requested review from evankanderson

@grantr grantr Awaiting requested review from grantr

@tcnghia tcnghia Awaiting requested review from tcnghia

2 more reviewers

@n3wscott n3wscott n3wscott left review comments

@whaught whaught whaught approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dprotaso dprotaso

@n3wscott n3wscott

@whaught whaught

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@mattmoor @knative-metrics-robot @whaught @dprotaso @knative-prow-robot @n3wscott @googlebot