embed kube-apiextensions inside of kube-apiserver #46055

deads2k · 2017-05-18T21:10:31Z

To reduce operation complexity, we decided to include the kube-apiextensions-server inside of kube-apiserver (https://github.com/kubernetes/community/blob/master/sig-api-machinery/api-extensions-position-statement.md#q-should-kube-aggregator-be-a-separate-binaryprocess-than-kube-apiserver). With the API reasonably well established and a finalizer about merge, I think its time to add ourselves.

This pull wires kube-apiextensions-server ahead of the TPRs so that one will replace the other if both are added by accident (CRDs should have priority) and wires a controller for automatic aggregation.

WIP because I still need tests: unit test for controller, test-cmd test to mirror the TPR test.

Adds the `CustomResourceDefinition` (crd) types to the `kube-apiserver`.  These are the successors to `ThirdPartyResource`.  See https://github.com/kubernetes/community/blob/master/contributors/design-proposals/thirdpartyresources.md for more details.

k8s-github-robot · 2017-05-18T21:10:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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

~~cmd/kube-apiserver/OWNERS~~ [deads2k]
~~pkg/master/OWNERS~~ [deads2k]
~~staging/src/k8s.io/kube-apiextensions-server/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

sttts · 2017-05-19T14:59:19Z

cmd/kube-apiserver/app/apiextensions.go

+	genericConfig.OpenAPIConfig = nil
+	genericConfig.SwaggerConfig = nil
+
+	// copy the loopbackclientconfig.  We're going to change the contenttype back to json until we get protobuf serializations for it


what's the issue here?

as discussed in irc, this is a follow-up

sttts · 2017-05-19T15:02:04Z

cmd/kube-apiserver/app/server.go

+		return err
+	}
+
+	// TPRs are enabled and not yet beta, since this these are the successor, they fall under the same rule


What do you mean here? the same rule = they are enabled by default?

sttts · 2017-05-19T15:09:55Z

pkg/master/thirdparty/tprregistration_controller.go

@@ -61,14 +66,18 @@ type tprRegistrationController struct {

 // NewAutoRegistrationController returns a controller which will register TPR GroupVersions with the auto APIService registration
 // controller so they automatically stay in sync.
-func NewAutoRegistrationController(tprInformer informers.ThirdPartyResourceInformer, apiServiceRegistration AutoAPIServiceRegistration) *tprRegistrationController {
+// In order to stay sane with both TPR and CRD present, we have a single controlle that manages both.  When choosing whether to have an


sttts · 2017-05-19T15:39:25Z

Looks good so far. We need some integration or even e2e tests for this.

deads2k · 2017-05-19T15:50:57Z

Looks good so far. We need some integration or even e2e tests for this.

You mean distinct from the integration tests for the apiextensions-server?

enisoc · 2017-05-19T19:38:08Z

cmd/kube-apiserver/app/server.go

+
+	// TPRs are enabled and not yet beta, since this these are the successor, they fall under the same enablement rule
+	// Subsequent API servers in between here and kube-apiserver will need to be gated.
+	// These come first so that if someone registers both a TPR and a CRD, the CRD is preferred.


What is your rationale for preferring CRD? I'm thinking of a scenario like this:

TPR already exists and is being used.

Upgrade to 1.7.

Accidentally create CRD that conflicts.

Effectively all your TPR data just got deleted. Outage ensues.

I would think then it actually makes more sense to let TPR have precedence over CRD. What do you think?

What is your rationale for preferring CRD? I'm thinking of a scenario like this:

TPR already exists and is being used.
Upgrade to 1.7.
Accidentally create CRD that conflicts.
Effectively all your TPR data just got deleted. Outage ensues.
I would think then it actually makes more sense to let TPR have precedence over CRD. What do you think?

I wanted to prefer the more current API and it makes finalization logic simpler. Otherwise we'll be considering non-REST API approaches to finalization to avoid deleting all your TPR instances.

We do have a spot that sits above both APIs and could have knowledge of them to avoid collisions. An admission plugin could be written to prevent conflicts between these.

deads2k · 2017-05-19T20:30:27Z

Added tests, this is ready for review.

sttts · 2017-05-22T07:40:46Z

hack/make-rules/test-cmd-util.sh

+    sleep ${wait}
+  done
+
+  kube::log::error "Timed out waiting for non-native-resources; tried ${times} waiting ${wait} between each"


sttts · 2017-05-22T07:45:05Z

Looks good so far. Some older comments still open.

deads2k · 2017-05-22T12:56:43Z

Looks good so far. Some older comments still open.

Got the typos.

sttts · 2017-05-22T13:03:20Z

Deserves a release note.

deads2k · 2017-05-22T13:06:55Z

Deserves a release note.

done

sttts · 2017-05-22T13:13:12Z

nit: why "logical"? They are the successor.

sttts · 2017-05-22T13:13:17Z

lgtm

enisoc

LGTM

deads2k · 2017-05-22T18:47:50Z

To catch this up to date with the migration conversation. This may move ordering with respect to the existing kube-apiserver overall, but it will still be included, so the "normal" client usage isn't affected.

k8s-github-robot · 2017-05-23T02:59:57Z

Automatic merge from submit-queue (batch tested with PRs 46022, 46055, 45308, 46209, 43590)

k8s-ci-robot · 2017-05-23T03:00:33Z

@deads2k: The following test(s) failed:

Test name	Commit	Details	Rerun command
pull-kubernetes-federation-e2e-gce	`446e959`	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

deads2k assigned sttts and enisoc May 18, 2017

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 18, 2017

k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels May 18, 2017

sttts reviewed May 19, 2017

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 19, 2017

sttts reviewed May 19, 2017

View reviewed changes

deads2k force-pushed the crd-01-embed branch from 619f028 to e968a8c Compare May 19, 2017 15:34

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 19, 2017

deads2k force-pushed the crd-01-embed branch 3 times, most recently from 73d79e7 to 22193fd Compare May 19, 2017 19:06

enisoc reviewed May 19, 2017

View reviewed changes

deads2k changed the title ~~[WIP] embed kube-apiextensions inside of kube-apiserver~~ embed kube-apiextensions inside of kube-apiserver May 19, 2017

sttts reviewed May 22, 2017

View reviewed changes

deads2k mentioned this pull request May 22, 2017

Proposal: Validation and Pre-Persist modifications for ThirdPartyResources #36885

Closed

deads2k added 2 commits May 22, 2017 08:53

embed apiextensions server into kube-apiserver

a637c49

make CRD apiservice controller

446e959

deads2k force-pushed the crd-01-embed branch from 22193fd to 446e959 Compare May 22, 2017 12:54

deads2k added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 22, 2017

deads2k removed the release-note-label-needed label May 22, 2017

enisoc approved these changes May 22, 2017

View reviewed changes

deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2017

k8s-github-robot merged commit bb56937 into kubernetes:master May 23, 2017

enisoc mentioned this pull request May 23, 2017

Temporary kube-apiextensions-server directions #45510

Closed

deads2k deleted the crd-01-embed branch August 3, 2017 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

embed kube-apiextensions inside of kube-apiserver #46055

embed kube-apiextensions inside of kube-apiserver #46055

deads2k commented May 18, 2017 •

edited

Loading

k8s-github-robot commented May 18, 2017

sttts May 19, 2017

sttts May 19, 2017

sttts May 19, 2017

sttts May 19, 2017

sttts May 19, 2017

sttts commented May 19, 2017

deads2k commented May 19, 2017

enisoc May 19, 2017

deads2k May 19, 2017

deads2k commented May 19, 2017

sttts May 22, 2017

sttts commented May 22, 2017

deads2k commented May 22, 2017

sttts commented May 22, 2017

deads2k commented May 22, 2017

sttts commented May 22, 2017

sttts commented May 22, 2017

enisoc left a comment

deads2k commented May 22, 2017

k8s-github-robot commented May 23, 2017

k8s-ci-robot commented May 23, 2017 •

edited

Loading

embed kube-apiextensions inside of kube-apiserver #46055

embed kube-apiextensions inside of kube-apiserver #46055

Conversation

deads2k commented May 18, 2017 • edited Loading

k8s-github-robot commented May 18, 2017

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

sttts commented May 19, 2017

deads2k commented May 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented May 19, 2017

Choose a reason for hiding this comment

sttts commented May 22, 2017

deads2k commented May 22, 2017

sttts commented May 22, 2017

deads2k commented May 22, 2017

sttts commented May 22, 2017

sttts commented May 22, 2017

enisoc left a comment

Choose a reason for hiding this comment

deads2k commented May 22, 2017

k8s-github-robot commented May 23, 2017

k8s-ci-robot commented May 23, 2017 • edited Loading

deads2k commented May 18, 2017 •

edited

Loading

k8s-ci-robot commented May 23, 2017 •

edited

Loading