Split manifest generation to produce both v1/v1beta1 CRDs #4489

pebrc · 2021-05-12T09:24:00Z

This PR moves all CRDs to apiVersion: v1 but keeps v1beta1 around for older Kubernetes versions that we still want to support (all the way back to 1.12 and OCP 3.11)

Upgrades from v1beta1 to v1 will be a bit tricky which is why I have marked this PR as breaking:

all-in-one.yaml users will have to replace the CRDs and not use kubectl apply because a meaningful merge of v1beta1 to v1 is not possible. There is however another gotcha with the webhook for which a replace is an illegal operation as it would remove an already populated clusterIP: failed to replace object: Service "elastic-operator-webhook" is invalid: spec.clusterIPs[0]: Invalid value: []string(nil): primary clusterIP can not be unset. A workaround is to just reapply the manifest. I therefore added a separate manifest with just the CRDs as a download.
Helm: a similar situation: users have to first upgrade the CRDs with the --force option to replace with v1 they should be able to follow this with a regular helm upgrade of the main chart.
OLM: in my testing it seems that OLM does the "right thing" and does correctly upgrade from v1beta1 to v1

Docs preview

https://cloud-on-k8s_4489.docs-preview.app.elstc.co/diff

Notes on the implementation:

this PR relies on controller-tools 0.6.0-beta which brings an important bugfix for embedded k8s runtime objects metadata
introduced global values in the Helm chart to allow access from the dependent CRD chart, this is a potential breaking change but the old values were marked as internal before as well so I guess it is reasonable to expect users to not rely on them
The size of the CRDs with schemas and multiple versions is significant (~ 1.6M) We can discuss whether it would make sense to drop the descriptions. So far I was able to apply the manifests with the description as the individual CRDs are still below the max limit.

Test scenarios:

OLM: build custom catalog image and upgrade to latest from a previously released version
Helm: v1beta1 to v1: install current chart and upgrade from this PR
Helm: verify future v1 to v1 upgrade: manually install v1 manifests and adopt them into a 1.6.0 release and try a regular helm upgrade without force
YAML: install operator 1.6.0 and upgrade to this PR
YAML: run our release upgrade test through multiple operator upgrades and finally upgrade to this PR

pebrc · 2021-05-14T09:20:29Z

run full pr build

pebrc · 2021-05-20T10:49:38Z

Marking this ready for review with the caveat that we have a dependency on #4491

…D chart

barkbay

Impressive work 👍
I only left a few nitpicks, I need some additional time to process and test the PR.

config/crds/v1/patches/elasticsearch-patches.yaml

Makefile

config/crds/v1/patches/ent-patches.yaml

config/crds/v1/patches/kustomization.yaml

docs/operating-eck/upgrading-eck.asciidoc

deploy/eck-operator/values.yaml

docs/operating-eck/upgrading-eck.asciidoc

Co-authored-by: Michael Morello <michael.morello@gmail.com>

barkbay

nit: there is a reference to the all in one here.

pebrc · 2021-06-10T14:17:18Z

nit: there is a reference to the all in one here.

Good catch, there is another one in the operatorhub tooling. Will fix!

barkbay

LGTM ! 🚀

docs/advanced-topics/service-meshes.asciidoc

hack/upgrade-test-harness/README.md

hack/operatorhub/main.go

pebrc · 2021-06-14T09:01:35Z

run full pr build

thbkrkr

Great work 👏
I left some minor and cosmetic changes.

Makefile

test/e2e/cmd/run/run.go

pkg/apis/elasticsearch/v1/elasticsearch_types.go

docs/operating-eck/installing-eck.asciidoc

docs/operating-eck/upgrading-eck.asciidoc

thbkrkr · 2021-06-14T12:27:42Z

config/crds/v1beta1/patches/agent-patches.yaml

@@ -25,3 +25,6 @@
  path: /spec/validation/openAPIV3Schema/properties/spec/properties/deployment/properties/strategy/properties/rollingUpdate/properties/maxUnavailable/x-kubernetes-int-or-string
 - op: remove
  path: /spec/validation/openAPIV3Schema/properties/spec/properties/daemonSet/properties/updateStrategy/properties/rollingUpdate/properties/maxUnavailable/x-kubernetes-int-or-string
+# we need to generate x-kubernetes-preserve-unknown-fields for v1 CRDs but they break v1beta so we have to remove them again here
+- op: remove
+  path: /spec/validation/openAPIV3Schema/properties/spec/properties/config/x-kubernetes-preserve-unknown-fields


Normal that we don't have the same count of x-kubernetes-preserve-unknown-fields and // +kubebuilder:pruning:PreserveUnknownFields?

Agent and Beat do not get:

- op: remove path: /spec/validation/openAPIV3Schema/properties/spec/properties/daemonSet/properties/podTemplate/x-kubernetes-preserve-unknown-fields - op: remove path: /spec/validation/openAPIV3Schema/properties/spec/properties/deployment/properties/podTemplate/x-kubernetes-preserve-unknown-fields

That's because we remove the whole podTemplate section for the v1beta1 version on the top of the file.

Of course.
That's not important but just for my understanding, we could do the same for the following patches where we also remove the whole podTemplate?

v1beta1/patches/apm-kibana-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/properties v1beta1/patches/apm-kibana-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/x-kubernetes-preserve-unknown-fields v1beta1/patches/elasticsearch-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/nodeSets/items/properties/podTemplate/properties v1beta1/patches/elasticsearch-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/nodeSets/items/properties/podTemplate/x-kubernetes-preserve-unknown-fields v1beta1/patches/ems-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/properties v1beta1/patches/ems-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/x-kubernetes-preserve-unknown-fields v1beta1/patches/ent-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/properties v1beta1/patches/ent-patches.yaml: path: /spec/validation/openAPIV3Schema/properties/spec/properties/podTemplate/x-kubernetes-preserve-unknown-fields

The way we remove the podTemplate differs a little bit between Agent and Beats and the other CRDs. Agent and Beats remove the whole podTemplate tree, while the other CRDs only remove the properties from the podTemplate section. This makes the other tweaking necessary. This is nothing this PR introduced but has been in place since the introduction of Beats and Agent. I kept it that way so that the legacy CRDs stay as close to the versions we currently ship as possible.

alaudazzi

A few minor suggestions for the text, otherwise LGTM.

docs/operating-eck/upgrading-eck.asciidoc

docs/operating-eck/webhook.asciidoc

Co-authored-by: alaudazzi <46651782+alaudazzi@users.noreply.github.com>

botelastic bot added the triage label May 12, 2021

pebrc force-pushed the api-version-split branch from 307f231 to af8cc1b Compare May 12, 2021 09:56

pebrc added v1.7.0 >breaking labels May 12, 2021

botelastic bot removed the triage label May 12, 2021

pebrc force-pushed the api-version-split branch 3 times, most recently from fde9062 to 1a493c5 Compare May 14, 2021 08:23

pebrc force-pushed the api-version-split branch from c148988 to 3f3654a Compare May 14, 2021 11:51

This was referenced May 14, 2021

Deprecation warnings shown when installing all-in-one (for K8s 1.16+) #3886

Closed

Move kind bootstrap into deployer #4491

Merged

pebrc marked this pull request as ready for review May 20, 2021 10:49

pebrc added 15 commits May 25, 2021 13:06

Split manifest generation to produce both v1/v1beta1 CRDs

71857c5

Run generate

c39a70a

Fix up make targets

ce7607a

Integration tests need v1beta1 CRDs until we upgrade kubebuilder

1bc02f6

Do not remove the complete podTemplate attribute

6c25edb

CI debugging

a0bf838

Move interal Helm values to globals to simplify use from dependent CR…

4547213

…D chart

Upload legacy CRDs

aa3fc0d

Make operatorhub tool compatible with v1 and v1beta1

8df2d72

First doc draft

2c07cc6

Inheriting from an empty kustomize base does not seem to work

cbed950

Generate a CRD only manifest to facilite upgrades

dfc170d

Fix yaml upload and adjust upgrade instructions

2cba9b6

Add license header

f85c264

remove debug bits

189ee86

pebrc force-pushed the api-version-split branch from e3bf85d to 189ee86 Compare May 25, 2021 11:09

Makefile cleanup

e777b1e

pebrc force-pushed the api-version-split branch from 0209439 to e777b1e Compare May 28, 2021 09:22

Update upgrade-test-harness to work with dedicated CRD manifest

76d9ee0

pebrc force-pushed the api-version-split branch from f082662 to 76d9ee0 Compare May 31, 2021 19:49

pebrc mentioned this pull request Jun 2, 2021

Upgrading instructions for ECK is not clear #4427

Closed

Add note about skipped releases

65d131a

barkbay reviewed Jun 8, 2021

View reviewed changes

barkbay reviewed Jun 9, 2021

View reviewed changes

deploy/eck-operator/values.yaml Show resolved Hide resolved

input from review

a3fc65b

barkbay reviewed Jun 9, 2021

View reviewed changes

docs/operating-eck/upgrading-eck.asciidoc Outdated Show resolved Hide resolved

docs/operating-eck/upgrading-eck.asciidoc Outdated Show resolved Hide resolved

pebrc and others added 2 commits June 9, 2021 13:27

Apply suggestions from code review

fd00d6d

Co-authored-by: Michael Morello <michael.morello@gmail.com>

Use release version of controller-gen

362b6e1

This was referenced Jun 9, 2021

Webhook certificate reconciliation relies on deprecated v1beta1 API #4555

Closed

OpenAPIV3 structural schema violations #2349

Closed

barkbay reviewed Jun 10, 2021

View reviewed changes

pebrc added 2 commits June 11, 2021 08:52

Replace references to all-in-one with new manifest names

7425e6a

Make operatorhub tool compatible with split install manifests

20769ac

barkbay approved these changes Jun 14, 2021

View reviewed changes

docs/advanced-topics/service-meshes.asciidoc Outdated Show resolved Hide resolved

hack/upgrade-test-harness/README.md Show resolved Hide resolved

hack/operatorhub/main.go Outdated Show resolved Hide resolved

hack/operatorhub/main.go Outdated Show resolved Hide resolved

review input

48aece1

thbkrkr approved these changes Jun 14, 2021

View reviewed changes

pebrc added 2 commits June 14, 2021 17:26

review input

4c560e7

review input

e930256

alaudazzi approved these changes Jun 15, 2021

View reviewed changes

docs/operating-eck/upgrading-eck.asciidoc Outdated Show resolved Hide resolved

docs/operating-eck/webhook.asciidoc Outdated Show resolved Hide resolved

Apply suggestions from code review

cadc6ee

Co-authored-by: alaudazzi <46651782+alaudazzi@users.noreply.github.com>

barkbay mentioned this pull request Jun 15, 2021

Detect admission registration API version #4569

Merged

pebrc merged commit 0f5313a into elastic:master Jun 15, 2021

lblackstone mentioned this pull request Oct 29, 2021

Cannot upgrade cert-manager due to errors updating the CRDs pulumi/pulumi-kubernetes#1293

Closed

Lucaber mentioned this pull request Dec 16, 2021

elasticsearch v1beta1 loosing configuration after upgrading eck-operator #5181

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split manifest generation to produce both v1/v1beta1 CRDs #4489

Split manifest generation to produce both v1/v1beta1 CRDs #4489

pebrc commented May 12, 2021 •

edited

Loading

pebrc commented May 14, 2021

pebrc commented May 20, 2021

barkbay left a comment

barkbay left a comment

pebrc commented Jun 10, 2021

barkbay left a comment

pebrc commented Jun 14, 2021

thbkrkr left a comment

thbkrkr Jun 14, 2021

pebrc Jun 14, 2021

thbkrkr Jun 14, 2021 •

edited

Loading

pebrc Jun 15, 2021

alaudazzi left a comment

Split manifest generation to produce both v1/v1beta1 CRDs #4489

Split manifest generation to produce both v1/v1beta1 CRDs #4489

Conversation

pebrc commented May 12, 2021 • edited Loading

Docs preview

Notes on the implementation:

Test scenarios:

pebrc commented May 14, 2021

pebrc commented May 20, 2021

barkbay left a comment

Choose a reason for hiding this comment

barkbay left a comment

Choose a reason for hiding this comment

pebrc commented Jun 10, 2021

barkbay left a comment

Choose a reason for hiding this comment

pebrc commented Jun 14, 2021

thbkrkr left a comment

Choose a reason for hiding this comment

thbkrkr Jun 14, 2021

Choose a reason for hiding this comment

pebrc Jun 14, 2021

Choose a reason for hiding this comment

thbkrkr Jun 14, 2021 • edited Loading

Choose a reason for hiding this comment

pebrc Jun 15, 2021

Choose a reason for hiding this comment

alaudazzi left a comment

Choose a reason for hiding this comment

pebrc commented May 12, 2021 •

edited

Loading

thbkrkr Jun 14, 2021 •

edited

Loading