fix: treat status conditions of gateway and gatewayclass as snapshots (…

…#2791)

internal/controllers/gateway/gateway_controller.go

@@ -301,14 +301,15 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
  // on the object is ready to begin.
  info(log, gateway, "marking gateway as scheduled")
  if !isGatewayScheduled(gateway) {
- gateway.Status.Conditions = append(gateway.Status.Conditions, metav1.Condition{
+ scheduledCondition := metav1.Condition{
  Type: string(gatewayv1alpha2.GatewayConditionScheduled),
  Status: metav1.ConditionTrue,
  ObservedGeneration: gateway.Generation,
  LastTransitionTime: metav1.Now(),
  Reason: string(gatewayv1alpha2.GatewayReasonScheduled),
  Message: "this unmanaged gateway has been picked up by the controller and will be processed",
- })
+ }
+ setGatewayCondition(gateway, scheduledCondition)
  return ctrl.Result{}, r.Status().Update(ctx, pruneGatewayStatusConds(gateway))
  }
 
@@ -563,14 +564,15 @@ func (r *GatewayReconciler) updateAddressesAndListenersStatus(
  if !isGatewayReady(gateway) {
  gateway.Status.Listeners = listenerStatuses
  gateway.Status.Addresses = gateway.Spec.Addresses
- gateway.Status.Conditions = append(gateway.Status.Conditions, metav1.Condition{
+ readyCondition := metav1.Condition{
  Type: string(gatewayv1alpha2.GatewayConditionReady),
  Status: metav1.ConditionTrue,
  ObservedGeneration: gateway.Generation,
  LastTransitionTime: metav1.Now(),
  Reason: string(gatewayv1alpha2.GatewayReasonReady),
  Message: "addresses and listeners for the Gateway resource were successfully updated",
- })
+ }
+ setGatewayCondition(gateway, readyCondition)
  return true, r.Status().Update(ctx, pruneGatewayStatusConds(gateway))
  }
  return false, nil

internal/controllers/gateway/gateway_controller_test.go

@@ -57,6 +57,110 @@ func Test_readyConditionExistsForObservedGeneration(t *testing.T) {
  assert.False(t, isGatewayReady(neverBeenReadyGateway))
 }
 
+func Test_setGatewayCondtion(t *testing.T) {
+ testCases := []struct {
+ name string
+ gw *gatewayv1alpha2.Gateway
+ condition metav1.Condition
+ conditionLength int
+ }{
+ {
+ name: "no_such_condition_should_append_one",
+ gw: &gatewayv1alpha2.Gateway{},
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 1,
+ },
+ {
+ name: "have_condition_with_type_should_replace",
+ gw: &gatewayv1alpha2.Gateway{
+ Status: gatewayv1alpha2.GatewayStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: "fake1",
+ Status: metav1.ConditionFalse,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ },
+ },
+ },
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 1,
+ },
+ {
+ name: "multiple_conditions_with_type_should_preserve_one",
+ gw: &gatewayv1alpha2.Gateway{
+ Status: gatewayv1alpha2.GatewayStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: "fake1",
+ Status: metav1.ConditionFalse,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ {
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake2",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ {
+ Type: "fake2",
+ Status: metav1.ConditionTrue,
+ Reason: "fake2",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ },
+ },
+ },
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 3,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 2,
+ },
+ }
+
+ for _, tc := range testCases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ setGatewayCondition(tc.gw, tc.condition)
+ t.Logf("checking conditions of gateway after setting")
+ assert.Len(t, tc.gw.Status.Conditions, tc.conditionLength)
+
+ conditionNum := 0
+ var observedCondition metav1.Condition
+ for _, condition := range tc.gw.Status.Conditions {
+ if condition.Type == tc.condition.Type {
+ conditionNum++
+ observedCondition = condition
+ }
+ }
+ assert.Equal(t, 1, conditionNum)
+ assert.EqualValues(t, tc.condition, observedCondition)
+ })
+ }
+}
+
 func Test_isGatewayMarkedAsScheduled(t *testing.T) {
  t.Log("verifying scheduled check for gateway object which has been scheduled")
  scheduledGateway := &gatewayv1alpha2.Gateway{

internal/controllers/gateway/gateway_utils.go

@@ -25,6 +25,21 @@ const (
  maxConds = 8
 )
 
+// setGatewayCondition sets the condition with specified type in gateway status
+// to expected condition in newCondition.
+// if the gateway status does not contain a condition with that type, add one more condition.
+// if the gateway status contains condition(s) with the type, then replace with the new condition.
+func setGatewayCondition(gateway *gatewayv1alpha2.Gateway, newCondition metav1.Condition) {
+ newConditions := []metav1.Condition{}
+ for _, condition := range gateway.Status.Conditions {
+ if condition.Type != newCondition.Type {
+ newConditions = append(newConditions, condition)
+ }
+ }
+ newConditions = append(newConditions, newCondition)
+ gateway.Status.Conditions = newConditions
+}
+
 // isGatewayScheduled returns boolean whether or not the gateway object was scheduled
 // previously by the gateway controller.
 func isGatewayScheduled(gateway *gatewayv1alpha2.Gateway) bool {

internal/controllers/gateway/gatewayclass_controller.go

@@ -90,14 +90,15 @@ func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
  }
 
  if !alreadyAccepted {
- gwc.Status.Conditions = append(gwc.Status.Conditions, metav1.Condition{
+ acceptedCondtion := metav1.Condition{
  Type: string(gatewayv1alpha2.GatewayClassConditionStatusAccepted),
  Status: metav1.ConditionTrue,
  ObservedGeneration: gwc.Generation,
  LastTransitionTime: metav1.Now(),
  Reason: string(gatewayv1alpha2.GatewayClassReasonAccepted),
  Message: "the gatewayclass has been accepted by the controller",
- })
+ }
+ setGatewayClassCondition(gwc, acceptedCondtion)
  return ctrl.Result{}, r.Status().Update(ctx, pruneGatewayClassStatusConds(gwc))
  }
  }
@@ -118,3 +119,18 @@ func pruneGatewayClassStatusConds(gwc *gatewayv1alpha2.GatewayClass) *gatewayv1a
  }
  return gwc
 }
+
+// setGatewayClassCondition sets the condition with specified type in gatewayclass status
+// to expected condition in newCondition.
+// if the gatewayclass status does not contain a condition with that type, add one more condition.
+// if the gatewayclass status contains condition(s) with the type, then replace with the new condition.
+func setGatewayClassCondition(gwc *gatewayv1alpha2.GatewayClass, newCondition metav1.Condition) {
+ newConditions := []metav1.Condition{}
+ for _, condition := range gwc.Status.Conditions {
+ if condition.Type != newCondition.Type {
+ newConditions = append(newConditions, condition)
+ }
+ }
+ newConditions = append(newConditions, newCondition)
+ gwc.Status.Conditions = newConditions
+}

internal/controllers/gateway/gatewayclass_controller_test.go

@@ -0,0 +1,113 @@
+package gateway
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+)
+
+func Test_setGatewayClassCondtion(t *testing.T) {
+ testCases := []struct {
+ name string
+ gwc *gatewayv1alpha2.GatewayClass
+ condition metav1.Condition
+ conditionLength int
+ }{
+ {
+ name: "no_such_condition_should_append_one",
+ gwc: &gatewayv1alpha2.GatewayClass{},
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 1,
+ },
+ {
+ name: "have_condition_with_type_should_replace",
+ gwc: &gatewayv1alpha2.GatewayClass{
+ Status: gatewayv1alpha2.GatewayClassStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: "fake1",
+ Status: metav1.ConditionFalse,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ },
+ },
+ },
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 1,
+ },
+ {
+ name: "multiple_conditions_with_type_should_preserve_one",
+ gwc: &gatewayv1alpha2.GatewayClass{
+ Status: gatewayv1alpha2.GatewayClassStatus{
+ Conditions: []metav1.Condition{
+ {
+ Type: "fake1",
+ Status: metav1.ConditionFalse,
+ Reason: "fake1",
+ ObservedGeneration: 1,
+ LastTransitionTime: metav1.Now(),
+ },
+ {
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake2",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ {
+ Type: "fake2",
+ Status: metav1.ConditionTrue,
+ Reason: "fake2",
+ ObservedGeneration: 2,
+ LastTransitionTime: metav1.Now(),
+ },
+ },
+ },
+ },
+ condition: metav1.Condition{
+ Type: "fake1",
+ Status: metav1.ConditionTrue,
+ Reason: "fake1",
+ ObservedGeneration: 3,
+ LastTransitionTime: metav1.Now(),
+ },
+ conditionLength: 2,
+ },
+ }
+
+ for _, tc := range testCases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ setGatewayClassCondition(tc.gwc, tc.condition)
+ t.Logf("checking conditions of gateway after setting")
+ assert.Len(t, tc.gwc.Status.Conditions, tc.conditionLength)
+
+ conditionNum := 0
+ var observedCondition metav1.Condition
+ for _, condition := range tc.gwc.Status.Conditions {
+ if condition.Type == tc.condition.Type {
+ conditionNum++
+ observedCondition = condition
+ }
+ }
+ assert.Equal(t, 1, conditionNum)
+ assert.EqualValues(t, tc.condition, observedCondition)
+ })
+ }
+}

test/integration/gateway_test.go

@@ -96,9 +96,10 @@ func TestUnmanagedGatewayBasics(t *testing.T) {
  require.Eventually(t, func() bool {
  gw, err = gatewayClient.GatewayV1alpha2().Gateways(ns.Name).Get(ctx, gw.Name, metav1.GetOptions{})
  require.NoError(t, err)
+ // The conditions should be snapshots, so we judge by the observed status of condition with type Ready.
  for _, cond := range gw.Status.Conditions {
- if cond.Reason == string(gatewayv1alpha2.GatewayReasonReady) {
- return true
+ if cond.Type == string(gatewayv1alpha2.GatewayConditionReady) {
+ return cond.Reason == string(gatewayv1alpha2.GatewayReasonReady)
  }
  }
  return false