feat: handle invalid profile when handling backend update (#207)

pkg/controllers/hub/trafficmanagerbackend/controller.go

@@ -16,6 +16,8 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/trafficmanager/armtrafficmanager"
 	"golang.org/x/sync/errgroup"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -24,6 +26,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"go.goms.io/fleet/pkg/utils/condition"
 	"go.goms.io/fleet/pkg/utils/controller"
 
 	fleetnetv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
@@ -200,10 +203,87 @@ func isEndpointOwnedByBackend(backend *fleetnetv1alpha1.TrafficManagerBackend, e
 	return strings.HasPrefix(strings.ToLower(endpoint), generateAzureTrafficManagerEndpointNamePrefixFunc(backend))
 }
 
-func (r *Reconciler) handleUpdate(_ context.Context, _ *fleetnetv1alpha1.TrafficManagerBackend) (ctrl.Result, error) {
+func (r *Reconciler) handleUpdate(ctx context.Context, backend *fleetnetv1alpha1.TrafficManagerBackend) (ctrl.Result, error) {
+	backendKObj := klog.KObj(backend)
+	profile, err := r.validateTrafficManagerProfile(ctx, backend)
+	if err != nil || profile == nil {
+		// We don't need to requeue the invalid Profile (err == nil and profile == nil) because 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 trafficManagerProfile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", klog.KObj(profile))
 	return ctrl.Result{}, nil
 }
 
+// validateTrafficManagerProfile returns not nil profile when the profile is valid.
+func (r *Reconciler) validateTrafficManagerProfile(ctx context.Context, backend *fleetnetv1alpha1.TrafficManagerBackend) (*fleetnetv1alpha1.TrafficManagerProfile, error) {
+	backendKObj := klog.KObj(backend)
+	var cond metav1.Condition
+	profile := &fleetnetv1alpha1.TrafficManagerProfile{}
+	if getProfileErr := r.Client.Get(ctx, types.NamespacedName{Name: backend.Spec.Profile.Name, Namespace: backend.Namespace}, profile); getProfileErr != nil {
+		if apierrors.IsNotFound(getProfileErr) {
+			klog.V(2).InfoS("NotFound trafficManagerProfile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", backend.Spec.Profile.Name)
+			setFalseCondition(backend, nil, fmt.Sprintf("TrafficManagerProfile %q is not found", backend.Spec.Profile.Name))
+			return nil, r.updateTrafficManagerBackendStatus(ctx, backend)
+		}
+		klog.ErrorS(getProfileErr, "Failed to get trafficManagerProfile", "trafficManagerBackend", backendKObj, "trafficManagerProfile", backend.Spec.Profile.Name)
+		setUnknownCondition(backend, fmt.Sprintf("Failed to get the trafficManagerProfile %q: %v", backend.Spec.Profile.Name, getProfileErr))
+		if err := r.updateTrafficManagerBackendStatus(ctx, backend); err != nil {
+			return nil, err
+		}
+		return nil, getProfileErr // need to return the error to requeue the request
+	}
+	programmedCondition := meta.FindStatusCondition(profile.Status.Conditions, string(fleetnetv1alpha1.TrafficManagerProfileConditionProgrammed))
+	if condition.IsConditionStatusTrue(programmedCondition, profile.GetGeneration()) {
+		return profile, nil // return directly if the trafficManagerProfile is programmed
+	} else if condition.IsConditionStatusFalse(programmedCondition, profile.GetGeneration()) {
+		setFalseCondition(backend, nil, fmt.Sprintf("Invalid trafficManagerProfile %q: %v", backend.Spec.Profile.Name, programmedCondition.Message))
+	} else {
+		setUnknownCondition(backend, fmt.Sprintf("In the processing of trafficManagerProfile %q", backend.Spec.Profile.Name))
+	}
+	klog.V(2).InfoS("Profile has not been accepted and updating the status", "trafficManagerBackend", backendKObj, "condition", cond)
+	return nil, r.updateTrafficManagerBackendStatus(ctx, backend)
+}
+
+func setFalseCondition(backend *fleetnetv1alpha1.TrafficManagerBackend, acceptedEndpoints []fleetnetv1alpha1.TrafficManagerEndpointStatus, message string) {
+	cond := metav1.Condition{
+		Type:               string(fleetnetv1alpha1.TrafficManagerBackendConditionAccepted),
+		Status:             metav1.ConditionFalse,
+		ObservedGeneration: backend.Generation,
+		Reason:             string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
+		Message:            message,
+	}
+	if len(acceptedEndpoints) == 0 {
+		backend.Status.Endpoints = []fleetnetv1alpha1.TrafficManagerEndpointStatus{}
+	} else {
+		backend.Status.Endpoints = acceptedEndpoints
+	}
+	meta.SetStatusCondition(&backend.Status.Conditions, cond)
+}
+
+func setUnknownCondition(backend *fleetnetv1alpha1.TrafficManagerBackend, message string) {
+	cond := metav1.Condition{
+		Type:               string(fleetnetv1alpha1.TrafficManagerBackendConditionAccepted),
+		Status:             metav1.ConditionUnknown,
+		ObservedGeneration: backend.Generation,
+		Reason:             string(fleetnetv1alpha1.TrafficManagerBackendReasonPending),
+		Message:            message,
+	}
+	backend.Status.Endpoints = []fleetnetv1alpha1.TrafficManagerEndpointStatus{}
+	meta.SetStatusCondition(&backend.Status.Conditions, cond)
+}
+
+func (r *Reconciler) updateTrafficManagerBackendStatus(ctx context.Context, backend *fleetnetv1alpha1.TrafficManagerBackend) error {
+	backendKObj := klog.KObj(backend)
+	if err := r.Client.Status().Update(ctx, backend); err != nil {
+		klog.ErrorS(err, "Failed to update trafficManagerBackend status", "trafficManagerBackend", backendKObj)
+		return controller.NewUpdateIgnoreConflictError(err)
+	}
+	klog.V(2).InfoS("Updated trafficManagerBackend status", "trafficManagerBackend", backendKObj, "status", backend.Status)
+	return nil
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	// set up an index for efficient trafficManagerBackend lookup

pkg/controllers/hub/trafficmanagerbackend/controller_integration_test.go

@@ -8,11 +8,13 @@ package trafficmanagerbackend
 import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 
 	fleetnetv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
+	"go.goms.io/fleet-networking/pkg/common/objectmeta"
 	"go.goms.io/fleet-networking/test/common/trafficmanager/fakeprovider"
 	"go.goms.io/fleet-networking/test/common/trafficmanager/validator"
 )
@@ -57,7 +59,24 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 		})
 
 		It("Validating trafficManagerBackend", func() {
-			validator.IsTrafficManagerBackendFinalizerAdded(ctx, k8sClient, namespacedName)
+			want := fleetnetv1alpha1.TrafficManagerBackend{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       name,
+					Namespace:  testNamespace,
+					Finalizers: []string{objectmeta.TrafficManagerBackendFinalizer},
+				},
+				Spec: backend.Spec,
+				Status: fleetnetv1alpha1.TrafficManagerBackendStatus{
+					Conditions: []metav1.Condition{
+						{
+							Status: metav1.ConditionFalse,
+							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
+							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
+						},
+					},
+				},
+			}
+			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
 		})
 
 		It("Deleting trafficManagerBackend", func() {
@@ -195,4 +214,108 @@ var _ = Describe("Test TrafficManagerBackend Controller", func() {
 			validator.IsTrafficManagerProfileDeleted(ctx, k8sClient, profileNamespacedName)
 		})
 	})
+
+	Context("When creating trafficManagerBackend with not accepted profile", Ordered, func() {
+		profileName := fakeprovider.ValidProfileWithEndpointsName
+		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 accepted false", func() {
+			By("By updating TrafficManagerProfile status")
+			cond := metav1.Condition{
+				Status:             metav1.ConditionFalse,
+				Type:               string(fleetnetv1alpha1.TrafficManagerProfileConditionProgrammed),
+				ObservedGeneration: profile.Generation,
+				Reason:             string(fleetnetv1alpha1.TrafficManagerProfileReasonInvalid),
+			}
+			meta.SetStatusCondition(&profile.Status.Conditions, cond)
+			Expect(k8sClient.Status().Update(ctx, profile)).Should(Succeed())
+		})
+
+		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: []metav1.Condition{
+						{
+							Status: metav1.ConditionFalse,
+							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
+							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonInvalid),
+						},
+					},
+				},
+			}
+			validator.ValidateTrafficManagerBackend(ctx, k8sClient, &want)
+		})
+
+		It("Updating TrafficManagerProfile status to accepted unknown and it should trigger controller", func() {
+			By("By updating TrafficManagerProfile status")
+			cond := metav1.Condition{
+				Status:             metav1.ConditionUnknown,
+				Type:               string(fleetnetv1alpha1.TrafficManagerProfileConditionProgrammed),
+				ObservedGeneration: profile.Generation,
+				Reason:             string(fleetnetv1alpha1.TrafficManagerProfileReasonPending),
+			}
+			meta.SetStatusCondition(&profile.Status.Conditions, cond)
+			Expect(k8sClient.Status().Update(ctx, profile)).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: []metav1.Condition{
+						{
+							Status: metav1.ConditionUnknown,
+							Type:   string(fleetnetv1alpha1.TrafficManagerBackendReasonAccepted),
+							Reason: string(fleetnetv1alpha1.TrafficManagerBackendReasonPending),
+						},
+					},
+				},
+			}
+			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)
+		})
+	})
 })

test/common/trafficmanager/validator/backend.go

@@ -9,6 +9,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
@@ -19,6 +21,13 @@ import (
 	"go.goms.io/fleet-networking/pkg/common/objectmeta"
 )
 
+var (
+	cmpTrafficManagerBackendOptions = cmp.Options{
+		commonCmpOptions,
+		cmpopts.IgnoreFields(fleetnetv1alpha1.TrafficManagerBackend{}, "TypeMeta"),
+	}
+)
+
 // IsTrafficManagerBackendFinalizerAdded validates whether the backend is created with the finalizer or not.
 func IsTrafficManagerBackendFinalizerAdded(ctx context.Context, k8sClient client.Client, name types.NamespacedName) {
 	gomega.Eventually(func() error {
@@ -33,6 +42,21 @@ func IsTrafficManagerBackendFinalizerAdded(ctx context.Context, k8sClient client
 	}, timeout, interval).Should(gomega.Succeed(), "Failed to add finalizer to trafficManagerBackend %s", name)
 }
 
+// ValidateTrafficManagerBackend validates the trafficManagerBackend object.
+func ValidateTrafficManagerBackend(ctx context.Context, k8sClient client.Client, want *fleetnetv1alpha1.TrafficManagerBackend) {
+	key := types.NamespacedName{Name: want.Name, Namespace: want.Namespace}
+	backend := &fleetnetv1alpha1.TrafficManagerBackend{}
+	gomega.Eventually(func() error {
+		if err := k8sClient.Get(ctx, key, backend); err != nil {
+			return err
+		}
+		if diff := cmp.Diff(want, backend, cmpTrafficManagerBackendOptions); diff != "" {
+			return fmt.Errorf("trafficManagerBackend mismatch (-want, +got) :\n%s", diff)
+		}
+		return nil
+	}, timeout, interval).Should(gomega.Succeed(), "Get() trafficManagerBackend mismatch")
+}
+
 // IsTrafficManagerBackendDeleted validates whether the backend is deleted or not.
 func IsTrafficManagerBackendDeleted(ctx context.Context, k8sClient client.Client, name types.NamespacedName) {
 	gomega.Eventually(func() error {

test/common/trafficmanager/validator/profile.go

@@ -30,15 +30,15 @@ var (
 	commonCmpOptions = cmp.Options{
 		cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion", "UID", "CreationTimestamp", "ManagedFields", "Generation"),
 		cmpopts.IgnoreFields(metav1.OwnerReference{}, "UID"),
-	}
-	cmpTrafficManagerProfileOptions = cmp.Options{
-		commonCmpOptions,
-		cmpopts.IgnoreFields(fleetnetv1alpha1.TrafficManagerProfile{}, "TypeMeta"),
 		cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime", "ObservedGeneration"),
 		cmpopts.SortSlices(func(c1, c2 metav1.Condition) bool {
 			return c1.Type < c2.Type
 		}),
 	}
+	cmpTrafficManagerProfileOptions = cmp.Options{
+		commonCmpOptions,
+		cmpopts.IgnoreFields(fleetnetv1alpha1.TrafficManagerProfile{}, "TypeMeta"),
+	}
 )
 
 // ValidateTrafficManagerProfile validates the trafficManagerProfile object.