From f5282831d8d1a1ed5b81703c0d8ba441049b37b0 Mon Sep 17 00:00:00 2001 From: Somanchi Poorna Sobhita Date: Tue, 4 Aug 2026 21:44:44 +0000 Subject: [PATCH] Do not report host deletion as completed on an unknown API error deleteHost() fetches the host StatefulSet and treats any error from that Get as "StatefulSet not found - already deleted", emitting a DeleteCompleted event and returning nil. Only NotFound actually means the host is gone; a Forbidden, a timeout or any other transient API error takes the same branch. That early return skips both deleteTables() - which the surrounding comment notes is required so ZooKeeper stops tracking the host's tables - and Controller.deleteHost(), which deletes the host's PVCs. Those PVCs carry no owner reference (see model/common/creator/pvc.go, where it is commented out to stay compatible with the PV retain policy), so the operator's own call is the only thing that ever reclaims them. Classify the error instead: keep the existing behaviour for NotFound, and on any other error emit DeleteFailed and return it rather than claiming the host was deleted. apiErrors.IsNotFound is already used this way elsewhere in the package, for example in worker-pdb.go. Signed-off-by: Somanchi Poorna Sobhita --- pkg/controller/chi/worker-deleter.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/pkg/controller/chi/worker-deleter.go b/pkg/controller/chi/worker-deleter.go index 9b1e38179..cb11cdea4 100644 --- a/pkg/controller/chi/worker-deleter.go +++ b/pkg/controller/chi/worker-deleter.go @@ -20,6 +20,7 @@ import ( "time" core "k8s.io/api/core/v1" + apiErrors "k8s.io/apimachinery/pkg/api/errors" meta "k8s.io/apimachinery/pkg/apis/meta/v1" log "github.com/altinity/clickhouse-operator/pkg/announcer" @@ -539,12 +540,25 @@ func (w *worker) deleteHost(ctx context.Context, chi *api.ClickHouseInstallation var err error if host.Runtime.CurStatefulSet, err = w.c.kube.STS().Get(ctx, host); err != nil { - w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteCompleted). - WithAction(host.GetCR()). + if apiErrors.IsNotFound(err) { + // StatefulSet is gone for sure - the host is already deleted. + w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteCompleted). + WithAction(host.GetCR()). + M(host).F(). + Info("Delete host: %s/%s - completed StatefulSet not found - already deleted", + host.Runtime.Address.ClusterName, host.GetName()) + return nil + } + // Unable to tell whether the StatefulSet exists. + // Do not report deletion as completed - the cleanup below is skipped, and the host's + // PVCs carry no owner reference (see model/common/creator/pvc.go), so nothing else + // would reclaim them. Report the failure and let the caller decide. + w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteFailed). + WithError(host.GetCR()). M(host).F(). - Info("Delete host: %s/%s - completed StatefulSet not found - already deleted? err: %v", + Error("Delete host: %s/%s - unable to get StatefulSet, host deletion not performed. err: %v", host.Runtime.Address.ClusterName, host.GetName(), err) - return nil + return err } // Pre-delete host hooks: run BEFORE we touch the host's k8s objects so the pod is