From b0aeb6f90a0025d4e5f5c8079d72fbe95a5d314e Mon Sep 17 00:00:00 2001
From: Martin Weindel <martin.weindel@sap.com>
Date: Fri, 17 Jan 2025 11:15:39 +0100
Subject: [PATCH] fix deletion of entries after controller restart

---
 pkg/dns/provider/changemodel.go | 41 +++++++++++++++++++--------------
 pkg/dns/provider/state_zone.go  | 24 +++++++++++++++----
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/pkg/dns/provider/changemodel.go b/pkg/dns/provider/changemodel.go
index f755affd..145df2a3 100644
--- a/pkg/dns/provider/changemodel.go
+++ b/pkg/dns/provider/changemodel.go
@@ -228,15 +228,16 @@ type TargetSpec = dnsutils.TargetSpec
 
 type ChangeModel struct {
 	logger.LogContext
-	config         Config
-	ownership      dns.Ownership
-	context        *zoneReconciliation
-	applied        map[dns.DNSSetName]*dns.DNSSet
-	dangling       *ChangeGroup
-	providergroups map[string]*ChangeGroup
-	zonestate      DNSZoneState
-	failedDNSNames dns.DNSNameSet
-	oldDNSSets     dns.DNSSets
+	config            Config
+	ownership         dns.Ownership
+	context           *zoneReconciliation
+	applied           map[dns.DNSSetName]*dns.DNSSet
+	dangling          *ChangeGroup
+	providergroups    map[string]*ChangeGroup
+	zonestate         DNSZoneState
+	succeededDNSNames dns.DNSNameSet
+	failedDNSNames    dns.DNSNameSet
+	oldDNSSets        dns.DNSSets
 }
 
 type ChangeResult struct {
@@ -247,14 +248,15 @@ type ChangeResult struct {
 
 func NewChangeModel(logger logger.LogContext, ownership dns.Ownership, req *zoneReconciliation, config Config, oldDNSSets dns.DNSSets) *ChangeModel {
 	return &ChangeModel{
-		LogContext:     logger,
-		config:         config,
-		ownership:      ownership,
-		context:        req,
-		applied:        map[dns.DNSSetName]*dns.DNSSet{},
-		providergroups: map[string]*ChangeGroup{},
-		failedDNSNames: dns.DNSNameSet{},
-		oldDNSSets:     oldDNSSets,
+		LogContext:        logger,
+		config:            config,
+		ownership:         ownership,
+		context:           req,
+		applied:           map[dns.DNSSetName]*dns.DNSSet{},
+		providergroups:    map[string]*ChangeGroup{},
+		succeededDNSNames: dns.DNSNameSet{},
+		failedDNSNames:    dns.DNSNameSet{},
+		oldDNSSets:        oldDNSSets,
 	}
 }
 
@@ -469,6 +471,10 @@ func (this *ChangeModel) IsFailed(name dns.DNSSetName) bool {
 	return this.failedDNSNames.Contains(name)
 }
 
+func (this *ChangeModel) IsSucceeded(name dns.DNSSetName) bool {
+	return this.succeededDNSNames.Contains(name)
+}
+
 func (this *ChangeModel) wrappedDoneHandler(name dns.DNSSetName, done DoneHandler) DoneHandler {
 	return &changeModelDoneHandler{
 		changeModel: this,
@@ -500,6 +506,7 @@ func (this *changeModelDoneHandler) Failed(err error) {
 }
 
 func (this *changeModelDoneHandler) Succeeded() {
+	this.changeModel.succeededDNSNames.Add(this.dnsSetName)
 	if this.inner != nil {
 		this.inner.Succeeded()
 	}
diff --git a/pkg/dns/provider/state_zone.go b/pkg/dns/provider/state_zone.go
index c7eb4e6c..373b5fa5 100644
--- a/pkg/dns/provider/state_zone.go
+++ b/pkg/dns/provider/state_zone.go
@@ -208,16 +208,19 @@ func (this *state) reconcileZone(logger logger.LogContext, req *zoneReconciliati
 	for _, e := range outdatedEntries {
 		if changes.IsFailed(e.DNSSetName()) {
 			if oldSet, ok := oldDNSSets[e.DNSSetName()]; ok {
-				if txn := this.getActiveZoneTransaction(zoneid); txn != nil {
-					txn.AddEntryChange(e.ObjectKey(), e.Object().GetGeneration(), oldSet, nil)
-				} else {
-					logger.Warnf("cleanup postpone failure: missing zone for %s", e.ObjectName())
-				}
+				this.addDeleteToNextTransaction(logger, req, zoneid, e, oldSet)
 			} else {
 				logger.Warnf("cleanup postpone failure: old set not found for %s", e.ObjectName())
 			}
 			continue
 		}
+		if !changes.IsSucceeded(e.DNSSetName()) {
+			// DNSEntry in deleting state, but not completely handled (e.g. after restart before zone reconciliation was running)
+			if oldSet, ok := changes.zonestate.GetDNSSets()[e.DNSSetName()]; ok {
+				this.addDeleteToNextTransaction(logger, req, zoneid, e, oldSet)
+				continue
+			}
+		}
 		logger.Infof("cleanup outdated entry %q", e.ObjectName())
 		err := e.RemoveFinalizer()
 		if err == nil || errors.IsNotFound(err) {
@@ -233,6 +236,17 @@ func (this *state) reconcileZone(logger logger.LogContext, req *zoneReconciliati
 	return err
 }
 
+func (this *state) addDeleteToNextTransaction(logger logger.LogContext, req *zoneReconciliation, zoneid dns.ZoneID, e *Entry, oldSet *dns.DNSSet) {
+	if txn := this.getActiveZoneTransaction(zoneid); txn != nil {
+		txn.AddEntryChange(e.ObjectKey(), e.Object().GetGeneration(), oldSet, nil)
+		if req.zone.nextTrigger == 0 {
+			req.zone.nextTrigger = this.config.Delay
+		}
+	} else {
+		logger.Warnf("cleanup postpone failure: missing zone for %s", e.ObjectName())
+	}
+}
+
 func (this *state) deleteZone(zoneid dns.ZoneID) {
 	metrics.DeleteZone(zoneid)
 	delete(this.zones, zoneid)