Deploy statefulset as component. #365

abdasgupta · 2022-07-04T06:19:57Z

How to categorize this PR?

/area robustness
/kind enhancement

What this PR does / why we need it:
This PR converts the statefulset yaml to client-go code to create ETCD statefulset programmatically. Earlier an example yaml was decoded and merged with correct values to create statefulset yaml. But this PR removes the use of yaml. Instead, it fetches values from ETCD spec and directly creates statefulset using client-go for kubernetes API.

This PR also removes the logic for claiming statefulset based on label selector.

Which issue(s) this PR fixes:
Fixes part of #278

Special notes for your reviewer:

Release note:

Deploying the etcd StatefulSet through a Helm chart has been abandoned. A codified version (component concept) is now used for this purpose.

`etcd` Statefulsets are not claimed anymore based on labels. Instead, the statefulsets are fetched using Name and Namespace combination. Thus, `etcd.spec.selector` does not have an effect on statefulsets anymore.

gardener-robot · 2022-07-04T06:20:02Z

@abdasgupta Labels area/todo, kind/todo do not exist.

controllers/etcd_controller.go

aaronfern

Please remove this function as it is not used anymore

timuthy · 2022-07-27T11:43:25Z

@gardener/etcd-druid-maintainers the PR is ready for review.

pkg/component/etcd/statefulset/values.go

pkg/component/etcd/statefulset/statefulset.go

pkg/utils/miscellaneous.go

controllers/controller_ref_manager.go

controllers/etcd_controller.go

unmarshall · 2022-07-29T06:21:44Z

controllers/etcd_controller.go

-	leaseDeployer := componentlease.New(r.Client, etcd.Namespace, val.Lease)
+	if etcd.Spec.Etcd.Image == nil {
+		if etcdImage == "" {
+			return nil, nil, fmt.Errorf("either etcd resource or image vector should have %s image while deploying statefulset", common.Etcd)


This will result in re-queueing of request and since the image is now nil this will keep getting re-queued right?

Actually it'll only be re-queued if the images are nil and etcd or etcd-backup-restore images cannot be found in charts/images.yaml, see

etcd-druid/controllers/etcd_controller.go

Line 992 in 954f6ef

images, err = imagevector.FindImages(im, imageNames)

So if the images cannot be found, re-queue will also never help, so continuously re-queueing will block on worker all the time with no real advantage. Is there any alert raised?

Yes, you are right. I added a TODO that this is supposed to be part of a validating webhook. No, there is no alert but the error message is put to etcd.status.lastError.

unmarshall · 2022-07-29T06:22:08Z

controllers/etcd_controller.go

+
+	if etcd.Spec.Backup.Image == nil {
+		if etcdBackupImage == "" {
+			return nil, nil, fmt.Errorf("either etcd resource or image vector should have %s image while deploying statefulset", common.BackupRestore)


This will result in re-queueing of request and since the image is now nil this will keep getting re-queued right?

timuthy · 2022-07-29T08:21:08Z

Thanks for your valuable feedback @unmarshall. I addressed most of the points in 9d31a98 and replied to the rest of the comments.

unmarshall

/lgtm

Comments resolved.

abdasgupta requested a review from a team as a code owner July 4, 2022 06:19

gardener-robot added the needs/review Needs review label Jul 4, 2022

abdasgupta marked this pull request as draft July 4, 2022 06:20

gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Jul 4, 2022

abdasgupta force-pushed the quorum-loss branch from 361ba9c to 094458c Compare July 5, 2022 08:25

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 Jul 5, 2022

abdasgupta force-pushed the quorum-loss branch from 094458c to c8e5b04 Compare July 7, 2022 08:13

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 Jul 7, 2022

gardener-robot-ci-3 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 Jul 7, 2022

abdasgupta force-pushed the quorum-loss branch from c8e5b04 to e8a9406 Compare July 7, 2022 20:00

gardener-robot-ci-2 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 Jul 7, 2022

gardener-robot-ci-3 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 Jul 7, 2022

abdasgupta force-pushed the quorum-loss branch from e8a9406 to 8d7c949 Compare July 11, 2022 08:30

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 Jul 11, 2022

gardener-robot-ci-3 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 Jul 11, 2022

abdasgupta force-pushed the quorum-loss branch from 8d7c949 to 139817d Compare July 13, 2022 06:31

gardener-robot-ci-3 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 Jul 13, 2022

abdasgupta marked this pull request as ready for review July 13, 2022 06:32

gardener-robot-ci-3 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 Jul 13, 2022

aaronfern reviewed Jul 14, 2022

View reviewed changes

controllers/etcd_controller.go Outdated Show resolved Hide resolved

controllers/etcd_controller.go Outdated Show resolved Hide resolved

aaronfern previously requested changes Jul 15, 2022

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Jul 15, 2022

abdasgupta force-pushed the quorum-loss branch from 139817d to cc812fb Compare July 15, 2022 08:45

gardener-robot-ci-3 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 Jul 15, 2022

Add test for storage providers

f14b520

timuthy force-pushed the quorum-loss branch from 0016f44 to f14b520 Compare July 27, 2022 06:48

gardener-robot-ci-3 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 Jul 27, 2022

Increase worker count for test

b8500a1

gardener-robot-ci-2 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 Jul 27, 2022

gardener-robot-ci-1 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 Jul 27, 2022

Increase wait timeout for StatefulSet

954f6ef

gardener-robot-ci-3 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 Jul 28, 2022

timuthy mentioned this pull request Jul 28, 2022

Implement "component" concept for all the K8s resources #278

Closed

8 tasks

unmarshall reviewed Jul 29, 2022

View reviewed changes

Address review comments

9d31a98

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 Jul 29, 2022

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 Jul 29, 2022

unmarshall approved these changes Aug 1, 2022

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Aug 1, 2022

timuthy approved these changes Aug 3, 2022

View reviewed changes

timuthy merged commit de66db1 into gardener:master Aug 3, 2022

gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Aug 3, 2022

timuthy mentioned this pull request Aug 3, 2022

StatefulSet component follow ups #393

Merged

ashwani2k mentioned this pull request Aug 4, 2022

[Feature] Handle quorum loss scenario by Druid in ETCD multinode #362

Closed

timuthy mentioned this pull request Aug 17, 2022

[BUG] Hostpath not configureable via backup secret #407

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy statefulset as component. #365

Deploy statefulset as component. #365

abdasgupta commented Jul 4, 2022 •

edited by timuthy

Loading

gardener-robot commented Jul 4, 2022

aaronfern left a comment

timuthy commented Jul 27, 2022 •

edited

Loading

unmarshall Jul 29, 2022

timuthy Jul 29, 2022

unmarshall Jul 30, 2022

timuthy Aug 3, 2022

unmarshall Jul 29, 2022

timuthy commented Jul 29, 2022

unmarshall left a comment

Deploy statefulset as component. #365

Deploy statefulset as component. #365

Conversation

abdasgupta commented Jul 4, 2022 • edited by timuthy Loading

gardener-robot commented Jul 4, 2022

aaronfern left a comment

Choose a reason for hiding this comment

timuthy commented Jul 27, 2022 • edited Loading

unmarshall Jul 29, 2022

Choose a reason for hiding this comment

timuthy Jul 29, 2022

Choose a reason for hiding this comment

unmarshall Jul 30, 2022

Choose a reason for hiding this comment

timuthy Aug 3, 2022

Choose a reason for hiding this comment

unmarshall Jul 29, 2022

Choose a reason for hiding this comment

timuthy commented Jul 29, 2022

unmarshall left a comment

Choose a reason for hiding this comment

abdasgupta commented Jul 4, 2022 •

edited by timuthy

Loading

timuthy commented Jul 27, 2022 •

edited

Loading