-
Notifications
You must be signed in to change notification settings - Fork 542
[Conformance]: Adds GatewayClass SupportedVersion Test #3368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/* | ||
Copyright 2024 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" | ||
|
||
"github.com/stretchr/testify/require" | ||
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" | ||
"k8s.io/apimachinery/pkg/types" | ||
|
||
"sigs.k8s.io/gateway-api/conformance/utils/kubernetes" | ||
"sigs.k8s.io/gateway-api/conformance/utils/suite" | ||
"sigs.k8s.io/gateway-api/pkg/consts" | ||
"sigs.k8s.io/gateway-api/pkg/features" | ||
) | ||
|
||
func init() { | ||
ConformanceTests = append(ConformanceTests, GatewayClassSupportedVersionCondition) | ||
} | ||
|
||
var GatewayClassSupportedVersionCondition = suite.ConformanceTest{ | ||
ShortName: "GatewayClassSupportedVersionCondition", | ||
Features: []features.FeatureName{ | ||
features.SupportGateway, | ||
}, | ||
Description: "A GatewayClass should set the SupportedVersion condition based on the presence and version of Gateway API CRDs in the cluster", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this makes sense. Consider I build an implementation that supports v1.1. v1.2 comes out. Per the spec, even if I am willing to support it, I must set the condition to "false" This means I cannot pass this conformance test for 1.2 until I upgrade to 1.2 which doesn't actually seem desired? |
||
Test: func(t *testing.T, s *suite.ConformanceTestSuite) { | ||
gwc := types.NamespacedName{Name: s.GatewayClassName} | ||
|
||
t.Run("SupportedVersion condition should be set correctly", func(t *testing.T) { | ||
ctx, cancel := context.WithTimeout(context.Background(), s.TimeoutConfig.DefaultTestTimeout) | ||
defer cancel() | ||
|
||
// Ensure GatewayClass conditions are as expected before proceeding | ||
kubernetes.GWCMustHaveAcceptedConditionTrue(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
kubernetes.GWCMustHaveSupportedVersionConditionAny(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
|
||
// Retrieve the GatewayClass CRD | ||
crd := &apiextv1.CustomResourceDefinition{} | ||
crdName := types.NamespacedName{Name: "gatewayclasses.gateway.networking.k8s.io"} | ||
err := s.Client.Get(ctx, crdName, crd) | ||
require.NoErrorf(t, err, "error getting GatewayClass CRD: %v", err) | ||
|
||
if crd.Annotations != nil { | ||
// Remove the bundle version annotation if it exists | ||
if _, exists := crd.Annotations[consts.BundleVersionAnnotation]; !exists { | ||
t.Fatalf("Annotation %q does not exist on CRD %s", consts.BundleVersionAnnotation, crdName) | ||
} | ||
delete(crd.Annotations, consts.BundleVersionAnnotation) | ||
if err := s.Client.Update(ctx, crd); err != nil { | ||
t.Fatalf("Failed to update CRD %s: %v", crdName, err) | ||
} | ||
} | ||
|
||
// Ensure the SupportedVersion status condition is false | ||
kubernetes.GWCMustHaveSupportedVersionConditionFalse(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really expect implementations to have to watch CRDs and constantly reconcile the Gatewayclass? I get how we got to this point, but it feels like we are building APIs in isolation from considerations about user needs and implementation complexity. Users are unlikely to ever have an incompatibility and somehow realize to check a random object (that they don't even create, so why would they know to look at it?). The cost to implement this, however, is extremely high... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW the api-machinery folks have, historically, advised against an implementation reading CRDs at all or even assuming an API is defined by a CRD (vs another mechanism). I realize this has not much to do with the test... but I missed the PR adding the condition |
||
|
||
// Add the bundle version annotation | ||
crd.Annotations[consts.BundleVersionAnnotation] = consts.BundleVersion | ||
if err := s.Client.Update(ctx, crd); err != nil { | ||
t.Fatalf("Failed to update CRD %s: %v", crdName, err) | ||
} | ||
|
||
// Ensure the SupportedVersion status condition is true | ||
kubernetes.GWCMustHaveSupportedVersionConditionTrue(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
|
||
// Set the bundle version annotation to an unsupported version | ||
crd.Annotations[consts.BundleVersionAnnotation] = "v0.0.0" | ||
if err := s.Client.Update(ctx, crd); err != nil { | ||
t.Fatalf("Failed to update CRD %s: %v", crdName, err) | ||
} | ||
|
||
// Ensure the SupportedVersion is false | ||
kubernetes.GWCMustHaveSupportedVersionConditionFalse(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some implementations like GKE will ~immediately overwrite this CRD with the original version, can you add that as an acceptable outcome of this test? |
||
|
||
// Add the bundle version annotation back | ||
crd.Annotations[consts.BundleVersionAnnotation] = consts.BundleVersion | ||
if err := s.Client.Update(ctx, crd); err != nil { | ||
t.Fatalf("Failed to update CRD %s: %v", crdName, err) | ||
} | ||
|
||
// Ensure the SupportedVersion is true | ||
kubernetes.GWCMustHaveSupportedVersionConditionTrue(t, s.Client, s.TimeoutConfig, gwc.Name) | ||
}) | ||
}, | ||
} |
Uh oh!
There was an error while loading. Please reload this page.