Move bootstrap script out of etcd-druid #162

shreyas-s-rao · 2021-04-20T17:45:05Z

Signed-off-by: Shreyas Rao shreyas.sriganesh.rao@sap.com

How to categorize this PR?

/area backup
/kind enhancement
/priority blocker

What this PR does / why we need it:
This PR moves bootstrap script out of etcd-druid. The bootstrap will now reside in the custom etcd image itself, to avoid the out-of-sync configmap and statefulset spec issue described by gardener-attic/gardener-resource-manager#104.

Which issue(s) this PR fixes:
Fixes #159

Special notes for your reviewer:
Druid now passes config to the bootstrap script by setting environment variables in the etcd container.
Older etcd versions (v3.4.13, v3.4.14 and v3.5.0-alpha.0) have been "backported" by manually building new container images of these with the built-in bootstrap script, tagged v3.4.13-bootstrap, v3.4.14-bootstrap and v3.5.0-alpha.0-bootstrap respectively. Please test these new, manually built images before merging.

Release note:

Etcd bootstrap script now resides in the custom etcd image instead of being mounted as a configmap.

shreyas-s-rao · 2021-04-20T18:04:14Z

Corresponding PR on etcd-custom image has been created here -> gardener/etcd-custom-image#1

ishan16696 · 2021-04-21T04:48:22Z

Hi @shreyas-s-rao ,
So you are just moving out the Etcd bootstrap script only, not the etcd.conf.yaml which is also passed as config map right?
I am asking this because I am making auto-compaction configurable in this PR. So, just take care of which PR should go first

shreyas-s-rao · 2021-04-21T05:24:18Z

@ishan16696 only the bootstrap script (bootstrap.sh) moves out, while etcd config (etcd.conf.yaml) stays within druid charts, so your PR will not be affected. Also, PRs will be merged in order of creation, so you need not rebase over my PR.

shreyas-s-rao · 2021-04-22T18:54:35Z

@abdasgupta made few changes to the PR. Please re-review the code changes. Thanks

amshuman-kr

Can you please clean up the image versions? Also, a question about tests below.

amshuman-kr · 2021-04-23T06:01:59Z

api/v1alpha1/etcd_types_test.go

-	imageEtcd := "eu.gcr.io/gardener-project/gardener/etcd:v3.4.13"
-	imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.11.1"
+	imageEtcd := "eu.gcr.io/gardener-project/gardener/etcd:v3.4.13-bootstrap"
+	imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"


Suggested change

imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"

imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0"

Did not want to change these yet, as etcdbrctl:v0.12.0 hasn't been released yet, and testing in local gardener setup becomes cumbersome since the etcd resource deployed per control plane doesn't specify image versions, so the default image versions are used, and changing it becomes difficult as it gets reconciled unless we turn off gardenlet, hence made these image versions point to latest etcdbr version.
Once we make etcdbr release, all these image versions will be changed to reflect that. Hope that's ok.

amshuman-kr · 2021-04-23T06:13:38Z

charts/etcd/templates/etcd-statefulset.yaml

+        - name: ENABLE_TLS
+          value: {{ .Values.etcd.enableTLS }}
+        - name: BACKUP_ENDPOINT
+          value: "http{{ if .Values.etcd.enableTLS }}s{{ end }}://{{ .Values.name }}-local:{{ .Values.backup.port }}"
+        - name: FAIL_BELOW_REVISION_PARAMETER
+          value: "{{ if .Values.backup.failBelowRevision }}&failbelowrevision={{ int $.Values.backup.failBelowRevision }}{{ end }}"


Just for my information, what are these for?

Since bootstrap script will now reside on etcd custom image, we need a way to pass some "configurable" parts of the bootstrap script to the etcd container, like backup endpoint and whether tls is enabled or not (to set the root CA certs). Hence passing them as env vars. They can also be mounted as configmap values, but env vars seemed like the simpler option. You can check the current (old) bootstrap script to see all the config that's coming in from the chart's values.yaml file

amshuman-kr · 2021-04-23T06:25:21Z

config/samples/druid_v1alpha1_etcd.yaml

@@ -29,7 +29,7 @@ spec:
        role: test
    etcd:
        metrics: basic
-        image: eu.gcr.io/gardener-project/gardener/etcd:v3.4.13
+        image: eu.gcr.io/gardener-project/gardener/etcd:v3.4.13-bootstrap


Shouldn't the etcdbrctl image version be changed too?

Yes, will make this change.

amshuman-kr · 2021-04-23T06:26:02Z

charts/images.yaml

@@ -2,8 +2,8 @@ images:
 - name: etcd-backup-restore
  sourceRepository: github.com/gardener/etcd-backup-restore
  repository: eu.gcr.io/gardener-project/gardener/etcdbrctl
-  tag: "v0.11.1"
+  tag: "v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"


Suggested change

tag: "v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"

tag: "v0.12.0"

amshuman-kr · 2021-04-23T06:31:09Z

controllers/etcd_controller_test.go

-	imageEtcd                     = "eu.gcr.io/gardener-project/gardener/etcd:v3.4.13"
-	imageBR                       = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.11.1"
+	imageEtcd                     = "eu.gcr.io/gardener-project/gardener/etcd:v3.4.13-bootstrap"
+	imageBR                       = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"


Suggested change

imageBR = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"

imageBR = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0"

I can't make this change yet since etcdbrctl:v0.12.0 has not yet been released, so the tests would fail. Have used the latest etcdbr version from master at the moment

amshuman-kr · 2021-04-23T06:32:42Z

controllers/etcd_controller_test.go

 						},
 						{
 							Name:  "backup-restore",
-							Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.11.1",
+							Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3",


Suggested change

Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3",

Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0",

controllers/etcd_controller_test.go

amshuman-kr · 2021-04-23T06:36:50Z

controllers/etcd_controller_test.go

 				},
 				{
 					Name:  "backup-restore",
-					Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.11.1",
+					Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3",


Suggested change

Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3",

Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0",

abdasgupta

LGTM

ishan16696 · 2021-04-26T11:23:52Z

LGTM

Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>

shreyas-s-rao added this to the v0.5.0 milestone Apr 20, 2021

shreyas-s-rao requested review from abdasgupta and amshuman-kr April 20, 2021 17:45

shreyas-s-rao requested a review from a team as a code owner April 20, 2021 17:45

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 20, 2021

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 20, 2021

shreyas-s-rao mentioned this pull request Apr 20, 2021

Add etcd bootstrap script to image gardener/etcd-custom-image#1

Merged

shreyas-s-rao added area/robustness Robustness, reliability, resilience related component/etcd-druid ETCD Druid labels Apr 20, 2021

shreyas-s-rao mentioned this pull request Apr 21, 2021

Move bootstrap script out of helm chart gardener/etcd-backup-restore#327

Merged

shreyas-s-rao force-pushed the move-bootstrap-script branch from 692d934 to 57cd875 Compare April 22, 2021 18:51

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021

amshuman-kr requested changes Apr 23, 2021

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Apr 23, 2021

shreyas-s-rao added the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Apr 23, 2021

abdasgupta approved these changes Apr 26, 2021

View reviewed changes

Move bootstrap script out of etcd-druid

2386ff1

Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>

shreyas-s-rao force-pushed the move-bootstrap-script branch from 57cd875 to 2386ff1 Compare April 26, 2021 13:04

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 26, 2021

abdasgupta approved these changes Apr 26, 2021

View reviewed changes

shreyas-s-rao merged commit 001726d into gardener:master Apr 26, 2021

shreyas-s-rao deleted the move-bootstrap-script branch April 26, 2021 13:19

dependabot bot mentioned this pull request May 19, 2021

Bump github.com/gardener/gardener from 1.22.1 to 1.23.0 afritzler/garden-universe#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move bootstrap script out of etcd-druid #162

Move bootstrap script out of etcd-druid #162

shreyas-s-rao commented Apr 20, 2021 •

edited

shreyas-s-rao commented Apr 20, 2021

ishan16696 commented Apr 21, 2021

shreyas-s-rao commented Apr 21, 2021

shreyas-s-rao commented Apr 22, 2021

amshuman-kr left a comment

amshuman-kr Apr 23, 2021

shreyas-s-rao Apr 23, 2021

amshuman-kr Apr 23, 2021

shreyas-s-rao Apr 23, 2021

amshuman-kr Apr 23, 2021

shreyas-s-rao Apr 23, 2021

amshuman-kr Apr 23, 2021

amshuman-kr Apr 23, 2021

shreyas-s-rao Apr 23, 2021

amshuman-kr Apr 23, 2021

amshuman-kr Apr 23, 2021

abdasgupta left a comment •

edited

ishan16696 commented Apr 26, 2021

	imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"
	imageBR := "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0"

	tag: "v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"
	tag: "v0.12.0"

	imageBR = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3"
	imageBR = "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0"

	Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0-dev-2cb339762d03d5cc9c78bda855fdf48f309de9c3",
	Image: "eu.gcr.io/gardener-project/gardener/etcdbrctl:v0.12.0",

Move bootstrap script out of etcd-druid #162

Move bootstrap script out of etcd-druid #162

Conversation

shreyas-s-rao commented Apr 20, 2021 • edited

shreyas-s-rao commented Apr 20, 2021

ishan16696 commented Apr 21, 2021

shreyas-s-rao commented Apr 21, 2021

shreyas-s-rao commented Apr 22, 2021

amshuman-kr left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdasgupta left a comment • edited

Choose a reason for hiding this comment

ishan16696 commented Apr 26, 2021

shreyas-s-rao commented Apr 20, 2021 •

edited

abdasgupta left a comment •

edited