Assert observedGeneration is incremented in Status.Conditions. (#1586)

* Update helpers so they check conditions when they are up to date

Prior when asserting against conditions the helpers didn't check their
observedGeneration. This means the assertions could occur with a stale
status.

* When updating Gateway status conditions should increment their observedGeneration

* allow injection of controllerNames

* When updating GatewayClass the status conditions should increment their observedGeneration

* fix casing of t.Error/Fatal messages

* removed pasted link

* refactor some helpers to assert observedGeneration is bumped

* add HTTPRoute observed generation tests

* use a unique port when adding a new listener

* increase test timeout to a minute

* use real backends

* fail test is an unexpected parent ref appears

* Provide better error messages for failed assertions

* fix fixture name

* fix stale count logging

* create a deep copy prior to mutation and updating

* drop redundant assertions
*MustHaveLatestConditions will ensure the observedGeneration has changed

* address linting

* use assertion helper

* log only the parent ref name

* update godoc

* put the gateway class observed gen bump behind a support flag

conformance/tests/gateway-observed-generation-bump.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tests
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/types"
+
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, GatewayObservedGenerationBump)
+}
+
+var GatewayObservedGenerationBump = suite.ConformanceTest{
+	ShortName:   "GatewayObservedGenerationBump",
+	Description: "A Gateway in the gateway-conformance-infra namespace should update the observedGeneration in all of it's Status.Conditions after an update to the spec",
+	Manifests:   []string{"tests/gateway-observed-generation-bump.yaml"},
+	Test: func(t *testing.T, s *suite.ConformanceTestSuite) {
+
+		gwNN := types.NamespacedName{Name: "gateway-observed-generation-bump", Namespace: "gateway-conformance-infra"}
+
+		t.Run("observedGeneration should increment", func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+
+			namespaces := []string{"gateway-conformance-infra"}
+			kubernetes.NamespacesMustBeAccepted(t, s.Client, s.TimeoutConfig, namespaces)
+
+			original := &v1beta1.Gateway{}
+			err := s.Client.Get(ctx, gwNN, original)
+			require.NoErrorf(t, err, "error getting Gateway: %v", err)
+
+			// Sanity check
+			kubernetes.GatewayMustHaveLatestConditions(t, original)
+
+			all := v1beta1.NamespacesFromAll
+
+			mutate := original.DeepCopy()
+
+			// mutate the Gateway Spec
+			mutate.Spec.Listeners = append(mutate.Spec.Listeners, v1beta1.Listener{
+				Name:     "alternate",
+				Port:     8080,
+				Protocol: v1beta1.HTTPProtocolType,
+				AllowedRoutes: &v1beta1.AllowedRoutes{
+					Namespaces: &v1beta1.RouteNamespaces{From: &all},
+				},
+			})
+
+			err = s.Client.Update(ctx, mutate)
+			require.NoErrorf(t, err, "error updating the Gateway: %v", err)
+
+			// Ensure the generation and observedGeneration sync up
+			kubernetes.NamespacesMustBeAccepted(t, s.Client, s.TimeoutConfig, namespaces)
+
+			updated := &v1beta1.Gateway{}
+			err = s.Client.Get(ctx, gwNN, updated)
+			require.NoErrorf(t, err, "error getting Gateway: %v", err)
+
+			// Sanity check
+			kubernetes.GatewayMustHaveLatestConditions(t, updated)
+
+			require.NotEqual(t, original.Generation, updated.Generation, "generation should change after an update")
+		})
+	},
+}

conformance/tests/gateway-observed-generation-bump.yaml

@@ -0,0 +1,14 @@
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: Gateway
+metadata:
+  name: gateway-observed-generation-bump
+  namespace: gateway-conformance-infra
+spec:
+  gatewayClassName: "{GATEWAY_CLASS_NAME}"
+  listeners:
+    - name: http
+      port: 80
+      protocol: HTTP
+      allowedRoutes:
+        namespaces:
+          from: All

conformance/tests/gatewayclass-observed-generation-bump.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tests
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/types"
+
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, GatewayClassObservedGenerationBump)
+}
+
+var GatewayClassObservedGenerationBump = suite.ConformanceTest{
+	ShortName:   "GatewayClassObservedGenerationBump",
+	Features:    []suite.SupportedFeature{suite.SupportGatewayClassObservedGenerationBump},
+	Description: "A GatewayClass should update the observedGeneration in all of it's Status.Conditions after an update to the spec",
+	Manifests:   []string{"tests/gatewayclass-observed-generation-bump.yaml"},
+	Test: func(t *testing.T, s *suite.ConformanceTestSuite) {
+		gwc := types.NamespacedName{Name: "gatewayclass-observed-generation-bump"}
+
+		t.Run("observedGeneration should increment", func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+
+			kubernetes.GWCMustBeAccepted(t, s.Client, s.TimeoutConfig, gwc.Name)
+
+			original := &v1beta1.GatewayClass{}
+			err := s.Client.Get(ctx, gwc, original)
+			require.NoErrorf(t, err, "error getting GatewayClass: %v", err)
+
+			// Sanity check
+			kubernetes.GatewayClassMustHaveLatestConditions(t, original)
+
+			mutate := original.DeepCopy()
+			desc := "new"
+			mutate.Spec.Description = &desc
+
+			err = s.Client.Update(ctx, mutate)
+			require.NoErrorf(t, err, "error updating the GatewayClass: %v", err)
+
+			// Ensure the generation and observedGeneration sync up
+			kubernetes.GWCMustBeAccepted(t, s.Client, s.TimeoutConfig, gwc.Name)
+
+			updated := &v1beta1.GatewayClass{}
+			err = s.Client.Get(ctx, gwc, updated)
+			require.NoErrorf(t, err, "error getting GatewayClass: %v", err)
+
+			// Sanity check
+			kubernetes.GatewayClassMustHaveLatestConditions(t, updated)
+
+			require.NotEqual(t, original.Generation, updated.Generation, "generation should change after an update")
+		})
+	},
+}

conformance/tests/gatewayclass-observed-generation-bump.yaml

@@ -0,0 +1,7 @@
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: GatewayClass
+metadata:
+  name: gatewayclass-observed-generation-bump
+spec:
+  controllerName: "{GATEWAY_CONTROLLER_NAME}"
+  description: "old"

conformance/tests/httproute-observed-generation-bump.go

@@ -0,0 +1,83 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tests
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, HTTPRouteObservedGenerationBump)
+}
+
+var HTTPRouteObservedGenerationBump = suite.ConformanceTest{
+	ShortName:   "HTTPRouteObservedGenerationBump",
+	Description: "A HTTPRoute in the gateway-conformance-infra namespace should update the observedGeneration in all of it's Status.Conditions after an update to the spec",
+	Manifests:   []string{"tests/httproute-observed-generation-bump.yaml"},
+	Test: func(t *testing.T, s *suite.ConformanceTestSuite) {
+
+		routeNN := types.NamespacedName{Name: "observed-generation-bump", Namespace: "gateway-conformance-infra"}
+		gwNN := types.NamespacedName{Name: "same-namespace", Namespace: "gateway-conformance-infra"}
+
+		acceptedCondition := metav1.Condition{
+			Type:   string(v1beta1.RouteConditionAccepted),
+			Status: metav1.ConditionTrue,
+			Reason: "", // any reason
+		}
+
+		t.Run("observedGeneration should increment", func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+
+			namespaces := []string{"gateway-conformance-infra"}
+			kubernetes.NamespacesMustBeAccepted(t, s.Client, s.TimeoutConfig, namespaces)
+
+			original := &v1beta1.HTTPRoute{}
+			err := s.Client.Get(ctx, routeNN, original)
+			require.NoErrorf(t, err, "error getting HTTPRoute: %v", err)
+
+			// Sanity check
+			kubernetes.HTTPRouteMustHaveLatestConditions(t, original)
+
+			mutate := original.DeepCopy()
+			mutate.Spec.Rules[0].BackendRefs[0].Name = "infra-backend-v2"
+			err = s.Client.Update(ctx, mutate)
+			require.NoErrorf(t, err, "error updating the HTTPRoute: %v", err)
+
+			kubernetes.HTTPRouteMustHaveCondition(t, s.Client, s.TimeoutConfig, routeNN, gwNN, acceptedCondition)
+
+			updated := &v1beta1.HTTPRoute{}
+			err = s.Client.Get(ctx, routeNN, updated)
+			require.NoErrorf(t, err, "error getting Gateway: %v", err)
+
+			// Sanity check
+			kubernetes.HTTPRouteMustHaveLatestConditions(t, updated)
+
+			require.NotEqual(t, original.Generation, updated.Generation, "generation should change after an update")
+		})
+	},
+}

conformance/tests/httproute-observed-generation-bump.yaml

@@ -0,0 +1,12 @@
+apiVersion: gateway.networking.k8s.io/v1beta1
+kind: HTTPRoute
+metadata:
+  name: observed-generation-bump
+  namespace: gateway-conformance-infra
+spec:
+  parentRefs:
+  - name: same-namespace
+  rules:
+  - backendRefs:
+    - name: infra-backend-v1
+      port: 8080

conformance/utils/kubernetes/apply.go

@@ -49,25 +49,31 @@ type Applier struct {
 	// four ValidUniqueListenerPorts.
 	// If empty or nil, ports are not modified.
 	ValidUniqueListenerPorts []v1beta1.PortNumber
+
+	// GatewayClass will be used as the spec.gatewayClassName when applying Gateway resources
+	GatewayClass string
+
+	// ControllerName will be used as the spec.controllerName when applying GatewayClass resources
+	ControllerName string
 }
 
 // prepareGateway adjusts both listener ports and the gatewayClassName. It
 // returns an index pointing to the next valid listener port.
-func prepareGateway(t *testing.T, uObj *unstructured.Unstructured, gatewayClassName string, validListenerPorts []v1beta1.PortNumber, portIndex int) int {
-	err := unstructured.SetNestedField(uObj.Object, gatewayClassName, "spec", "gatewayClassName")
+func (a Applier) prepareGateway(t *testing.T, uObj *unstructured.Unstructured, portIndex int) int {
+	err := unstructured.SetNestedField(uObj.Object, a.GatewayClass, "spec", "gatewayClassName")
 	require.NoErrorf(t, err, "error setting `spec.gatewayClassName` on %s Gateway resource", uObj.GetName())
 
-	if len(validListenerPorts) > 0 {
+	if len(a.ValidUniqueListenerPorts) > 0 {
 		listeners, _, err := unstructured.NestedSlice(uObj.Object, "spec", "listeners")
 		require.NoErrorf(t, err, "error getting `spec.listeners` on %s Gateway resource", uObj.GetName())
 
 		for i, uListener := range listeners {
-			require.Less(t, portIndex, len(validListenerPorts), "not enough unassigned valid ports for `spec.listeners[%d]` on %s Gateway resource", i, uObj.GetName())
+			require.Less(t, portIndex, len(a.ValidUniqueListenerPorts), "not enough unassigned valid ports for `spec.listeners[%d]` on %s Gateway resource", i, uObj.GetName())
 
 			listener, ok := uListener.(map[string]interface{})
 			require.Truef(t, ok, "unexpected type at `spec.listeners[%d]` on %s Gateway resource", i, uObj.GetName())
 
-			nextPort := validListenerPorts[portIndex]
+			nextPort := a.ValidUniqueListenerPorts[portIndex]
 			err = unstructured.SetNestedField(listener, int64(nextPort), "port")
 			require.NoErrorf(t, err, "error setting `spec.listeners[%d].port` on %s Gateway resource", i, uObj.GetName())
 
@@ -82,6 +88,12 @@ func prepareGateway(t *testing.T, uObj *unstructured.Unstructured, gatewayClassN
 	return portIndex
 }
 
+// prepareGatewayClass adjust the spec.controllerName on the resource
+func (a Applier) prepareGatewayClass(t *testing.T, uObj *unstructured.Unstructured) {
+	err := unstructured.SetNestedField(uObj.Object, a.ControllerName, "spec", "controllerName")
+	require.NoErrorf(t, err, "error setting `spec.controllerName` on %s GatewayClass resource", uObj.GetName())
+}
+
 // prepareNamespace adjusts the Namespace labels.
 func prepareNamespace(t *testing.T, uObj *unstructured.Unstructured, namespaceLabels map[string]string) {
 	labels, _, err := unstructured.NestedStringMap(uObj.Object, "metadata", "labels")
@@ -104,7 +116,7 @@ func prepareNamespace(t *testing.T, uObj *unstructured.Unstructured, namespaceLa
 
 // prepareResources uses the options from an Applier to tweak resources given by
 // a set of manifests.
-func (a Applier) prepareResources(t *testing.T, decoder *yaml.YAMLOrJSONDecoder, gcName string) ([]unstructured.Unstructured, error) {
+func (a Applier) prepareResources(t *testing.T, decoder *yaml.YAMLOrJSONDecoder) ([]unstructured.Unstructured, error) {
 	var resources []unstructured.Unstructured
 
 	// portIndex is incremented for each listener we see. For a manifest file
@@ -123,8 +135,11 @@ func (a Applier) prepareResources(t *testing.T, decoder *yaml.YAMLOrJSONDecoder,
 			continue
 		}
 
+		if uObj.GetKind() == "GatewayClass" {
+			a.prepareGatewayClass(t, &uObj)
+		}
 		if uObj.GetKind() == "Gateway" {
-			portIndex = prepareGateway(t, &uObj, gcName, a.ValidUniqueListenerPorts, portIndex)
+			portIndex = a.prepareGateway(t, &uObj, portIndex)
 		}
 
 		if uObj.GetKind() == "Namespace" && uObj.GetObjectKind().GroupVersionKind().Group == "" {
@@ -168,13 +183,13 @@ func (a Applier) MustApplyObjectsWithCleanup(t *testing.T, c client.Client, time
 // MustApplyWithCleanup creates or updates Kubernetes resources defined with the
 // provided YAML file and registers a cleanup function for resources it created.
 // Note that this does not remove resources that already existed in the cluster.
-func (a Applier) MustApplyWithCleanup(t *testing.T, c client.Client, timeoutConfig config.TimeoutConfig, location string, gcName string, cleanup bool) {
+func (a Applier) MustApplyWithCleanup(t *testing.T, c client.Client, timeoutConfig config.TimeoutConfig, location string, cleanup bool) {
 	data, err := getContentsFromPathOrURL(location, timeoutConfig)
 	require.NoError(t, err)
 
 	decoder := yaml.NewYAMLOrJSONDecoder(data, 4096)
 
-	resources, err := a.prepareResources(t, decoder, gcName)
+	resources, err := a.prepareResources(t, decoder)
 	if err != nil {
 		t.Logf("manifest: %s", data.String())
 		require.NoErrorf(t, err, "error parsing manifest")