-
Notifications
You must be signed in to change notification settings - Fork 47
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
Allow adding and deleting worker nodes #205
Conversation
Co-authored-by: Kuldip Nanda <kuldip.nanda@ibm.com> Signed-off-by: Yussuf Shaikh <yussuf.shaikh@ibm.com>
Signed-off-by: Yussuf Shaikh <yussuf.shaikh@ibm.com>
oc adm cordon worker-${count.index} | ||
oc adm drain worker-${count.index} --force --delete-local-data --ignore-daemonsets | ||
oc delete node worker-${count.index} | ||
EOF |
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.
Is my understanding correct that we can't delete a node by name specifically ?
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.
oc delete TYPE
works with (NAME | -l label | --all)
if that is what you are asking.
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.
My question was more on flexibility of deleting specific node. But I understand it now. Thanks
@@ -133,6 +133,11 @@ locals { | |||
} | |||
|
|||
resource "null_resource" "config" { | |||
|
|||
triggers = { | |||
worker_count = length(var.worker_ips) |
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.
Can you please explain this trigger ?
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.
Whenever there is a change in the trigger attributes the resource would be re-created after apply. Here whenever the count changes it will run the helpernode again. This is needed for changing the configuration for named, dhcp, etc.
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.
@yussufsh we are going to need this on kvm also. Is that on your list?
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.
I do not have a KVM box to implement this at this time. I am working on getting my test setup.
Only issue I am worried about is the use of module dependency we have used in kvm repo. Once I have a test setup I will be able to contribute this change there.
TEST ON OCP4.5.4Environment
Test
Initial Run
2nd Run - Increase Worker Count
3rd Run - Decrease Worker Count
4th Run - Increase Worker Count
5th Run - Increase Worker Count
|
Test for 4.6.0 - All Tests done on a new cluster (< 24 hrs.)Environment
TestInitial Run
2nd Run - Increase Worker Count
3rd Run - Decrease Worker Count
|
/cc @Prajyot-Parab |
TEST ON OCP4.5.23Environment
Test
Initial Run
Increase Worker Count
NAME STATUS ROLES AGE VERSION
master-0 Ready master 17h v1.18.3+fa69cae
master-1 Ready master 17h v1.18.3+fa69cae
master-2 Ready master 17h v1.18.3+fa69cae
worker-0 Ready worker 17h v1.18.3+fa69cae
worker-1 Ready worker 16h v1.18.3+fa69cae
worker-2 Ready worker 23m v1.18.3+fa69cae
worker-3 Ready worker 17m v1.18.3+fa69cae Decrease Worker Count
Increase Worker Count
Decrease Worker Count
|
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.
/lgtm
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.
/lgtm
Thanks @yussufsh @Kuldip-Nanda
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bpradipt, Prajyot-Parab, yussufsh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Co-authored-by: Kuldip Nanda kuldip.nanda@ibm.com
Signed-off-by: Yussuf Shaikh yussuf.shaikh@ibm.com