Improve APIService auto-registration for HA/upgrade scenarios #51921

liggitt · 2017-09-05T01:48:56Z

Fixes #51912

Required for 1.8 due to impact on HA upgrades.

cc @kubernetes/sig-api-machinery-bugs

Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served.

deads2k · 2017-09-05T12:10:05Z

cmd/kube-apiserver/app/aggregator.go

 		go crdRegistrationController.Run(5, context.StopCh)
+		go func() {
+			// let the CRD controller register APIServices for existing CRDs before starting the autoregistration controller


this is "what is it doing", not "why is it doing it". I can tell its waiting on priming for something, but its not obvious why that matters. Particularly since this won't gate a health check.

deads2k · 2017-09-05T12:59:45Z

staging/src/k8s.io/kube-aggregator/pkg/controllers/autoregister/autoregister_controller.go

-func (c *autoRegisterController) checkAPIService(name string) error {
+// checkAPIService syncs the current APIService against a list of desired APIService objects
+//
+// Current:           \          Desired: | A. not found | B. sync on start | C. sync always


I must be misunderstanding this table. I expected to see current \ desired in the first column, but nothing has a backslash and most don't have two conditions. Is this column just "observed state"?

deads2k · 2017-09-05T13:00:17Z

staging/src/k8s.io/kube-aggregator/pkg/controllers/autoregister/autoregister_controller.go

+// 1. error                               | error        | error            | error
+// 2. not found                           | -            | create once      | create
+// 3. no sync                             | -            | -                | -
+// 4. sync on start, not present at start | -            | -                | -


is "not present at start" desired or actual state? I think its desired, but that wouldn't match the table header.

x column was desired, y column was current. fixed the headers to clarify.

deads2k · 2017-09-05T13:04:38Z

staging/src/k8s.io/kube-aggregator/pkg/controllers/autoregister/autoregister_controller.go

 	case apierrors.IsNotFound(err) && desired != nil:
 		_, err := c.apiServiceClient.APIServices().Create(desired)
 		return err

-	// we aren't trying to manage this APIService.  If the user removes the label, he's taken over management himself
-	case curr.Labels[AutoRegisterManagedLabel] != "true":


This worked out nicely. Old controllers just stop messing with them.

exactly. I'll still pick this back to 1.7, but this at least makes upgrades from 1.7 servers with this issue leave the new servers in a good state.

deads2k · 2017-09-05T13:34:28Z

cmd/kube-apiserver/app/aggregator.go

+		if allAvailable.Load().(bool) {
+			return nil
+		}
+
 		items, err := aggregatorServer.APIRegistrationInformers.Apiregistration().InternalVersion().APIServices().Lister().List(labels.Everything())


We only want to wait until everything has been observed at least once, right? Seems like that requires a watch from a listed resource version, not a lister since this lister is vulnerable to a create, then delete.

This pull makes it much less likely if you want to fix it later.

switched to a watch

deads2k · 2017-09-05T17:51:11Z

/lgtm

k8s-github-robot · 2017-09-05T17:51:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt

Associated issue: 51912

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kube-apiserver/OWNERS~~ [deads2k,liggitt]
~~pkg/master/OWNERS~~ [deads2k]
~~staging/src/k8s.io/kube-aggregator/OWNERS~~ [deads2k]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

liggitt · 2017-09-05T19:04:12Z

flake on #50695
/test pull-kubernetes-e2e-gce-etcd3

fejta-bot · 2017-09-05T22:03:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-06T02:36:50Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

k8s-github-robot · 2017-09-06T03:38:21Z

Automatic merge from submit-queue

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of #51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ```

lavalamp · 2017-09-07T20:42:30Z

/assign @cheftako

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of kubernetes/kubernetes#51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ``` Kubernetes-commit: f20e19779c9ef97c930ec1be080c78c1b33345f7

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of kubernetes/kubernetes#51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ``` Kubernetes-commit: f20e19779c9ef97c930ec1be080c78c1b33345f7

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of kubernetes/kubernetes#51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ``` Kubernetes-commit: f20e19779c9ef97c930ec1be080c78c1b33345f7

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of kubernetes/kubernetes#51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ``` Kubernetes-commit: f20e19779c9ef97c930ec1be080c78c1b33345f7

@deads2k

…1-upstream-release-1.7-1504632714 Automatic merge from submit-queue Automated cherry pick of #51921 Cherry pick of kubernetes/kubernetes#51921 on release-1.7. Fixes #51912 Picking to 1.7.x due to impact on HA upgrades. /assign @deads2k cc @kubernetes/sig-api-machinery-bugs ```release-note Fixes an issue with APIService auto-registration affecting rolling HA apiserver restarts that add or remove API groups being served. ``` Kubernetes-commit: f20e19779c9ef97c930ec1be080c78c1b33345f7

k8s-ci-robot assigned deads2k Sep 5, 2017

k8s-github-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Sep 5, 2017

liggitt force-pushed the local-apiservice branch 3 times, most recently from a3dd9ba to 80cef8d Compare September 5, 2017 03:49

deads2k reviewed Sep 5, 2017

View reviewed changes

liggitt force-pushed the local-apiservice branch from 80cef8d to 60170da Compare September 5, 2017 13:22

liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. 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 Sep 5, 2017

liggitt added this to the v1.8 milestone Sep 5, 2017

liggitt added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. cherrypick-candidate labels Sep 5, 2017

deads2k reviewed Sep 5, 2017

View reviewed changes

liggitt added 4 commits September 5, 2017 13:12

Limit APIService healthz check to startup

9a8b111

Make local APIService objects available on create

d353adc

Prevent flutter of CRD APIServices on start

0529dd4

Sync local APIService objects once

8ca6d99

liggitt force-pushed the local-apiservice branch from 60170da to 8ca6d99 Compare September 5, 2017 17:14

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 5, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2017

liggitt mentioned this pull request Sep 5, 2017

Automated cherry pick of #51921 #51969

Merged

k8s-github-robot merged commit bd4aaf4 into kubernetes:master Sep 6, 2017

liggitt deleted the local-apiservice branch September 6, 2017 13:09

k8s-ci-robot assigned cheftako Sep 7, 2017

jpbetz added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed cherrypick-candidate labels Oct 25, 2017

enj mentioned this pull request Dec 2, 2022

adding annotation for kube-aggregator automanaged in labels-md kubernetes/website#38161

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve APIService auto-registration for HA/upgrade scenarios #51921

Improve APIService auto-registration for HA/upgrade scenarios #51921

liggitt commented Sep 5, 2017 •

edited

Loading

deads2k Sep 5, 2017

liggitt Sep 5, 2017

deads2k Sep 5, 2017

deads2k Sep 5, 2017

liggitt Sep 5, 2017

deads2k Sep 5, 2017 •

edited

Loading

liggitt Sep 5, 2017

deads2k Sep 5, 2017

liggitt Sep 5, 2017

deads2k commented Sep 5, 2017

k8s-github-robot commented Sep 5, 2017

liggitt commented Sep 5, 2017

fejta-bot commented Sep 5, 2017

fejta-bot commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

lavalamp commented Sep 7, 2017

Improve APIService auto-registration for HA/upgrade scenarios #51921

Improve APIService auto-registration for HA/upgrade scenarios #51921

Conversation

liggitt commented Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Sep 5, 2017

k8s-github-robot commented Sep 5, 2017

liggitt commented Sep 5, 2017

fejta-bot commented Sep 5, 2017

fejta-bot commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

lavalamp commented Sep 7, 2017

liggitt commented Sep 5, 2017 •

edited

Loading

deads2k Sep 5, 2017 •

edited

Loading