HYPERFLEET-576 | add the clean steps to delete the resources in deployment progress #25
Conversation
WalkthroughThis PR extends the deployment scripts with comprehensive uninstall and resource cleanup capabilities. Three new control flags (DELETE_K8S_RESOURCES, DELETE_CLOUD_RESOURCES, DELETE_ALL) are introduced to enable selective deletion of Kubernetes resources (Helm releases and namespaces) and GCP Pub/Sub resources (topics and subscriptions). A new GCP utility module is added to handle cloud resource discovery and deletion, the main deployment script is enhanced to orchestrate the uninstall flow with error tracking, and a helper function is added for safe Kubernetes namespace deletion. Sequence DiagramsequenceDiagram
actor User
participant deploy-clm.sh as Deploy Script
participant K8s as Kubernetes Cluster
participant GCP as GCP Cloud
User->>deploy-clm.sh: Execute with --delete-k8s-resources<br/>and/or --delete-cloud-resources
deploy-clm.sh->>deploy-clm.sh: Parse flags & validate config
alt DELETE_K8S_RESOURCES enabled
deploy-clm.sh->>K8s: Uninstall Helm releases<br/>(Adapter → Sentinel → API)
K8s-->>deploy-clm.sh: Resources removed
deploy-clm.sh->>K8s: Delete namespace
K8s-->>deploy-clm.sh: Namespace deleted
end
alt DELETE_CLOUD_RESOURCES enabled
deploy-clm.sh->>GCP: Discover Pub/Sub subscriptions<br/>(filter by namespace)
GCP-->>deploy-clm.sh: Subscription list
deploy-clm.sh->>GCP: Delete each subscription
GCP-->>deploy-clm.sh: Subscriptions deleted
deploy-clm.sh->>GCP: Discover Pub/Sub topics<br/>(filter by namespace)
GCP-->>deploy-clm.sh: Topic list
deploy-clm.sh->>GCP: Delete each topic
GCP-->>deploy-clm.sh: Topics deleted
end
deploy-clm.sh->>deploy-clm.sh: Aggregate results &<br/>summarize cleanup
deploy-clm.sh-->>User: Final summary with<br/>error count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy-scripts/deploy-clm.sh (1)
334-338:⚠️ Potential issue | 🟡 MinorValidation runs unconditionally but error message is install-specific.
This validation block executes for both
installanduninstallactions, but the error message references "installation". For uninstall with--delete-k8s-resourcesor--delete-cloud-resources, a user might legitimately want to skip all component uninstalls but still delete the namespace or cloud resources.Consider either:
- Making this validation conditional on
ACTION == "install", or- For uninstall, allowing all components to be skipped when deletion flags are set
🛠️ Suggested fix
- # Validate at least one component is selected - if [[ "${INSTALL_API}" == "false" && "${INSTALL_SENTINEL}" == "false" && "${INSTALL_ADAPTER}" == "false" ]]; then - log_error "At least one component must be selected for installation" - exit 1 - fi + # Validate at least one component is selected (only for install action) + if [[ "${ACTION}" == "install" ]]; then + if [[ "${INSTALL_API}" == "false" && "${INSTALL_SENTINEL}" == "false" && "${INSTALL_ADAPTER}" == "false" ]]; then + log_error "At least one component must be selected for installation" + exit 1 + fi + fi
🧹 Nitpick comments (3)
deploy-scripts/deploy-clm.sh (1)
448-465: Inconsistent error logging for component uninstalls.The warning log for API uninstall failure (line 462) is inconsistent with adapter and sentinel handling. Looking at the referenced
uninstall_apifunction indeploy-scripts/lib/api.sh(lines 97-122), it already logslog_error "Failed to uninstall API"on failure.This creates potential double-logging for API failures while adapter and sentinel failures are handled consistently (only internal logging).
♻️ Suggested fix for consistency
if [[ "${INSTALL_API}" == "true" ]]; then if ! uninstall_api; then - log_warning "Failed to uninstall API" ((uninstall_errors++)) fi fideploy-scripts/lib/gcp.sh (2)
225-234: Remove duplicate error logging.
delete_pubsub_subscriptionalready logs an error on failure (line 186). The additionallog_erroron line 230 results in duplicate error messages for the same failure.♻️ Suggested fix
while IFS= read -r subscription; do if [[ -n "${subscription}" ]]; then if ! delete_pubsub_subscription "${subscription}"; then - log_error "Failed to delete subscription: ${subscription}" ((failed++)) fi fi done <<< "${subscriptions}"
257-266: Remove duplicate error logging (same pattern as subscriptions).
delete_pubsub_topicalready logs an error on failure (line 208). The additionallog_erroron line 262 creates duplicate error messages.♻️ Suggested fix
while IFS= read -r topic; do if [[ -n "${topic}" ]]; then if ! delete_pubsub_topic "${topic}"; then - log_error "Failed to delete topic: ${topic}" ((failed++)) fi fi done <<< "${topics}"
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yasun1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 1a1dab4
into
openshift-hyperfleet:main
2 participants
…
Summary by CodeRabbit
Release Notes
New Features
Chores