GH-36 tests update

internal/controller/dnsrecord_controller.go

@@ -69,6 +69,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	logger := log.FromContext(ctx)
 
 	reconcileStart = metav1.Now()
+	logger.Info("BOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOP")
 	previous := &v1alpha1.DNSRecord{}
 	err := r.Client.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: req.Name}, previous)
 	if err != nil {
@@ -79,7 +80,6 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 	}
 	dnsRecord := previous.DeepCopy()
-	dnsRecord.Status.QueuedAt = reconcileStart
 
 	if dnsRecord.DeletionTimestamp != nil && !dnsRecord.DeletionTimestamp.IsZero() {
 		if err = r.deleteRecord(ctx, dnsRecord); err != nil {
@@ -326,14 +326,15 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
 	plan = plan.Calculate()
 
 	dnsRecord.Status.ValidFor = defaultRequeueTime.String()
+	dnsRecord.Status.QueuedAt = reconcileStart
 	if plan.Changes.HasChanges() {
 		// generation has not changed but there are changes.
 		// implies that they were overridden - bump write counter
 		if !generationChanged(dnsRecord) {
 			dnsRecord.Status.WriteCounter++
 		}
 		dnsRecord.Status.ValidFor = validationRequeueTime.String()
-		setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, "Awaiting validation", "Awaiting validation")
+		setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, "AwaitingValidation", "Awaiting validation")
 		logger.Info("Applying changes")
 		err = registry.ApplyChanges(ctx, plan.Changes)
 		if err != nil {

internal/controller/dnsrecord_controller_test.go

@@ -28,7 +28,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	externaldnsendpoint "sigs.k8s.io/external-dns/endpoint"
 
 	"github.com/kuadrant/dns-operator/api/v1alpha1"
 )
@@ -53,19 +52,7 @@ var _ = Describe("DNSRecordReconciler", func() {
 				ManagedZoneRef: &v1alpha1.ManagedZoneReference{
 					Name: managedZone.Name,
 				},
-				Endpoints: []*externaldnsendpoint.Endpoint{
-					{
-						DNSName: "foo.example.com",
-						Targets: []string{
-							"127.0.0.1",
-						},
-						RecordType:       "A",
-						SetIdentifier:    "",
-						RecordTTL:        60,
-						Labels:           nil,
-						ProviderSpecific: nil,
-					},
-				},
+				Endpoints: getTestEndpoints(),
 			},
 		}
 	})
@@ -96,6 +83,7 @@ var _ = Describe("DNSRecordReconciler", func() {
 				})),
 			)
 			g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
+			g.Expect(dnsRecord.Status.WriteCounter).To(BeZero())
 		}, TestTimeoutMedium, time.Second).Should(Succeed())
 	})
 
@@ -144,4 +132,31 @@ var _ = Describe("DNSRecordReconciler", func() {
 
 	})
 
+	It("should increase write counter if fail to publish record or record is overridden", func() {
+		dnsRecord.Spec.Endpoints = getTestNonExistingEndpoints()
+		Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
+
+		// should be requeue record for validation after the write attempt
+		Eventually(func(g Gomega) {
+			err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(dnsRecord.Status.Conditions).To(
+				ContainElement(MatchFields(IgnoreExtras, Fields{
+					"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+					"Status":             Equal(metav1.ConditionFalse),
+					"Reason":             Equal("AwaitingValidation"),
+					"Message":            Equal("Awaiting validation"),
+					"ObservedGeneration": Equal(dnsRecord.Generation),
+				})),
+			)
+		}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+		// should be increasing write counter
+		Eventually(func(g Gomega) {
+			err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(dnsRecord.Status.WriteCounter).To(BeNumerically(">", int64(0)))
+		}, TestTimeoutLong, time.Second).Should(Succeed())
+	})
+
 })

internal/controller/helper_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	externaldnsendpoint "sigs.k8s.io/external-dns/endpoint"
 
 	kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 )
@@ -14,6 +15,8 @@ const (
 	TestTimeoutMedium       = time.Second * 10
 	TestTimeoutLong         = time.Second * 30
 	TestRetryIntervalMedium = time.Millisecond * 250
+	RequeueDuration         = time.Second * 6
+	ValidityDuration        = time.Second * 3
 	defaultNS               = "default"
 	providerCredential      = "secretname"
 )
@@ -34,3 +37,35 @@ func testBuildManagedZone(name, ns, domainName string) *kuadrantdnsv1alpha1.Mana
 		},
 	}
 }
+
+func getTestEndpoints() []*externaldnsendpoint.Endpoint {
+	return []*externaldnsendpoint.Endpoint{
+		{
+			DNSName: "foo.example.com",
+			Targets: []string{
+				"127.0.0.1",
+			},
+			RecordType:       "A",
+			SetIdentifier:    "",
+			RecordTTL:        60,
+			Labels:           nil,
+			ProviderSpecific: nil,
+		},
+	}
+}
+
+func getTestNonExistingEndpoints() []*externaldnsendpoint.Endpoint {
+	return []*externaldnsendpoint.Endpoint{
+		{
+			DNSName: "nope.example.com",
+			Targets: []string{
+				"127.0.0.1",
+			},
+			RecordType:       "A",
+			SetIdentifier:    "",
+			RecordTTL:        60,
+			Labels:           nil,
+			ProviderSpecific: nil,
+		},
+	}
+}

internal/controller/suite_test.go

@@ -53,11 +53,6 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
-const (
-	RequeueDuration  = time.Minute * 1
-	ValidityDuration = time.Second * 30
-)
-
 var cfg *rest.Config
 var k8sClient client.Client
 var testEnv *envtest.Environment
@@ -67,7 +62,7 @@ var dnsProviderFactory = &providerFake.Factory{
 	ProviderForFunc: func(ctx context.Context, pa v1alpha1.ProviderAccessor, c provider.Config) (provider.Provider, error) {
 		return &providerFake.Provider{
 			RecordsFunc: func(context.Context) ([]*externaldnsendpoint.Endpoint, error) {
-				return []*externaldnsendpoint.Endpoint{}, nil
+				return getTestEndpoints(), nil
 			},
 			ApplyChangesFunc: func(context.Context, *externaldnsplan.Changes) error {
 				return nil

test/e2e/single_cluster_record_test.go

@@ -89,7 +89,7 @@ var _ = Describe("Single Cluster Record Test", func() {
 						"Status": Equal(metav1.ConditionTrue),
 					})),
 				)
-			}, 300*time.Second, 10*time.Second, ctx).Should(Succeed())
+			}, 20*time.Minute, 1*time.Minute, ctx).Should(Succeed())
 
 			By("ensuring the authoritative nameserver resolves the hostname")
 			// speed up things by using the authoritative nameserver
@@ -213,7 +213,7 @@ var _ = Describe("Single Cluster Record Test", func() {
 						"Status": Equal(metav1.ConditionTrue),
 					})),
 				)
-			}, 300*time.Second, 10*time.Second, ctx).Should(Succeed())
+			}, 20*time.Minute, 1*time.Minute, ctx).Should(Succeed())
 
 			By("ensuring the authoritative nameserver resolves the hostname")
 			// speed up things by using the authoritative nameserver
@@ -365,7 +365,7 @@ var _ = Describe("Single Cluster Record Test", func() {
 							"Status": Equal(metav1.ConditionTrue),
 						})),
 					)
-				}, 300*time.Second, 10*time.Second, ctx).Should(Succeed())
+				}, 20*time.Minute, 1*time.Minute, ctx).Should(Succeed())
 
 				By("ensuring the authoritative nameserver resolves the hostname")
 				// speed up things by using the authoritative nameserver
@@ -498,7 +498,7 @@ var _ = Describe("Single Cluster Record Test", func() {
 							"Status": Equal(metav1.ConditionTrue),
 						})),
 					)
-				}, 300*time.Second, 10*time.Second, ctx).Should(Succeed())
+				}, 20*time.Minute, 1*time.Minute, ctx).Should(Succeed())
 
 				By("ensuring the authoritative nameserver resolves the hostname")
 				// speed up things by using the authoritative nameserver