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

feat: add option to override default kubernetes scheduler #2013

Merged
merged 8 commits into from May 18, 2023
Merged

feat: add option to override default kubernetes scheduler #2013

merged 8 commits into from May 18, 2023

Conversation

Adrixop95
Copy link
Contributor

@Adrixop95 Adrixop95 commented May 9, 2023 •
edited by mnencia

Added the ability to override the default k8s sheduler with a custom one.

Fixes #1863

@github-actions github-actions bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.18 release-1.19 release-1.20 labels May 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@Adrixop95
Copy link
Contributor Author

/test

phisco
phisco requested changes May 12, 2023
View reviewed changes
api/v1/cluster_types.go Outdated Show resolved Hide resolved
@phisco phisco self-requested a review May 13, 2023 08:08
phisco
phisco reviewed May 13, 2023
View reviewed changes
api/v1/cluster_types.go Outdated Show resolved Hide resolved
phisco
phisco approved these changes May 13, 2023
View reviewed changes
Copy link
Contributor

@phisco phisco left a comment

Choose a reason for hiding this comment

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

For some reason it looks like I'm you have a few already merged commits, so this should be rebased before merging, other than that, LGTM

@phisco
Copy link
Contributor

phisco commented May 13, 2023

/test

@github-actions
Copy link
Contributor

@phisco, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/4967618528

phisco
phisco requested changes May 13, 2023
View reviewed changes
Copy link
Contributor

@phisco phisco left a comment

Choose a reason for hiding this comment

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

I have to investigate the failures in the e2es

@phisco
Copy link
Contributor

phisco commented May 14, 2023

/test

@github-actions
Copy link
Contributor

@phisco, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/4973018285

@Adrixop95
Copy link
Contributor Author

/test

Hey, from what I can see, 2 test scenarios didn't execute correctly, it seems to me that this fail is not entirely related to this change, but some timeout/waiting for the cluster to recreate resources. Could you please confirm this and re-run the tests or guide me what might cause such a problem so I can try to fix it?

@armru
Copy link
Member

armru commented May 17, 2023

/test limit=local

@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/5001495750

@mnencia
Copy link
Member

mnencia commented May 17, 2023

I would instead prefer to have a rollout when we change the scheduler. The rationale is that if you break the pod configuration, it is better to immediately have a not-working replica and then notice it after months when you need to do a rollout for an unrelated change.

@armru
Copy link
Member

armru commented May 17, 2023

I agree with @mnencia

@armru
Copy link
Member

armru commented May 17, 2023

/test limit=local

@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/5004044802

Adrixop95 and others added 7 commits May 18, 2023 10:55
@Adrixop95 @armru
add option to change default k8s sheduler
198c9b4 
Signed-off-by: adrixop95 <adrixop95@me.com>
@Adrixop95 @armru
add kubebuilder default setting to sheduler type
c76fdac 
Signed-off-by: adrixop95 <adrixop95@me.com>
@Adrixop95 @armru
regenrate docs, CRDs and update description
94ec2f4 
Signed-off-by: adrixop95 <adrixop95@me.com>
@phisco @armru
review: fix default and made message similar to upstream description
d562512 
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
@armru
chore: review
4f4a12e 
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@armru
feat: execute a rollout on scheduler change
aaa9a71 
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@armru
fix: ensure rollout is triggered only when needed
f2d7a1e 
This patch ensures that we trigger the rollout only needed, the only drawback is that we cannot evaluate the empty string

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@armru
Copy link
Member

armru commented May 18, 2023

/test limit=local

@github-actions
Copy link
Contributor

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/5012278137

@github-actions github-actions bot added the ok to merge 👌 This PR can be merged label May 18, 2023
@mnencia
chore: remove leftover status field
924235a 
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia mnencia merged commit 50113b8 into cloudnative-pg:main May 18, 2023
21 of 22 checks passed
cnpg-bot pushed a commit that referenced this pull request May 18, 2023
@Adrixop95
feat: add option to override default kubernetes scheduler (#2013)
961cf03 
Add the `.spec.schedulerName` field to specify what scheduler should dispatch
the Cluster pods.

This patch ensures that we trigger a rollout of the pods when the scheduler changes;
the only limitation is that no rollout will be started by setting the field empty.

Signed-off-by: adrixop95 <adrixop95@me.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
(cherry picked from commit 50113b8)
cnpg-bot pushed a commit that referenced this pull request May 18, 2023
@Adrixop95
feat: add option to override default kubernetes scheduler (#2013)
b657b42 
Add the `.spec.schedulerName` field to specify what scheduler should dispatch
the Cluster pods.

This patch ensures that we trigger a rollout of the pods when the scheduler changes;
the only limitation is that no rollout will be started by setting the field empty.

Signed-off-by: adrixop95 <adrixop95@me.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
(cherry picked from commit 50113b8)
cnpg-bot pushed a commit that referenced this pull request May 18, 2023
@Adrixop95
feat: add option to override default kubernetes scheduler (#2013)
794a158 
Add the `.spec.schedulerName` field to specify what scheduler should dispatch
the Cluster pods.

This patch ensures that we trigger a rollout of the pods when the scheduler changes;
the only limitation is that no rollout will be started by setting the field empty.

Signed-off-by: adrixop95 <adrixop95@me.com>
Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Philippe Scorsolini <p.scorsolini@gmail.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
(cherry picked from commit 50113b8)
@Adrixop95 Adrixop95 deleted the dev/1863 branch May 18, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnencia mnencia mnencia approved these changes

@phisco phisco phisco approved these changes

@armru armru armru approved these changes

@fcanovai fcanovai Awaiting requested review from fcanovai

@gbartolini gbartolini Awaiting requested review from gbartolini

@leonardoce leonardoce Awaiting requested review from leonardoce

@sxd sxd Awaiting requested review from sxd

Assignees
No one assigned
Labels
backport-requested ◀️ This pull request should be backported to all supported releases ok to merge 👌 This PR can be merged release-1.18 release-1.19 release-1.20
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to override default kubernetes scheduler
4 participants
@Adrixop95 @phisco @armru @mnencia