-
Notifications
You must be signed in to change notification settings - Fork 1.2k
CKS: use --delete-emptydir-data instead of deprecated --delete-local-data #10234
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 |
|
@weizhouapache 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10234 +/- ##
============================================
- Coverage 15.13% 15.13% -0.01%
+ Complexity 11279 11276 -3
============================================
Files 5408 5408
Lines 474007 474007
Branches 57822 57822
============================================
- Hits 71745 71734 -11
- Misses 394241 394254 +13
+ Partials 8021 8019 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12160 |
|
@blueorangutan test keepEnv |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-12163)
|
DaanHoogland
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.
clgtm
needs regressions testing and maybe deprecation of some older kubctl versions as supported.
kiranchavala
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.
LGTM
Tested manually
- Generate a cks iso based on 1.31.1 version of kubernetes
./create-kubernetes-binaries-iso.sh ./ 1.31.1 1.3.0 1.27.0 https://raw.githubusercontent.com/weaveworks/weave/master/prog/weave-kube/weave-daemonset-k8s-1.11.yaml https://raw.githubusercontent.com/kubernetes/dashboard/v2.7.0/aio/deploy/recommended.yaml setup-1.31.1
-
Register it as a kubernetes iso and deploy a cks cluster
-
Enable autoscaling on the cluster ( specify min and max size)
-
Deploy a test deployment say nginx on the cks cluster
-
Trigger cks autoscaling by executing the command
kubectl scale --replicas=250 deployment/nginx-deployment -
The node count of the cks cluster will increase
kubectl get nodes -
Trigger cks autodescaling
kubectl scale --replicas=25 deployment/nginx-deployment
Check the logs
Before fix
2025-01-28 11:10:51,991 INFO [c.c.k.c.a.KubernetesClusterScaleWorker] (API-Job-Executor-50:[ctx-ddc3650b, job-74, ctx-e8087ea8]) (logid:a9c772dc) Scaling Kubernetes cluster : test
2025-01-28 11:10:51,999 INFO [c.c.k.c.a.KubernetesClusterScaleWorker] (API-Job-Executor-50:[ctx-ddc3650b, job-74, ctx-e8087ea8]) (logid:a9c772dc) Removing vm : test-node-194ac81c54e from cluster test
2025-01-28 11:10:52,719 ERROR [c.c.u.s.SshHelper] (API-Job-Executor-50:[ctx-ddc3650b, job-74, ctx-e8087ea8]) (logid:a9c772dc) SSH execution of command sudo /opt/bin/kubectl drain test-node-194ac81c54e --ignore-daemonsets --delete-local-data has an error status code in return. Result output: error: unknown flag: --delete-local-data
See 'kubectl drain --help' for usage.
After fix
No error message observed , and cks cluster scaled downed successfully
|
thanks @kiranchavala for the testing |
Description
This PR fixes #10233
--delete-local-datais dropped since Kubernetes 1.31https://github.com/kubernetes/kubernetes/blob/a78983906f4cf6f94029a3e6cf3171ef42b395d7/CHANGELOG/CHANGELOG-1.31.md?plain=1#L1338
--delete-emptydir-datahas been supported since 2020kubernetes/kubernetes@625e47a
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?