Ensure record for DNSEntries are left untouched during migration (#408)

* ensure record for DNSEntries are left untouched during migration

* fix NPE if extension has been deleted

* drop indirection for gardener operator constants

go.mod

@@ -4,7 +4,7 @@ go 1.23.0
 
 require (
 	github.com/ahmetb/gen-crd-api-reference-docs v0.3.1-0.20241014194617-ffc4efda75d4
-	github.com/gardener/external-dns-management v0.22.1
+	github.com/gardener/external-dns-management v0.22.2
 	github.com/gardener/gardener v1.110.0
 	github.com/go-logr/logr v1.4.2
 	github.com/hashicorp/go-multierror v1.1.1
@@ -43,6 +43,7 @@ require (
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
 	github.com/gardener/cert-management v0.17.1 // indirect
+	github.com/gardener/controller-manager-library v0.2.1-0.20241206090116-9fadce45689c // indirect
 	github.com/gardener/etcd-druid v0.25.0 // indirect
 	github.com/gardener/machine-controller-manager v0.55.1 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect

go.sum

@@ -95,10 +95,12 @@ github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv
 github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ=
 github.com/gardener/cert-management v0.17.1 h1:vawZGN+rsCRMviacnnMSWELbuIJsNXHaqaLbZ4hYADw=
 github.com/gardener/cert-management v0.17.1/go.mod h1:cwSsyN935017HojKVuWqw2TBhiaxSisX132D9Tn+n9I=
+github.com/gardener/controller-manager-library v0.2.1-0.20241206090116-9fadce45689c h1:im/eYurY6+CzlRtxYzes6IfqMPIuzQ7Cy3sA8D/t528=
+github.com/gardener/controller-manager-library v0.2.1-0.20241206090116-9fadce45689c/go.mod h1:fyLOrcaKtGno4McZKW21b6QtwNghCF0IemTLKcwKZlM=
 github.com/gardener/etcd-druid v0.25.0 h1:mR9/x5r27pO+I+XzpNcN2DDenam+7ITrhc7qKt9rbsI=
 github.com/gardener/etcd-druid v0.25.0/go.mod h1:6C0eyfdlw6CowLm/l4ZiKwrvkc+5NHrnc/rY2wCUwys=
-github.com/gardener/external-dns-management v0.22.1 h1:WEwCDOersJ7ezeDJelbGVac1BTmEveJuds3JlJc84Xg=
-github.com/gardener/external-dns-management v0.22.1/go.mod h1:2P7PamBPMKIOZMYRhl/VFhxZEBn4VUTdjESjKPxvOXA=
+github.com/gardener/external-dns-management v0.22.2 h1:caSPJBLFHv9Y95IAwk1HvarIUCjDccLcyuyjW1qqwhM=
+github.com/gardener/external-dns-management v0.22.2/go.mod h1:adBY3qQ39Fvc2PvihP4xzEE5Y2//GuurMXQpKylMOJ4=
 github.com/gardener/gardener v1.110.0 h1:Ix/NeYJyYIIDRHqO0126JYPGNVKy2kDEco7RyXuCYwo=
 github.com/gardener/gardener v1.110.0/go.mod h1:Ge2wQMWm0NmQZP3L/WMejpfXsnGbfTFBEZud819P3vU=
 github.com/gardener/machine-controller-manager v0.55.1 h1:d6mTnuYko+jWeIi7tAFWgWnL1nR5hGcI6pRCDcH0TGY=

pkg/controller/common/utils.go

@@ -19,11 +19,8 @@ import (
 	"github.com/gardener/gardener-extension-shoot-dns-service/pkg/service"
 )
 
-const (
-	ANNOTATION_OPERATION         = constants.GardenerOperation
-	ANNOTATION_OPERATION_MIGRATE = constants.GardenerOperationMigrate
-	ANNOTATION_OPERATION_RESTORE = constants.GardenerOperationRestore
-)
+// gardenerOperationRestorePrepare is an internal value for the restore prepare operation
+const gardenerOperationRestorePrepare = "restore/prepare"
 
 func CopyMap(m map[string]string) map[string]string {
 	if m == nil {
@@ -57,14 +54,28 @@ func IsMigrating(ex *extensionsv1alpha1.Extension) bool {
 	if ex.Annotations == nil {
 		return false
 	}
-	return ex.Annotations[ANNOTATION_OPERATION] == ANNOTATION_OPERATION_MIGRATE
+	return ex.Annotations[constants.GardenerOperation] == constants.GardenerOperationMigrate
 }
 
 func IsRestoring(ex *extensionsv1alpha1.Extension) bool {
 	if ex.Annotations == nil {
 		return false
 	}
-	return ex.Annotations[ANNOTATION_OPERATION] == ANNOTATION_OPERATION_RESTORE
+	return ex.Annotations[constants.GardenerOperation] == constants.GardenerOperationRestore
+}
+
+func IsPreparingRestore(ex *extensionsv1alpha1.Extension) bool {
+	if ex.Annotations == nil {
+		return false
+	}
+	return ex.Annotations[constants.GardenerOperation] == gardenerOperationRestorePrepare
+}
+
+func SetRestorePrepareAnnotation(ex *extensionsv1alpha1.Extension) {
+	if ex.Annotations == nil {
+		ex.Annotations = map[string]string{}
+	}
+	ex.Annotations[constants.GardenerOperation] = gardenerOperationRestorePrepare
 }
 
 // ShortenID shortens an identifier longer than maxlen characters by cutting the string

pkg/controller/lifecycle/actuator.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	dnsv1alpha1 "github.com/gardener/external-dns-management/pkg/apis/dns/v1alpha1"
+	"github.com/gardener/external-dns-management/pkg/dns"
 	"github.com/gardener/gardener/extensions/pkg/controller"
 	"github.com/gardener/gardener/extensions/pkg/controller/extension"
 	"github.com/gardener/gardener/extensions/pkg/util"
@@ -258,6 +259,19 @@ func (a *actuator) delete(ctx context.Context, log logr.Logger, ex *extensionsv1
 
 // Restore the Extension resource.
 func (a *actuator) Restore(ctx context.Context, log logr.Logger, ex *extensionsv1alpha1.Extension) error {
+	// TODO(martinweindel): Drop this section once the DNS owner has been removed for all seeds on all landscapes.
+	// First, run extension reconciliation with deactivated DNSOwner to avoid
+	// zone reconciliation before all entries are reconciled.
+	// Premature zone reconciliation can lead to DNS entries being deleted temporarily.
+	exCopy := ex.DeepCopy()
+	common.SetRestorePrepareAnnotation(exCopy)
+	if err := a.Reconcile(ctx, log, exCopy); err != nil {
+		return err
+	}
+
+	if err := a.waitForEntryReconciliation(ctx, log, ex); err != nil {
+		return err
+	}
 	return a.Reconcile(ctx, log, ex)
 }
 
@@ -268,9 +282,73 @@ func (a *actuator) Migrate(ctx context.Context, log logr.Logger, ex *extensionsv
 		return err
 	}
 
+	if err := a.ignoreDNSEntriesForMigration(ctx, ex); err != nil {
+		return err
+	}
+
 	return a.delete(ctx, log, ex, true)
 }
 
+func (a *actuator) ignoreDNSEntriesForMigration(ctx context.Context, ex *extensionsv1alpha1.Extension) error {
+	entriesHelper := common.NewShootDNSEntriesHelper(ctx, a.Client(), ex)
+	list, err := entriesHelper.List()
+	if err != nil {
+		return err
+	}
+	for _, entry := range list {
+		patch := client.MergeFrom(entry.DeepCopy())
+		if entry.Annotations == nil {
+			entry.Annotations = map[string]string{}
+		}
+		entry.Annotations[dns.AnnotationHardIgnore] = "true"
+		if err := client.IgnoreNotFound(a.Client().Patch(ctx, &entry, patch)); err != nil {
+			return fmt.Errorf("failed to ignore DNS entry %q: %w", entry.Name, err)
+		}
+	}
+	return nil
+}
+
+func (a *actuator) waitForEntryReconciliation(ctx context.Context, log logr.Logger, ex *extensionsv1alpha1.Extension) error {
+	entriesHelper := common.NewShootDNSEntriesHelper(ctx, a.Client(), ex)
+	list, err := entriesHelper.List()
+	if err != nil {
+		return err
+	}
+
+	// annotate all entries with gardener.cloud/operation=reconcile
+	for _, entry := range list {
+		patch := client.MergeFrom(entry.DeepCopy())
+		if entry.Annotations == nil {
+			entry.Annotations = map[string]string{}
+		}
+		entry.Annotations[v1beta1constants.GardenerOperation] = v1beta1constants.GardenerOperationReconcile
+		delete(entry.Annotations, dns.AnnotationHardIgnore) // should not be needed as the DNSEntries have been recreated, but just to be sure
+		if err := client.IgnoreNotFound(a.Client().Patch(ctx, &entry, patch)); err != nil {
+			return fmt.Errorf("failed to revert ignore DNS entry %q: %w", entry.Name, err)
+		}
+	}
+
+	// wait for all entries to be reconciled, i.e. gardener.cloud/operation annotation is removed
+	start := time.Now()
+	for _, entry := range list {
+		for {
+			if err := a.Client().Get(ctx, client.ObjectKeyFromObject(&entry), &entry); err != nil {
+				return err
+			}
+			if _, ok := entry.Annotations[v1beta1constants.GardenerOperation]; !ok {
+				log.Info("DNS entry reconciled", "entry", entry.Name)
+				break
+			}
+			if time.Since(start) > 3*time.Minute {
+				return fmt.Errorf("timeout waiting for DNS entry %q to be reconciled", entry.Name)
+			}
+			time.Sleep(1 * time.Second)
+		}
+	}
+
+	return nil
+}
+
 func (a *actuator) isManagingDNSProviders(dns *gardencorev1beta1.DNS) bool {
 	return a.Config().ManageDNSProviders && dns != nil && dns.Domain != nil
 }
@@ -312,7 +390,7 @@ func (a *actuator) createOrUpdateSeedResources(ctx context.Context, dnsconfig *a
 	if !deploymentEnabled || a.isHibernated(cluster) {
 		replicas = 0
 	}
-	shootActive := !common.IsMigrating(ex)
+	shootActive := !common.IsMigrating(ex) && !common.IsPreparingRestore(ex)
 
 	chartValues := map[string]interface{}{
 		"serviceName":                      service.ServiceName,

pkg/controller/replication/reconciler.go

@@ -53,7 +53,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	if err != nil {
 		return result, err
 	}
-	if common.IsMigrating(ext) {
+	if ext == nil || common.IsMigrating(ext) {
 		return result, nil
 	}
 	statehandler, err := common.NewStateHandler(ctx, r.Env, ext, false)