Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add SchedulingConstraints to Etcd spec #329

Merged
merged 4 commits into from
Apr 11, 2022
Merged

Add SchedulingConstraints to Etcd spec #329

merged 4 commits into from
Apr 11, 2022

Conversation

shreyas-s-rao
Copy link
Contributor

@shreyas-s-rao shreyas-s-rao commented Apr 8, 2022 •
edited
Loading

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

How to categorize this PR?

/area high-availability
/kind enhancement

What this PR does / why we need it:
This PR adds support for adding scheduling constraints to the etcd pod(s), via the SchedulingConstraints field in the Etcd template. The currently supported constraints are Affinity and TopologySpreadConstraints. This is required by gardener/gardener#5741 .

Which issue(s) this PR fixes:
Fixes partially #107

Special notes for your reviewer:
cc @abdasgupta @unmarshall @timuthy @ashwani2k

Release note:

Add `SchedulingConstraints` field to Etcd spec. The currently supported constraints are `Affinity` and `TopologySpreadConstraints`.

@shreyas-s-rao
Add SchedulingConstraints to etcd spec
a774a34 
Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>
@shreyas-s-rao shreyas-s-rao requested a review from a team as a code owner April 8, 2022 12:22
@gardener-robot gardener-robot added area/high-availability High availability related kind/enhancement Enhancement, improvement, extension needs/review Needs review 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 Apr 8, 2022
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) 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 8, 2022
@shreyas-s-rao
Update Etcd example resource
b9e0319 
Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>
@gardener-robot-ci-2 gardener-robot-ci-2 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 8, 2022
@shreyas-s-rao
Run make generate and make check-generate
adaf4e9 
Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>
@gardener-robot-ci-2 gardener-robot-ci-2 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 8, 2022
abdasgupta
abdasgupta suggested changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@abdasgupta abdasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I posted just one doubt as review. Also, unit test is failing

charts/etcd/templates/etcd-statefulset.yaml Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Apr 8, 2022
@shreyas-s-rao shreyas-s-rao 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 9, 2022
@gardener-robot-ci-3 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 Apr 9, 2022
@shreyas-s-rao
Copy link
Contributor Author

@abdasgupta the test has passed now. It was from an unrelated error.

aaronfern
aaronfern requested changes Apr 9, 2022
View reviewed changes
Copy link
Contributor

@aaronfern aaronfern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
Looks good, just one small nit

config/samples/druid_v1alpha1_etcd.yaml Outdated Show resolved Hide resolved
@shreyas-s-rao @aaronfern
Address review comments
edafef4 
Co-authored-by: Aaron Francis Fernandes <79958509+aaronfern@users.noreply.github.com>
@gardener-robot-ci-3 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 Apr 9, 2022
@shreyas-s-rao
Copy link
Contributor Author

@aaronfern thank you so much for the review. I've fixed it now. PTAL

@gardener-robot-ci-1 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 Apr 9, 2022
@abdasgupta
Copy link
Contributor

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Apr 11, 2022
@gardener-robot gardener-robot removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Apr 11, 2022
@aaronfern aaronfern merged commit 1830cb8 into gardener:master Apr 11, 2022
@shreyas-s-rao shreyas-s-rao deleted the feature/scheduling-policies branch April 11, 2022 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unmarshall unmarshall unmarshall left review comments

@abdasgupta abdasgupta abdasgupta approved these changes

@aaronfern aaronfern aaronfern approved these changes

Assignees
No one assigned
Labels
area/high-availability High availability related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@shreyas-s-rao @abdasgupta @unmarshall @aaronfern @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot