Skip to content

Commit

Permalink
fix: Ignore managed zone missing errors on deletion
Browse files Browse the repository at this point in the history
When we are deleting a DNSRecord we just ignore all errors relating to
the ManagedZone being missing since there is nothing we can do about it,
and we don't want the record staying around forever.

Check was missing for health checks
Kuadrant#78
  • Loading branch information
mikenairn authored and philbrookes committed May 3, 2024
1 parent 84ec0d5 commit 3c0620b
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/controller/dnsrecord_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
dnsRecord := previous.DeepCopy()

if dnsRecord.DeletionTimestamp != nil && !dnsRecord.DeletionTimestamp.IsZero() {
if err := r.ReconcileHealthChecks(ctx, dnsRecord); err != nil {
if err := r.ReconcileHealthChecks(ctx, dnsRecord); client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, err
}
requeueTime, err := r.deleteRecord(ctx, dnsRecord)
Expand Down
41 changes: 41 additions & 0 deletions internal/controller/dnsrecord_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
package controller

import (
"context"
"time"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -74,6 +75,46 @@ var _ = Describe("DNSRecordReconciler", func() {
}
})

It("can delete a record with an invalid managed zone", func(ctx SpecContext) {
dnsRecord = &v1alpha1.DNSRecord{
ObjectMeta: metav1.ObjectMeta{
Name: "foo.example.com",
Namespace: testNamespace,
},
Spec: v1alpha1.DNSRecordSpec{
ManagedZoneRef: &v1alpha1.ManagedZoneReference{
Name: "doesnotexist",
},
Endpoints: getTestEndpoints(),
},
}
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())

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("ProviderError"),
"Message": Equal("The DNS provider failed to ensure the record: ManagedZone.kuadrant.io \"doesnotexist\" not found"),
"ObservedGeneration": Equal(dnsRecord.Generation),
})),
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
}, TestTimeoutMedium, time.Second).Should(Succeed())

err := k8sClient.Delete(ctx, dnsRecord)
Expect(err).ToNot(HaveOccurred())

Eventually(func(g Gomega, ctx context.Context) {
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
g.Expect(err).To(HaveOccurred())
g.Expect(err).To(MatchError(ContainSubstring("not found")))
}, 5*time.Second, time.Second, ctx).Should(Succeed())
})

It("should have ready condition with status true", func() {
Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
Eventually(func(g Gomega) {
Expand Down

0 comments on commit 3c0620b

Please sign in to comment.