-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix scaleKubernetesCluster API #11652
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.
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
Conversation
|
@blueorangutan package |
|
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11652 +/- ##
============================================
- Coverage 17.39% 17.39% -0.01%
- Complexity 15283 15284 +1
============================================
Files 5889 5889
Lines 526141 526184 +43
Branches 64234 64242 +8
============================================
+ Hits 91542 91544 +2
- Misses 424265 424296 +31
- Partials 10334 10344 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15053 |
|
Is this an issue with main branch only ? @harikrishna-patnala The smoke tests result of the PR #11598 looks good. Can you test scaling cks cluster with the same offering ? |
main and 4.21. I think this happened as part of forward merging 4.20 to main. 4.20 does not have this loop, so return statement is working there. |
Thanks @harikrishna-patnala Maybe we can remove the lines and add state transitions in case nothing happens if offering is same as before. Stopped -> OperationSucceeded -> Stopped and for Running state too |
|
@harikrishna-patnala, thanks for raising the PR.
Yes, exactly. #11598 worked fine in the 4.20 branch. However, the scale Kubernetes workflow now has an iteration for each possible node type. Thus, we should have been more cautious when porting it to the main branch. |
bernardodemarco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @weizhouapache mentioned (see #11652 (comment)), it'll be required to add the following transitions to avoid the throwing of exceptions when scaling Stopped and Running clusters to the same attributes they already have:
- From the
Stoppedstate, when it is received anOperationSucceededevent, transit to theStoppedstate - From the
Runningstate, when it is received anOperationSucceededevent, transit to theRunningstate
|
thanks for suggestions @weizhouapache and @bernardodemarco . I've added them and tested the scenario of scaling stopped and running k8s environment with same offering and observed no errors. |
|
@blueorangutan package |
|
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15094 |
|
@blueorangutan package |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
thanks @harikrishna-patnala for the fix
Pearl1594
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
bernardodemarco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm, thanks @harikrishna-patnala
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15106 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14399)
|
* Fix scaleKubernetesCluster * Added more state transitions
Description
This PR fixes the scaleKubernetesCluster API regression caused in main by #11598
Issue is that scaleKubernetesCluster API is not scaling the nodes on running k8s cluster. This is also causing smoke test failures
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?