Skip to content

Let a SolrCloud be deleted when its providedConfigMap is gone - #888

Open
syahra712 wants to merge 4 commits into
apache:mainfrom
syahra712:fix/828-deletion-blocked-by-missing-configmap
Open

syahra712 wants to merge 4 commits into
apache:mainfrom
syahra712:fix/828-deletion-blocked-by-missing-configmap

Conversation

@syahra712

Copy link
Copy Markdown

Fixes #828.

The bug

Reconcile has no deletion branch. A SolrCloud with a deletionTimestamp goes through the full reconcile (ZK, services, ConfigMap, StatefulSet…) before it reaches reconcileStorageFinalizer. If any step on the way returns an error, the storage finalizer is never removed and the SolrCloud can't be deleted.

The case from the issue: when providedConfigMap is already gone (Helm deletes it in the same uninstall/upgrade), the lookup returns NotFound on every pass, so the CR is stuck for good.

The fix

When the SolrCloud is being deleted, Reconcile now goes straight to a small reconcileDeletion that only handles the storage finalizer. Owned resources are garbage collected by Kubernetes, so nothing else needs reconciling on that path.

  • It uses the live StatefulSet's selector to find the PVCs, the same as before.
  • If the StatefulSet is already gone (e.g. foreground deletion), it falls back to SharedLabels(). That is the selector the operator always generates for the StatefulSet, and the StatefulSet controller copies the selector's labels onto every PVC it creates (getPersistentVolumeClaims in pkg/controller/statefulset/stateful_set_utils.go).
  • The "no selector labels → don't touch PVCs" guard and the 15s retry when PVC cleanup fails are kept as they were.

Tests

New spec in solrcloud_controller_storage_test.go: a SolrCloud with a provided ConfigMap and VolumeReclaimPolicy: Delete. The test deletes the ConfigMap, then the SolrCloud, and expects the SolrCloud to go away.

  • Without the fix it times out, and the operator logs ConfigMap "provided-config-deleted-first" not found on every reconcile, same as in the issue.
  • With the fix it passes. The existing "PVC Delete on Reclaim" deletion spec also still passes.

The full go test ./api/... ./controllers/... run passes (78 specs) against envtest 1.37.0, and so do make check-format vet check-generated check-manifests.

AI-assisted (Claude)

Reconcile ran the whole reconcile loop for a SolrCloud being deleted,
so the providedConfigMap lookup failed with NotFound on every pass and
the storage finalizer was never removed.

A SolrCloud with a deletionTimestamp now only runs the storage
finalizer logic. If the StatefulSet is already gone, the PVCs are
found by the selector labels the operator always generates.

Fixes apache#828

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The fallback selector can delete unrelated non-Solr PVCs sharing the cloud label, and the regression test creates no PVCs to verify deletion scope.

2 open issues
What changed in this PR

This PR addresses #828 by giving SolrCloud deletion a dedicated reconciliation path, preventing a missing provided ConfigMap from blocking storage finalizer removal.

Changes:

  • Bypass normal resource reconciliation during deletion and handle storage cleanup directly.
  • Add a regression test for deleting the provided ConfigMap before the SolrCloud.
  • Document the fix in the operator Helm chart changelog.
File Description
helm/​solr-operator/​Chart.yaml Adds a changelog entry for the deletion fix.
controllers/​solrcloud_controller.go Adds deletion-only reconciliation and a fallback PVC selector when the StatefulSet is absent.
controllers/​solrcloud_controller_storage_test.go Tests finalizer removal after the provided ConfigMap is deleted.

Review effort: Balanced

Comment thread controllers/solrcloud_controller.go Resolved
Comment thread controllers/solrcloud_controller.go Resolved
Without it, deleting a SolrCloud whose StatefulSet is already gone
also deleted PVCs that only carry the solr-cloud label, which the
StatefulSet selector would never match.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SolrCloud reconciler blocks deletion when providedConfigMap is missing

2 participants