go back to always deleting the dns record

Author: mattdjenkinson

internal/controller/gateway_controller.go

@@ -908,43 +908,19 @@ func (r *GatewayReconciler) cleanupDNSRecordSets(
 		return result
 	}
 
-	deletedCount := 0
-	for i := range recordSetList.Items {
-		rs := &recordSetList.Items[i]
-		hostname := rs.Annotations[annotationDNSHostname]
-		if rs.Annotations[annotationDNSRetain] == "true" {
-			logger.Info("retaining DNSRecordSet during gateway finalization, removing managed-by labels",
-				"name", rs.Name,
-				"hostname", hostname,
-			)
-			if rs.Labels != nil {
-				labels := maps.Clone(rs.Labels)
-				delete(labels, labelDNSManaged)
-				delete(labels, labelManagedBy)
-				delete(labels, labelDNSSourceKind)
-				delete(labels, labelDNSSourceName)
-				delete(labels, labelDNSSourceNS)
-				rs.Labels = labels
-				if err := upstreamClient.Update(ctx, rs); err != nil {
-					result.Err = fmt.Errorf("failed to update DNSRecordSet %q (retain): %w", rs.Name, err)
-					return result
-				}
-			}
-			continue
-		}
+	for _, rs := range recordSetList.Items {
 		logger.Info("deleting DNSRecordSet during gateway finalization",
 			"name", rs.Name,
-			"hostname", hostname,
+			"hostname", rs.Annotations[annotationDNSHostname],
 		)
-		if err := upstreamClient.Delete(ctx, rs); err != nil && !apierrors.IsNotFound(err) {
+		if err := upstreamClient.Delete(ctx, &rs); err != nil && !apierrors.IsNotFound(err) {
 			result.Err = fmt.Errorf("failed to delete DNSRecordSet %q: %w", rs.Name, err)
 			return result
 		}
-		deletedCount++
 	}
 
-	if deletedCount > 0 {
-		logger.Info("deleted DNSRecordSets during gateway finalization", "count", deletedCount)
+	if len(recordSetList.Items) > 0 {
+		logger.Info("deleted DNSRecordSets during gateway finalization", "count", len(recordSetList.Items))
 	}
 
 	return result

internal/controller/gateway_dns_controller.go

@@ -42,7 +42,6 @@ const (
 	labelDNSSourceName    = "dns.datumapis.com/source-name"
 	labelDNSSourceNS      = "dns.datumapis.com/source-namespace"
 	annotationDNSHostname = "dns.datumapis.com/hostname"
-	annotationDNSRetain   = "dns.datumapis.com/retain"
 	annotationSyncStart   = "dns.datumapis.com/sync-started-at"
 )
 

internal/controller/gateway_dns_controller_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -858,138 +857,6 @@ func TestGarbageCollectDNSRecordSets_DoesNotDeleteInUseRecords(t *testing.T) {
 	require.NoError(t, cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: rsName}, &remaining))
 }
 
-// ---------------------------------------------------------------------------
-// TestCleanupDNSRecordSets
-// ---------------------------------------------------------------------------
-
-func TestCleanupDNSRecordSets_DeletesManagedRecords(t *testing.T) {
-	const ns = "test-ns"
-	ctx := log.IntoContext(context.Background(), zap.New())
-	s := newDNSTestScheme(t)
-
-	testConfig := config.NetworkServicesOperator{
-		Gateway: config.GatewayConfig{
-			TargetDomain:         "gateways.test.local",
-			EnableDNSIntegration: true,
-		},
-	}
-
-	gw := newTestGatewayForDNS(ns, "my-gw")
-	makeRS := func(name, hostname string) *dnsv1alpha1.DNSRecordSet {
-		return &dnsv1alpha1.DNSRecordSet{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: ns,
-				Name:      name,
-				UID:       uuid.NewUUID(),
-				Labels: map[string]string{
-					labelManagedBy:     labelManagedByValue,
-					labelDNSManaged:    "true",
-					labelDNSSourceKind: KindGateway,
-					labelDNSSourceName: gw.Name,
-					labelDNSSourceNS:   ns,
-				},
-				Annotations: map[string]string{
-					annotationDNSHostname: hostname,
-				},
-			},
-			Spec: dnsv1alpha1.DNSRecordSetSpec{
-				DNSZoneRef: corev1.LocalObjectReference{Name: "example-com"},
-				RecordType: dnsv1alpha1.RRTypeCNAME,
-				Records:    []dnsv1alpha1.RecordEntry{{Name: hostname + "."}},
-			},
-		}
-	}
-
-	rs1 := makeRS("my-gw-api", "api.example.com")
-	rs2 := makeRS("my-gw-other", "other.example.com")
-	allObjects := []client.Object{gw, rs1, rs2}
-	for _, obj := range allObjects {
-		if obj.GetUID() == "" {
-			obj.SetUID(uuid.NewUUID())
-		}
-		obj.SetCreationTimestamp(metav1.Now())
-	}
-
-	cl := buildFakeUpstreamClientForDNS(s, allObjects...)
-	reconciler := newDNSReconciler(testConfig)
-
-	result := reconciler.cleanupDNSRecordSets(ctx, cl, gw)
-	require.NoError(t, result.Err)
-
-	// Both records should be deleted.
-	var deleted1 dnsv1alpha1.DNSRecordSet
-	err1 := cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: rs1.Name}, &deleted1)
-	assert.True(t, apierrors.IsNotFound(err1), "DNSRecordSet %q should have been deleted: %v", rs1.Name, err1)
-
-	var deleted2 dnsv1alpha1.DNSRecordSet
-	err2 := cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: rs2.Name}, &deleted2)
-	assert.True(t, apierrors.IsNotFound(err2), "DNSRecordSet %q should have been deleted: %v", rs2.Name, err2)
-}
-
-func TestCleanupDNSRecordSets_RetainsAndStripsLabelsWhenAnnotationSet(t *testing.T) {
-	const ns = "test-ns"
-	ctx := log.IntoContext(context.Background(), zap.New())
-	s := newDNSTestScheme(t)
-
-	testConfig := config.NetworkServicesOperator{
-		Gateway: config.GatewayConfig{
-			TargetDomain:         "gateways.test.local",
-			EnableDNSIntegration: true,
-		},
-	}
-
-	gw := newTestGatewayForDNS(ns, "my-gw")
-	rsName := dnsRecordSetName(gw.Name, "api.example.com")
-	rs := &dnsv1alpha1.DNSRecordSet{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: ns,
-			Name:      rsName,
-			UID:       uuid.NewUUID(),
-			Labels: map[string]string{
-				labelManagedBy:     labelManagedByValue,
-				labelDNSManaged:    "true",
-				labelDNSSourceKind: KindGateway,
-				labelDNSSourceName: gw.Name,
-				labelDNSSourceNS:   ns,
-			},
-			Annotations: map[string]string{
-				annotationDNSHostname: "api.example.com",
-				annotationDNSRetain:   "true",
-			},
-		},
-		Spec: dnsv1alpha1.DNSRecordSetSpec{
-			DNSZoneRef: corev1.LocalObjectReference{Name: "example-com"},
-			RecordType: dnsv1alpha1.RRTypeCNAME,
-			Records:    []dnsv1alpha1.RecordEntry{{Name: "api.example.com."}},
-		},
-	}
-
-	allObjects := []client.Object{gw, rs}
-	for _, obj := range allObjects {
-		if obj.GetUID() == "" {
-			obj.SetUID(uuid.NewUUID())
-		}
-		obj.SetCreationTimestamp(metav1.Now())
-	}
-
-	cl := buildFakeUpstreamClientForDNS(s, allObjects...)
-	reconciler := newDNSReconciler(testConfig)
-
-	result := reconciler.cleanupDNSRecordSets(ctx, cl, gw)
-	require.NoError(t, result.Err)
-
-	// Record should still exist.
-	var updated dnsv1alpha1.DNSRecordSet
-	require.NoError(t, cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: rsName}, &updated))
-
-	// Managed-by labels should have been removed.
-	assert.Empty(t, updated.Labels[labelDNSManaged], "label %q should be removed", labelDNSManaged)
-	assert.Empty(t, updated.Labels[labelManagedBy], "label %q should be removed", labelManagedBy)
-	assert.Empty(t, updated.Labels[labelDNSSourceKind], "label %q should be removed", labelDNSSourceKind)
-	assert.Empty(t, updated.Labels[labelDNSSourceName], "label %q should be removed", labelDNSSourceName)
-	assert.Empty(t, updated.Labels[labelDNSSourceNS], "label %q should be removed", labelDNSSourceNS)
-}
-
 // ---------------------------------------------------------------------------
 // TestReconcileDNSStatus
 // ---------------------------------------------------------------------------