cleanup: move the truncate and tidy tests

Signed-off-by: Kensei Nakada <handbomusic@gmail.com>
envoyproxy · Jul 3, 2024 · deb57ac · deb57ac
1 parent e3c93ac
commit deb57ac
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 40 deletions.
diff --git a/internal/gatewayapi/status/gateway.go b/internal/gatewayapi/status/gateway.go
@@ -98,13 +98,7 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Ser
 	}
 
 	// Update the programmed condition.
-	gw.Status.Conditions = MergeConditions(gw.Status.Conditions, computeGatewayProgrammedCondition(gw, deployment))
-
-	if len(gw.Status.Addresses) > 16 {
-		// Truncate the addresses to 16
-		// so that the status can be updated successfully.
-		gw.Status.Addresses = gw.Status.Addresses[:16]
-	}
+	computeGatewayProgrammedCondition(gw, deployment)
 }
 
 func SetGatewayListenerStatusCondition(gateway *gwapiv1.Gateway, listenerStatusIdx int,
@@ -135,33 +129,45 @@ func computeGatewayAcceptedCondition(gw *gwapiv1.Gateway, accepted bool) metav1.
 	}
 }
 
+const (
+	messageAddressNotAssigned  = "No addresses have been assigned to the Gateway"
+	messageFmtTooManyAddresses = "Too many addresses (%d) have been assigned to the Gateway, the maximum number of addresses is 16"
+	messageNoResources         = "Deployment replicas unavailable"
+	messageFmtProgrammed       = "Address assigned to the Gateway, %d/%d envoy Deployment replicas available"
+)
+
 // computeGatewayProgrammedCondition computes the Gateway Programmed status condition.
 // Programmed condition surfaces true when the Envoy Deployment status is ready.
-func computeGatewayProgrammedCondition(gw *gwapiv1.Gateway, deployment *appsv1.Deployment) metav1.Condition {
+func computeGatewayProgrammedCondition(gw *gwapiv1.Gateway, deployment *appsv1.Deployment) {
 	if len(gw.Status.Addresses) == 0 {
-		return newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse,
-			string(gwapiv1.GatewayReasonAddressNotAssigned),
-			"No addresses have been assigned to the Gateway", time.Now(), gw.Generation)
+		gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
+			newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonAddressNotAssigned),
+				messageAddressNotAssigned, time.Now(), gw.Generation))
+		return
 	}
 
 	if len(gw.Status.Addresses) > 16 {
-		return newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse,
-			string(gwapiv1.GatewayReasonInvalid),
-			fmt.Sprintf("Too many addresses (%d) have been assigned to the Gateway, the maximum number of addresses is 16",
-				len(gw.Status.Addresses)), time.Now(), gw.Generation)
+		gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
+			newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonInvalid),
+				fmt.Sprintf(messageFmtTooManyAddresses, len(gw.Status.Addresses)), time.Now(), gw.Generation))
+
+		// Truncate the addresses to 16
+		// so that the status can be updated successfully.
+		gw.Status.Addresses = gw.Status.Addresses[:16]
+		return
 	}
 
 	// If there are no available replicas for the Envoy Deployment, don't
 	// mark the Gateway as ready yet.
 
 	if deployment == nil || deployment.Status.AvailableReplicas == 0 {
-		return newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse,
-			string(gwapiv1.GatewayReasonNoResources),
-			"Deployment replicas unavailable", time.Now(), gw.Generation)
+		gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
+			newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionFalse, string(gwapiv1.GatewayReasonNoResources),
+				messageNoResources, time.Now(), gw.Generation))
+		return
 	}
 
-	message := fmt.Sprintf("Address assigned to the Gateway, %d/%d envoy Deployment replicas available",
-		deployment.Status.AvailableReplicas, deployment.Status.Replicas)
-	return newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionTrue,
-		string(gwapiv1.GatewayConditionProgrammed), message, time.Now(), gw.Generation)
+	gw.Status.Conditions = MergeConditions(gw.Status.Conditions,
+		newCondition(string(gwapiv1.GatewayConditionProgrammed), metav1.ConditionTrue, string(gwapiv1.GatewayConditionProgrammed),
+			fmt.Sprintf(messageFmtProgrammed, deployment.Status.AvailableReplicas, deployment.Status.Replicas), time.Now(), gw.Generation))
 }
diff --git a/internal/gatewayapi/status/gateway_test.go b/internal/gatewayapi/status/gateway_test.go
@@ -6,10 +6,13 @@
 package status
 
 import (
+	"fmt"
 	"reflect"
 	"strconv"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -190,42 +193,58 @@ func TestGatewayReadyCondition(t *testing.T) {
 		// serviceAddressNum indicates how many addresses are set in the Gateway status.
 		serviceAddressNum int
 		deploymentStatus  appsv1.DeploymentStatus
-		expect            metav1.Condition
+		expectCondition   []metav1.Condition
 	}{
 		{
 			name:              "ready gateway",
 			serviceAddressNum: 1,
-			deploymentStatus:  appsv1.DeploymentStatus{AvailableReplicas: 1},
-			expect: metav1.Condition{
-				Status: metav1.ConditionTrue,
-				Reason: string(gwapiv1.GatewayConditionProgrammed),
+			deploymentStatus:  appsv1.DeploymentStatus{AvailableReplicas: 1, Replicas: 1},
+			expectCondition: []metav1.Condition{
+				{
+					Type:    string(gwapiv1.GatewayConditionProgrammed),
+					Status:  metav1.ConditionTrue,
+					Reason:  string(gwapiv1.GatewayConditionProgrammed),
+					Message: fmt.Sprintf(messageFmtProgrammed, 1, 1),
+				},
 			},
 		},
 		{
 			name:              "not ready gateway without address",
 			serviceAddressNum: 0,
 			deploymentStatus:  appsv1.DeploymentStatus{AvailableReplicas: 1},
-			expect: metav1.Condition{
-				Status: metav1.ConditionFalse,
-				Reason: string(gwapiv1.GatewayReasonAddressNotAssigned),
+			expectCondition: []metav1.Condition{
+				{
+					Type:    string(gwapiv1.GatewayConditionProgrammed),
+					Status:  metav1.ConditionFalse,
+					Reason:  string(gwapiv1.GatewayReasonAddressNotAssigned),
+					Message: messageAddressNotAssigned,
+				},
 			},
 		},
 		{
 			name:              "not ready gateway with too many addresses",
 			serviceAddressNum: 17,
 			deploymentStatus:  appsv1.DeploymentStatus{AvailableReplicas: 1},
-			expect: metav1.Condition{
-				Status: metav1.ConditionFalse,
-				Reason: string(gwapiv1.GatewayReasonInvalid),
+			expectCondition: []metav1.Condition{
+				{
+					Type:    string(gwapiv1.GatewayConditionProgrammed),
+					Status:  metav1.ConditionFalse,
+					Reason:  string(gwapiv1.GatewayReasonInvalid),
+					Message: fmt.Sprintf(messageFmtTooManyAddresses, 17),
+				},
 			},
 		},
 		{
 			name:              "not ready gateway with address unavailable pods",
 			serviceAddressNum: 1,
 			deploymentStatus:  appsv1.DeploymentStatus{AvailableReplicas: 0},
-			expect: metav1.Condition{
-				Status: metav1.ConditionFalse,
-				Reason: string(gwapiv1.GatewayReasonNoResources),
+			expectCondition: []metav1.Condition{
+				{
+					Type:    string(gwapiv1.GatewayConditionProgrammed),
+					Status:  metav1.ConditionFalse,
+					Reason:  string(gwapiv1.GatewayReasonNoResources),
+					Message: messageNoResources,
+				},
 			},
 		},
 	}
@@ -244,11 +263,11 @@ func TestGatewayReadyCondition(t *testing.T) {
 			}
 
 			deployment := &appsv1.Deployment{Status: tc.deploymentStatus}
-			got := computeGatewayProgrammedCondition(gtw, deployment)
+			computeGatewayProgrammedCondition(gtw, deployment)
 
-			assert.Equal(t, string(gwapiv1.GatewayConditionProgrammed), got.Type)
-			assert.Equal(t, tc.expect.Status, got.Status)
-			assert.Equal(t, tc.expect.Reason, got.Reason)
+			if d := cmp.Diff(tc.expectCondition, gtw.Status.Conditions, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")); d != "" {
+				t.Errorf("unexpected condition diff: %s", d)
+			}
 		})
 	}
 }