Skip to content

Commit f528283

Browse files
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 <somanchi004@gmail.com>
1 parent 1a3fc27 commit f528283

1 file changed

Lines changed: 18 additions & 4 deletions

File tree

pkg/controller/chi/worker-deleter.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"time"
2121

2222
core "k8s.io/api/core/v1"
23+
apiErrors "k8s.io/apimachinery/pkg/api/errors"
2324
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
2425

2526
log "github.com/altinity/clickhouse-operator/pkg/announcer"
@@ -539,12 +540,25 @@ func (w *worker) deleteHost(ctx context.Context, chi *api.ClickHouseInstallation
539540

540541
var err error
541542
if host.Runtime.CurStatefulSet, err = w.c.kube.STS().Get(ctx, host); err != nil {
542-
w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteCompleted).
543-
WithAction(host.GetCR()).
543+
if apiErrors.IsNotFound(err) {
544+
// StatefulSet is gone for sure - the host is already deleted.
545+
w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteCompleted).
546+
WithAction(host.GetCR()).
547+
M(host).F().
548+
Info("Delete host: %s/%s - completed StatefulSet not found - already deleted",
549+
host.Runtime.Address.ClusterName, host.GetName())
550+
return nil
551+
}
552+
// Unable to tell whether the StatefulSet exists.
553+
// Do not report deletion as completed - the cleanup below is skipped, and the host's
554+
// PVCs carry no owner reference (see model/common/creator/pvc.go), so nothing else
555+
// would reclaim them. Report the failure and let the caller decide.
556+
w.a.WithEvent(host.GetCR(), a.EventActionDelete, a.EventReasonDeleteFailed).
557+
WithError(host.GetCR()).
544558
M(host).F().
545-
Info("Delete host: %s/%s - completed StatefulSet not found - already deleted? err: %v",
559+
Error("Delete host: %s/%s - unable to get StatefulSet, host deletion not performed. err: %v",
546560
host.Runtime.Address.ClusterName, host.GetName(), err)
547-
return nil
561+
return err
548562
}
549563

550564
// Pre-delete host hooks: run BEFORE we touch the host's k8s objects so the pod is

0 commit comments

Comments
 (0)