feat: validate atm profile when handling backend update (#208)

pkg/controllers/hub/trafficmanagerbackend/controller.go

@@ -212,7 +212,17 @@ func (r *Reconciler) handleUpdate(ctx context.Context, backend *fleetnetv1alpha1
 		// The controller will retry when err is not nil.
 		return ctrl.Result{}, err
 	}
-	klog.V(2).InfoS("Found the valid trafficManagerProfile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", klog.KObj(profile))
+	profileKObj := klog.KObj(profile)
+	klog.V(2).InfoS("Found the valid trafficManagerProfile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", profileKObj)
+
+	atmProfile, err := r.validateAzureTrafficManagerProfile(ctx, backend, profile)
+	if err != nil || atmProfile == nil {
+		// We don't need to requeue the invalid Azure Traffic Manager profile (err == nil and atmProfile == nil) as when
+		// the profile becomes valid, the controller will be re-triggered again.
+		// The controller will retry when err is not nil.
+		return ctrl.Result{}, err
+	}
+	klog.V(2).InfoS("Found the valid Azure Traffic Manager Profile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", profileKObj, "atmProfileName", atmProfile.Name)
 	return ctrl.Result{}, nil
 }
 
@@ -246,6 +256,35 @@ func (r *Reconciler) validateTrafficManagerProfile(ctx context.Context, backend
 	return nil, r.updateTrafficManagerBackendStatus(ctx, backend)
 }
 
+// validateAzureTrafficManagerProfile returns not nil Azure Traffic Manager profile when the atm profile is valid.
+func (r *Reconciler) validateAzureTrafficManagerProfile(ctx context.Context, backend *fleetnetv1alpha1.TrafficManagerBackend, profile *fleetnetv1alpha1.TrafficManagerProfile) (*armtrafficmanager.Profile, error) {
+	atmProfileName := generateAzureTrafficManagerProfileNameFunc(profile)
+	backendKObj := klog.KObj(backend)
+	profileKObj := klog.KObj(profile)
+	getRes, getErr := r.ProfilesClient.Get(ctx, r.ResourceGroupName, atmProfileName, nil)
+	if getErr != nil {
+		if azureerrors.IsNotFound(getErr) {
+			// We've already checked the TrafficManagerProfile condition before getting Azure resource.
+			// It may happen when
+			// 1. customers delete the azure profile manually
+			// 2. the TrafficManagerProfile info is stale.
+			// For the case 1, retry won't help to recover the Azure Traffic Manager profile resource.
+			// For the case 2, the controller will be re-triggered when the TrafficManagerProfile is updated.
+			klog.ErrorS(getErr, "NotFound Azure Traffic Manager profile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", profileKObj, "atmProfileName", atmProfileName)
+			// none of the endpoints are accepted by the TrafficManager
+			setFalseCondition(backend, nil, fmt.Sprintf("Azure Traffic Manager profile %q under %q is not found", atmProfileName, r.ResourceGroupName))
+			return nil, r.updateTrafficManagerBackendStatus(ctx, backend)
+		}
+		klog.V(2).InfoS("Failed to get Azure Traffic Manager profile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", profileKObj, "atmProfileName", atmProfileName)
+		setUnknownCondition(backend, fmt.Sprintf("Failed to get the Azure Traffic Manager profile %q under %q: %v", atmProfileName, r.ResourceGroupName, getErr))
+		if err := r.updateTrafficManagerBackendStatus(ctx, backend); err != nil {
+			return nil, err
+		}
+		return nil, getErr // need to return the error to requeue the request
+	}
+	return &getRes.Profile, nil
+}
+
 func setFalseCondition(backend *fleetnetv1alpha1.TrafficManagerBackend, acceptedEndpoints []fleetnetv1alpha1.TrafficManagerEndpointStatus, message string) {
 	cond := metav1.Condition{
 		Type:               string(fleetnetv1alpha1.TrafficManagerBackendConditionAccepted),

pkg/controllers/hub/trafficmanagerbackend/controller_integration_test.go

@@ -6,6 +6,9 @@ Licensed under the MIT license.
 package trafficmanagerbackend
 
 import (
+	"context"
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -19,6 +22,11 @@ import (
 	"go.goms.io/fleet-networking/test/common/trafficmanager/validator"
 )
 
+const (
+	timeout  = time.Second * 10
+	interval = time.Millisecond * 250
+)
+
 func trafficManagerBackendForTest(name, profileName, serviceImportName string) *fleetnetv1alpha1.TrafficManagerBackend {
 	return &fleetnetv1alpha1.TrafficManagerBackend{
 		ObjectMeta: metav1.ObjectMeta{
@@ -47,6 +55,37 @@ func trafficManagerProfileForTest(name string) *fleetnetv1alpha1.TrafficManagerP
 	}
 }
 
+func buildFalseCondition() []metav1.Condition {
+	return []metav1.Condition{
+		{
+			Status: metav1.ConditionFalse,
+			Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
+			Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
+		},
+	}
+}
+
+func buildUnknownCondition() []metav1.Condition {
+	return []metav1.Condition{
+		{
+			Status: metav1.ConditionUnknown,
+			Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
+			Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonPending),
+		},
+	}
+}
+
+func updateTrafficManagerProfileStatusToTrue(ctx context.Context, profile *fleetnetv1alpha1.TrafficManagerProfile) {
+	cond := metav1.Condition{
+		Status:             metav1.ConditionTrue,
+		Type:               string(fleetnetv1alpha1.TrafficManagerProfileConditionProgrammed),
+		ObservedGeneration: profile.Generation,
+		Reason:             string(fleetnetv1alpha1.TrafficManagerProfileReasonProgrammed),
+	}
+	meta.SetStatusCondition(&profile.Status.Conditions, cond)
+	Expect(k8sClient.Status().Update(ctx, profile)).Should(Succeed())
+}
+
 var _ = Describe("Test TrafficManagerBackend Controller", func() {
 	Context("When creating trafficManagerBackend with invalid profile", Ordered, func() {
 		name := fakeprovider.ValidBackendName
@@ -67,13 +106,7 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 				},
 				Spec: backend.Spec,
 				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
-					Conditions: []metav1.Condition{
-						{
-							Status: metav1.ConditionFalse,
-							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
-							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
-						},
-					},
+					Conditions: buildFalseCondition(),
 				},
 			}
 			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
@@ -109,14 +142,117 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 		})
 
 		It("Validating trafficManagerBackend", func() {
-			validator.IsTrafficManagerBackendFinalizerAdded(ctx, k8sClient, backendNamespacedName)
+			want := fleetnetv1alpha1.TrafficManagerBackend{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       backendName,
+					Namespace:  testNamespace,
+					Finalizers: []string{objectmeta.TrafficManagerBackendFinalizer},
+				},
+				Spec: backend.Spec,
+				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
+					Conditions: buildUnknownCondition(),
+				},
+			}
+			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
+		})
+
+		It("Updating TrafficManagerProfile status to programmed true and it should trigger controller", func() {
+			By("By updating TrafficManagerProfile status")
+			updateTrafficManagerProfileStatusToTrue(ctx, profile)
+		})
+
+		It("Validating trafficManagerBackend", func() {
+			want := fleetnetv1alpha1.TrafficManagerBackend{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       backendName,
+					Namespace:  testNamespace,
+					Finalizers: []string{objectmeta.TrafficManagerBackendFinalizer},
+				},
+				Spec: backend.Spec,
+				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
+					Conditions: buildFalseCondition(),
+				},
+			}
+			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
+		})
+
+		It("Deleting trafficManagerBackend", func() {
+			err := k8sClient.Delete(ctx, backend)
+			Expect(err).Should(Succeed(), "failed to delete trafficManagerBackend")
+		})
+
+		It("Validating trafficManagerBackend is deleted", func() {
+			validator.IsTrafficManagerBackendDeleted(ctx, k8sClient, backendNamespacedName)
+		})
+
+		It("Deleting trafficManagerProfile", func() {
+			err := k8sClient.Delete(ctx, profile)
+			Expect(err).Should(Succeed(), "failed to delete trafficManagerProfile")
+		})
+
+		It("Validating trafficManagerProfile is deleted", func() {
+			validator.IsTrafficManagerProfileDeleted(ctx, k8sClient, profileNamespacedName)
+		})
+	})
+
+	Context("When creating trafficManagerBackend and failing to get Azure Traffic Manager profile", Ordered, func() {
+		profileName := fakeprovider.RequestTimeoutProfileName
+		profileNamespacedName := types.NamespacedName{Namespace: testNamespace, Name: profileName}
+		var profile *fleetnetv1alpha1.TrafficManagerProfile
+		backendName := fakeprovider.ValidBackendName
+		backendNamespacedName := types.NamespacedName{Namespace: testNamespace, Name: backendName}
+		var backend *fleetnetv1alpha1.TrafficManagerBackend
+
+		It("Creating a new TrafficManagerProfile", func() {
+			By("By creating a new TrafficManagerProfile")
+			profile = trafficManagerProfileForTest(profileName)
+			Expect(k8sClient.Create(ctx, profile)).Should(Succeed())
+		})
+
+		It("Updating TrafficManagerProfile status to programmed true", func() {
+			By("By updating TrafficManagerProfile status")
+			updateTrafficManagerProfileStatusToTrue(ctx, profile)
+		})
+
+		It("Creating TrafficManagerBackend", func() {
+			backend = trafficManagerBackendForTest(backendName, profileName, "not-exist")
+			Expect(k8sClient.Create(ctx, backend)).Should(Succeed())
+		})
+
+		It("Validating trafficManagerBackend", func() {
+			want := fleetnetv1alpha1.TrafficManagerBackend{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       backendName,
+					Namespace:  testNamespace,
+					Finalizers: []string{objectmeta.TrafficManagerBackendFinalizer},
+				},
+				Spec: backend.Spec,
+				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
+					Conditions: buildUnknownCondition(),
+				},
+			}
+			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
 		})
 
 		It("Deleting trafficManagerBackend", func() {
 			err := k8sClient.Delete(ctx, backend)
 			Expect(err).Should(Succeed(), "failed to delete trafficManagerBackend")
 		})
 
+		It("Validating trafficManagerBackend cannot be deleted", func() {
+			validator.ValidateTrafficManagerConsistentlyExist(ctx, k8sClient, backendNamespacedName)
+		})
+
+		It("Removing the finalizer from trafficManagerBackend", func() {
+			Eventually(func() error {
+				if err := k8sClient.Get(ctx, backendNamespacedName, backend); err != nil {
+					return err
+				}
+				backend.Finalizers = nil
+				return k8sClient.Update(ctx, backend)
+			}, timeout, interval).Should(Succeed(), "failed to remove trafficManagerBackend finalizer")
+		})
+
 		It("Validating trafficManagerBackend is deleted", func() {
 			validator.IsTrafficManagerBackendDeleted(ctx, k8sClient, backendNamespacedName)
 		})
@@ -145,6 +281,11 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 			Expect(k8sClient.Create(ctx, profile)).Should(Succeed())
 		})
 
+		It("Updating TrafficManagerProfile status to programmed true", func() {
+			By("By updating TrafficManagerProfile status")
+			updateTrafficManagerProfileStatusToTrue(ctx, profile)
+		})
+
 		It("Creating TrafficManagerBackend", func() {
 			backend = trafficManagerBackendForTest(backendName, profileName, "not-exist")
 			Expect(k8sClient.Create(ctx, backend)).Should(Succeed())
@@ -187,6 +328,11 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 			Expect(k8sClient.Create(ctx, profile)).Should(Succeed())
 		})
 
+		It("Updating TrafficManagerProfile status to programmed true", func() {
+			By("By updating TrafficManagerProfile status")
+			updateTrafficManagerProfileStatusToTrue(ctx, profile)
+		})
+
 		It("Creating TrafficManagerBackend", func() {
 			backend = trafficManagerBackendForTest(backendName, profileName, "not-exist")
 			Expect(k8sClient.Create(ctx, backend)).Should(Succeed())
@@ -229,7 +375,7 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 			Expect(k8sClient.Create(ctx, profile)).Should(Succeed())
 		})
 
-		It("Updating TrafficManagerProfile status to accepted false", func() {
+		It("Updating TrafficManagerProfile status to programmed false", func() {
 			By("By updating TrafficManagerProfile status")
 			cond := metav1.Condition{
 				Status:             metav1.ConditionFalse,
@@ -255,13 +401,7 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 				},
 				Spec: backend.Spec,
 				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
-					Conditions: []metav1.Condition{
-						{
-							Status: metav1.ConditionFalse,
-							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
-							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
-						},
-					},
+					Conditions: buildFalseCondition(),
 				},
 			}
 			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
@@ -288,13 +428,7 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 				},
 				Spec: backend.Spec,
 				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
-					Conditions: []metav1.Condition{
-						{
-							Status: metav1.ConditionUnknown,
-							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
-							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonPending),
-						},
-					},
+					Conditions: buildUnknownCondition(),
 				},
 			}
 			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)

pkg/controllers/hub/trafficmanagerbackend/suite_test.go

@@ -21,6 +21,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -54,7 +55,9 @@ func TestAPIs(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
-	klog.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+	logger := zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))
+	klog.SetLogger(logger)
+	log.SetLogger(logger)
 
 	ctx, cancel = context.WithCancel(context.TODO())
 

pkg/controllers/hub/trafficmanagerprofile/suite_test.go

@@ -21,6 +21,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -53,7 +54,9 @@ func TestAPIs(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
-	klog.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+	logger := zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))
+	klog.SetLogger(logger)
+	log.SetLogger(logger)
 
 	ctx, cancel = context.WithCancel(context.TODO())
 

test/common/trafficmanager/fakeprovider/profile.go

@@ -30,6 +30,7 @@ const (
 	ConflictErrProfileName            = "conflict-err-profile"
 	InternalServerErrProfileName      = "internal-server-err-profile"
 	ThrottledErrProfileName           = "throttled-err-profile"
+	RequestTimeoutProfileName         = "request-timeout-profile"
 
 	ValidBackendName  = "valid-backend"
 	ServiceImportName = "test-import"
@@ -116,6 +117,8 @@ func ProfileGet(_ context.Context, resourceGroupName string, profileName string,
 				Location: ptr.To("global"),
 			}}
 		resp.SetResponse(http.StatusOK, profileResp, nil)
+	case RequestTimeoutProfileName:
+		errResp.SetResponseError(http.StatusRequestTimeout, "RequestTimeoutError")
 	default:
 		errResp.SetResponseError(http.StatusNotFound, "NotFoundError")
 	}

test/common/trafficmanager/validator/backend.go

@@ -67,3 +67,14 @@ func IsTrafficManagerBackendDeleted(ctx context.Context, k8sClient client.Client
 		return nil
 	}, timeout, interval).Should(gomega.Succeed(), "Failed to remove trafficManagerBackend %s ", name)
 }
+
+// ValidateTrafficManagerConsistentlyExist validates whether the backend consistently exists.
+func ValidateTrafficManagerConsistentlyExist(ctx context.Context, k8sClient client.Client, name types.NamespacedName) {
+	gomega.Consistently(func() error {
+		backend := &fleetnetv1alpha1.TrafficManagerBackend{}
+		if err := k8sClient.Get(ctx, name, backend); errors.IsNotFound(err) {
+			return fmt.Errorf("trafficManagerBackend %s does not exist: %w", name, err)
+		}
+		return nil
+	}, duration, interval).Should(gomega.Succeed(), "Failed to find trafficManagerBackend %s ", name)
+}

test/common/trafficmanager/validator/profile.go

@@ -24,6 +24,8 @@ import (
 const (
 	timeout  = time.Second * 10
 	interval = time.Millisecond * 250
+	// duration used by consistently
+	duration = time.Second * 30
 )
 
 var (