Add Patch #44

johnsonj · 2019-05-08T21:17:29Z

Introduce Patch. An Addon can implement the Patchable interface to
accept a set of Patches that are overlaid on the deployment. For
example:

apiVersion: addons.sigs.k8s.io/v1alpha1
kind: Dashboard
metadata:
  name: dashboard-sample
  namespace: kube-system
spec:
  patches:
  - apiVersion: apps/v1beta2
    kind: Deployment
    metadata:
      name: kubernetes-dashboard
      namespace: kube-system
    spec:
      replicas: 5

This patch will change the replica count of the dashboard deployment.
This allows us to provide a break-glass experience for the user to set
configuration options while not requiring every configuration option be
directly exposed on the CRD.

It's not clear if we want this on all addons. If so, we should
graduate it to CommonObject
This patching is equivalent/borrowed from kustomize which is not
currently avaliable as a library.

johnsonj · 2019-05-08T21:17:36Z

/assign @justinsb

k8s-ci-robot · 2019-05-08T21:17:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnsonj

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:

~~OWNERS~~ [johnsonj]

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

stealthybox · 2019-05-13T17:23:50Z

Would we want to be able to specify kustomize bases as well?

This would allow you to check your overlays/patches into source control and use overlays that are distributed via channels or the operator's file system.

johnsonj · 2019-05-15T18:29:08Z

That seems interesting @stealthybox!

I think we need to think about if the patches on the CRD are the 'final assembly' or the a higher level (declaration of the intent?). If they're the sort of final instructions, then I think we can keep patch as simple as possible. If they're higher level then should we just completely accept a kustomization field?

I guess we should think about: 'would a user patch the CRD with kustomize' or 'is this CRD an interface into kustomize'?

justinsb · 2019-05-21T08:36:32Z

examples/dashboard-operator/pkg/controller/dashboard/dashboard_controller.go

 		declarative.WithObjectTransform(addon.TransformApplicationFromStatus),
+		declarative.WithManagedApplication(srcLabels),
+		declarative.WithObjectTransform(addon.ApplyPatches),


It is going to be weird if the field is there, but the patches don't actually do anything. Maybe this should be the default?

This is necessary today because the addons package defines the types, so the declarative package can't extract the patch out. We could workaround this with reflection.

I think what we really need is an AddonsReconciler that applies all (or most?) of these transforms.

justinsb · 2019-05-21T08:38:42Z

pkg/patterns/declarative/application.go

+	for _, gk := range uniqueGroupKind(objects) {
+		componentGroupKinds = append(componentGroupKinds, map[string]interface{}{"group": gk.Group, "kind": gk.Kind})
+	}
+	app.SetNestedSlice(componentGroupKinds, "spec", "componentGroupKinds")


Looks good, though why are we doing this? Just to break the dependency on metav1?

I had to do this because the actual metav1.LabelSelector{} couldn't be deep copied (which is why SetNestedFieldNoCopy) was used. That version didn't properly set these values (but the API server accepted it because the CRD was basically untyped).

justinsb · 2019-05-21T12:49:32Z

This lgtm.

On the base question, I think we can consider that separately from this PR @stealthybox ? My guess is that the base should be defined by the operator itself - or maybe by a version/channel field in the object, and that we wouldn't expect to allow arbitrary bases. But that's just my guess!

justinsb · 2019-06-24T15:56:15Z

Thanks @johnsonj

/lgtm

johnsonj · 2019-06-24T16:13:47Z

/test pull-declarative-test

Introduce Patch. An Addon can implement the Patchable interface to accept a set of Patches that are overlaid on the deployment. For example: apiVersion: addons.sigs.k8s.io/v1alpha1 kind: Dashboard metadata: name: dashboard-sample namespace: kube-system spec: patches: - apiVersion: apps/v1beta2 kind: Deployment metadata: name: kubernetes-dashboard namespace: kube-system spec: replicas: 5 This patch will change the replica count of the dashboard deployment. This allows us to provide a break-glass experience for the user to set configuration options while not requiring every configuration option be directly exposed on the CRD. - It's not clear if we want this on all addons. If so, we should graduate it to CommonObject - Patch doesn't work if it happens after the addons.TransformApplicationFromStatus transform. It raises a DeepCopy error. This is a bug. - This patching is equivalent/borrowed from kustomize which is not currently avaliable as a library.

Change the mutation of Application to be based on map[string]interface{} instead of using k8s types (like GroupKind). The k8s types do not properly deserialize.

to match kubernetes-sigs#45

johnsonj · 2019-06-24T16:44:06Z

/test pull-declarative-test

johnsonj · 2019-06-24T17:01:31Z

/ok-to-test

johnsonj · 2019-06-24T17:07:56Z

/retest

justinsb · 2019-06-24T17:17:03Z

Thanks for rebasing & fixing tests!

/lgtm

Based on functionality added in kubernetes-sigs/kubebuilder-declarative-pattern#44

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

k8s-ci-robot assigned justinsb May 8, 2019

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label May 8, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 8, 2019

k8s-ci-robot requested review from droot and justinsb May 8, 2019 21:17

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 8, 2019

johnsonj mentioned this pull request May 8, 2019

Strawman coredns: Add Patch justinsb/cluster-addons#1

Open

justinsb reviewed May 21, 2019

View reviewed changes

stealthybox mentioned this pull request Jun 14, 2019

Kustomize Patch Support for coredns-operator base manifests kubernetes-sigs/cluster-addons#12

Closed

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

johnsonj added 5 commits June 24, 2019 16:38

vendor: vendor/github.com/evanphx/json-patch

0b94df1

declarative: fix issues with serialization of Application

67ac78e

Change the mutation of Application to be based on map[string]interface{} instead of using k8s types (like GroupKind). The k8s types do not properly deserialize.

addon: add deep-copy gen for PatchSpec

eab5c4b

update URI to addons.example.org

40281cd

to match kubernetes-sigs#45

johnsonj force-pushed the add-patch branch from b477ef3 to 40281cd Compare June 24, 2019 16:43

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

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jun 24, 2019

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

k8s-ci-robot merged commit 3bfb586 into kubernetes-sigs:master Jun 24, 2019

johnsonj added a commit to johnsonj/addon-operators that referenced this pull request Jun 24, 2019

Add Patch to CoreDNS

d5c90b7

Based on functionality added in kubernetes-sigs/kubebuilder-declarative-pattern#44

johnsonj added a commit to johnsonj/addon-operators that referenced this pull request Jun 24, 2019

Add Patch to CoreDNS

c58cfd4

Based on functionality added in kubernetes-sigs/kubebuilder-declarative-pattern#44

johnsonj mentioned this pull request Jun 24, 2019

Add Patch to CoreDNS kubernetes-sigs/cluster-addons#18

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Patch #44

Add Patch #44

johnsonj commented May 8, 2019 •

edited

Loading

johnsonj commented May 8, 2019

k8s-ci-robot commented May 8, 2019

stealthybox commented May 13, 2019

johnsonj commented May 15, 2019

justinsb May 21, 2019

johnsonj Jun 14, 2019

justinsb May 21, 2019

johnsonj Jun 14, 2019 •

edited

Loading

justinsb commented May 21, 2019

justinsb commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

justinsb commented Jun 24, 2019

Add Patch #44

Add Patch #44

Conversation

johnsonj commented May 8, 2019 • edited Loading

johnsonj commented May 8, 2019

k8s-ci-robot commented May 8, 2019

stealthybox commented May 13, 2019

johnsonj commented May 15, 2019

justinsb May 21, 2019

Choose a reason for hiding this comment

johnsonj Jun 14, 2019

Choose a reason for hiding this comment

justinsb May 21, 2019

Choose a reason for hiding this comment

johnsonj Jun 14, 2019 • edited Loading

Choose a reason for hiding this comment

justinsb commented May 21, 2019

justinsb commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

johnsonj commented Jun 24, 2019

justinsb commented Jun 24, 2019

johnsonj commented May 8, 2019 •

edited

Loading

johnsonj Jun 14, 2019 •

edited

Loading