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

🐛[KCP] fix for machine selection logic during scale down #2768

Merged
merged 1 commit into from
Mar 25, 2020
Merged

🐛[KCP] fix for machine selection logic during scale down #2768

merged 1 commit into from
Mar 25, 2020

Conversation

sedefsavas
Copy link

@sedefsavas sedefsavas commented Mar 24, 2020 •
edited
Loading

What this PR does / why we need it:
Select the failure domain with most machines in a smaller set of machines instead of all controlplane machines.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2760

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 24, 2020
@sedefsavas sedefsavas closed this Mar 24, 2020
@k8s-ci-robot
Copy link
Contributor

@sedefsavas: Closed this PR.

In response to this:

wrong repo sorry
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sedefsavas sedefsavas changed the title Fix for 2760 [WIP]🐛[KCP] fix for machine selection logic during scale down Mar 24, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2020
@sedefsavas
Copy link
Author

/reopen

@k8s-ci-robot
Copy link
Contributor

@sedefsavas: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot reopened this Mar 24, 2020
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 24, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 24, 2020
@sedefsavas sedefsavas changed the title [WIP]🐛[KCP] fix for machine selection logic during scale down 🐛[KCP] fix for machine selection logic during scale down Mar 24, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2020
@sedefsavas
Copy link
Author

@vincepri @wfernandes @detiber
This PR fixes e2e issues. Can we target to add this to the next release?

@sedefsavas sedefsavas mentioned this pull request Mar 24, 2020
Closed
JoelSpeed
JoelSpeed reviewed Mar 25, 2020
View reviewed changes
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 25, 2020
@wfernandes
Copy link
Contributor

Currently reviewing and running tests locally.

@wfernandes
Copy link
Contributor

After running make test-capd-e2e, the Basic create test was the only one that ran and it passed on my machine.

The Full upgrade test didn't run.

I'm guessing this is intended behavior for now since we have yet to add the other component upgrades (coredns, etcd, kube-proxy) as part of the Full upgrade test as discussed in #2753

@wfernandes
Copy link
Contributor

/lgtm

@wfernandes
Copy link
Contributor

@detiber @vincepri for approval.