From deb57ac974174fe187b64c544525ddb9369201a7 Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Wed, 3 Jul 2024 23:29:28 +0900 Subject: [PATCH] cleanup: move the truncate and tidy tests Signed-off-by: Kensei Nakada --- internal/gatewayapi/status/gateway.go | 50 +++++++++++--------- internal/gatewayapi/status/gateway_test.go | 55 +++++++++++++++------- 2 files changed, 65 insertions(+), 40 deletions(-) diff --git a/internal/gatewayapi/status/gateway.go b/internal/gatewayapi/status/gateway.go index a5aae828887..8db4c282aac 100644 --- 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 index 2d2af9a3d06..0bd822d095e 100644 --- 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) + } }) } }