You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "Pearl1594 (via GitHub)" <gi...@apache.org> on 2025/11/03 18:09:28 UTC
[PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Pearl1594 opened a new pull request, #11968:
URL: https://github.com/apache/cloudstack/pull/11968
### Description
This PR fixes PVC deletion script to avoid "(standard input)" and use kubectl -o name/jsonpath
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ******************************************************************************* -->
<!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ******************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [X] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3483227715
@Pearl1594 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-4847388478
<b>[SF] Trillian test result (tid-16451)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 49543 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11968-t16451-kvm-ol8.zip
Smoke tests completed. 151 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-4834283456
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5605041940
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19182
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3483398030
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15622
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5479162345
I don't believe we don't deploy the CSI driver as part of CKS test suite - so no, it isn't covered by tests @DaanHoogland
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3482088799
## [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=11968) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=11968&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=11968&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=11968&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=11968&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=11968&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=11968)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5603675270
> > oh, wait @Pearl1594 is this applicable for 4.22 too ?
>
> Yes @weizhouapache - I've rebased it. Thanks
thanks @Pearl1594
let's merge when smoke tests pass.
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5597778372
> Tested this on a live CKS cluster (KVM, CSI enabled). Deployed a workload with a PVC (reclaimPolicy: Delete), then deleted the cluster to trigger this script.
>
> With the patched script, the log correctly showed:
>
> ```
> Processing PVC pr11968-test-pvc in namespace default (PV: pvc-... has reclaimPolicy=Delete)
> Deleting Deployment: pr11968-test-app
> Deleting PVC pr11968-test-pvc in namespace default
> Script completed successfully!
> ```
>
> Deployment was identified and deleted by its actual name — no (standard input) errors. Confirms the fix works as intended.
cool, thanks @sudo87
let's merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5597790290
oh, wait
@Pearl1594
is this applicable for 4.22 too ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5603417811
> oh, wait @Pearl1594 is this applicable for 4.22 too ?
Yes @weizhouapache - I've rebased it. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5477377812
> also, this code is touched by the kubernetes integration tests, is it? (i.e. smoke tests suffice?)
is it @possible i.e. can we merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3998411271
@Pearl1594 , anything to do here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-4834266562
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5603712571
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3483224330
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3994797167
This PR is stale because it has been open for 120 days with no activity. It may be removed by administrators of this project at any time. Remove the stale label or comment to request for removal of it to prevent this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "sudo87 (via GitHub)" <gi...@apache.org>.
sudo87 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5597688700
Tested this on a live CKS cluster (KVM, CSI enabled). Deployed a workload with a PVC (reclaimPolicy: Delete), then deleted the cluster to trigger this script.
With the patched script, the log correctly showed:
```
Processing PVC pr11968-test-pvc in namespace default (PV: pvc-... has reclaimPolicy=Delete)
Deleting Deployment: pr11968-test-app
Deleting PVC pr11968-test-pvc in namespace default
Script completed successfully!
```
Deployment was identified and deleted by its actual name — no (standard input) errors. Confirms the fix works as intended.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5628570659
<b>[SF] Trillian test result (tid-16968)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 64675 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11968-t16968-kvm-ol8.zip
Smoke tests completed. 147 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_enableHumanReadableLogs | `Error` | 0.17 | test_human_readable_logs.py
test_list_vms_metrics_admin | `Error` | 50.53 | test_metrics_api.py
test_list_vms_metrics_history | `Error` | 5.70 | test_metrics_api.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-4834779595
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18411
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5479358295
I believe it's ready for merge. I tested it almost a year ago, I think it's worthwhile testing again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#discussion_r3969663592
##########
plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete:
##########
@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
local deleted_count=0
# Find & delete any deployment using the PVC
- /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r deployment; do
- if [ -n "$deployment" ]; then
- deployment_name=$(echo "$deployment" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting Deployment: $deployment_name"
- /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+ # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+ /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
+ if [ -z "$resource" ]; then
+ continue
+ fi
+ name=${resource#*/}
+ dep_volumes=$(/opt/bin/kubectl get deployment "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+ if [ -n "$dep_volumes" ] && echo "$dep_volumes" | grep -F -q "$pvc_name"; then
+ echo "$(timestamp) - Deleting Deployment: $name"
+ /opt/bin/kubectl delete deployment "$name" -n "$namespace" --ignore-not-found=true
deleted_count=$((deleted_count + 1))
fi
done
# Find and delete any StatefulSet using the PVC
- /opt/bin/kubectl get statefulsets -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r sts; do
- if [ -n "$sts" ]; then
- sts_name=$(echo "$sts" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting StatefulSet: $sts_name"
- /opt/bin/kubectl delete statefulset "$sts_name" -n "$namespace" --ignore-not-found=true
+ /opt/bin/kubectl get statefulsets -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
+ if [ -z "$resource" ]; then
+ continue
+ fi
+ name=${resource#*/}
+ # Check both template volumes and volumeClaimTemplates
+ sts_volumes=$(/opt/bin/kubectl get statefulset "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName} {.spec.volumeClaimTemplates[*].metadata.name}' 2>/dev/null || echo "")
Review Comment:
The `volumeClaimTemplates[*].metadata.name` values are template base names, not the actual PVC names created by a StatefulSet (which are typically `<templateName>-<statefulsetName>-<ordinal>`). As written, this will usually fail to detect the owning StatefulSet for a real PVC like `data-mysql-0`, and it can also create false positives for PVCs whose name happens to equal a template name. A more reliable approach is to fetch the PVC and use `metadata.ownerReferences` (or, if that’s not viable, parse the expected generated name format using the StatefulSet name and template names).
##########
plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete:
##########
@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
local deleted_count=0
# Find & delete any deployment using the PVC
- /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r deployment; do
- if [ -n "$deployment" ]; then
- deployment_name=$(echo "$deployment" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting Deployment: $deployment_name"
- /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+ # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+ /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
+ if [ -z "$resource" ]; then
+ continue
+ fi
+ name=${resource#*/}
+ dep_volumes=$(/opt/bin/kubectl get deployment "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+ if [ -n "$dep_volumes" ] && echo "$dep_volumes" | grep -F -q "$pvc_name"; then
+ echo "$(timestamp) - Deleting Deployment: $name"
+ /opt/bin/kubectl delete deployment "$name" -n "$namespace" --ignore-not-found=true
Review Comment:
This introduces an N+1 pattern (one `kubectl get deployment ...` per deployment, similarly for other kinds), which can be very slow and puts extra load on the API server in namespaces with many resources. Prefer fetching each kind once (e.g., one `kubectl get <kind> -o json` / jsonpath “range” output) and filtering locally, or derive the owning workload directly from the PVC (via `metadata.ownerReferences`) to avoid scanning the entire namespace.
##########
plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete:
##########
@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
local deleted_count=0
# Find & delete any deployment using the PVC
- /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r deployment; do
- if [ -n "$deployment" ]; then
- deployment_name=$(echo "$deployment" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting Deployment: $deployment_name"
- /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+ # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+ /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
+ if [ -z "$resource" ]; then
+ continue
+ fi
+ name=${resource#*/}
+ dep_volumes=$(/opt/bin/kubectl get deployment "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+ if [ -n "$dep_volumes" ] && echo "$dep_volumes" | grep -F -q "$pvc_name"; then
Review Comment:
The PVC match is substring-based. If `$pvc_name` is `data`, this will also match `data2`, `my-data`, etc., potentially deleting workloads that don’t actually reference the target PVC. Consider comparing exact tokens (e.g., normalize to one claim name per line and use an exact match) rather than substring searching.
##########
plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete:
##########
@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
local deleted_count=0
# Find & delete any deployment using the PVC
- /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r deployment; do
- if [ -n "$deployment" ]; then
- deployment_name=$(echo "$deployment" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting Deployment: $deployment_name"
- /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+ # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+ /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
Review Comment:
Because the `while` loop is fed by a pipeline, it will run in a subshell in many shells, so `deleted_count` increments may not be visible after the loop finishes. If `deleted_count` is used later in this function, switch to a construct that avoids the subshell (e.g., redirect from process substitution or a temporary file) so the counter is updated in the parent shell.
##########
plugins/integrations/kubernetes-service/src/main/resources/script/delete-pv-reclaimpolicy-delete:
##########
@@ -33,23 +33,33 @@ delete_workloads_using_pvc() {
local deleted_count=0
# Find & delete any deployment using the PVC
- /opt/bin/kubectl get deployments -n "$namespace" -o json 2>/dev/null | grep -l "$pvc_name" | \
- while IFS= read -r deployment; do
- if [ -n "$deployment" ]; then
- deployment_name=$(echo "$deployment" | cut -d'/' -f2)
- echo "$(timestamp) - Deleting Deployment: $deployment_name"
- /opt/bin/kubectl delete deployment "$deployment_name" -n "$namespace" --ignore-not-found=true
+ # Iterate resource names and check the volumes via jsonpath to avoid grep -l on JSON (which prints "(standard input)")
+ /opt/bin/kubectl get deployments -n "$namespace" -o name 2>/dev/null | \
+ while IFS= read -r resource; do
+ if [ -z "$resource" ]; then
+ continue
+ fi
+ name=${resource#*/}
+ dep_volumes=$(/opt/bin/kubectl get deployment "$name" -n "$namespace" -o jsonpath='{.spec.template.spec.volumes[*].persistentVolumeClaim.claimName}' 2>/dev/null || echo "")
+ if [ -n "$dep_volumes" ] && echo "$dep_volumes" | grep -F -q "$pvc_name"; then
+ echo "$(timestamp) - Deleting Deployment: $name"
+ /opt/bin/kubectl delete deployment "$name" -n "$namespace" --ignore-not-found=true
deleted_count=$((deleted_count + 1))
fi
done
Review Comment:
Because the `while` loop is fed by a pipeline, it will run in a subshell in many shells, so `deleted_count` increments may not be visible after the loop finishes. If `deleted_count` is used later in this function, switch to a construct that avoids the subshell (e.g., redirect from process substitution or a temporary file) so the counter is updated in the parent shell.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5614734588
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-3481883843
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/11968?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 3.58%. Comparing base ([`3d6cafe`](https://app.codecov.io/gh/apache/cloudstack/commit/3d6cafe1936ed2403a1031cd5ad42dd205934412?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fdd4ce4`](https://app.codecov.io/gh/apache/cloudstack/commit/fdd4ce48188463e178319deb1e2ccf8a090bc59b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (3d6cafe) and HEAD (fdd4ce4). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (3d6cafe) | HEAD (fdd4ce4) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #11968 +/- ##
=============================================
- Coverage 17.56% 3.58% -13.99%
=============================================
Files 5909 445 -5464
Lines 529056 37532 -491524
Branches 64617 6901 -57716
=============================================
- Hits 92941 1346 -91595
+ Misses 425661 36022 -389639
+ Partials 10454 164 -10290
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/11968/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.58% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/11968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/11968?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5614722631
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-5906763816
@sudo87 it seems you tested but didn't leave an approval, is that right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CKS/CSI: fix PVC deletion script to avoid '(standard input)' and use kubectl -o name/jsonpath [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #11968:
URL: https://github.com/apache/cloudstack/pull/11968#issuecomment-4833566604
> @Pearl1594 , anything to do here?
This was more of a cleanup related PR - nothing more to do @DaanHoogland - ready for review / merge..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org