-
Notifications
You must be signed in to change notification settings - Fork 4.8k
DRA: fix alpha/beta DRA API test if TPNU #31566
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
base: main
Are you sure you want to change the base?
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 | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,7 +20,7 @@ var _ = g.Describe("[sig-node][DRA][OCPFeatureGate:DynamicResourceAllocation]", | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| g.Context("Dynamic Resource Allocation", func() { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| g.It("should verify beta and alpha DRA APIs are disabled [apigroup:resource.k8s.io]", func(ctx context.Context) { | ||||||||||||||||||||||||||
| g.It("should verify the v1 DRA API is enabled [apigroup:resource.k8s.io]", func(ctx context.Context) { | ||||||||||||||||||||||||||
| g.By("discovering available API versions for resource.k8s.io group") | ||||||||||||||||||||||||||
| discoveryClient := oc.AdminKubeClient().Discovery() | ||||||||||||||||||||||||||
| apiGroup, err := discoveryClient.ServerResourcesForGroupVersion("resource.k8s.io/v1") | ||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
|
|
@@ -42,14 +42,13 @@ var _ = g.Describe("[sig-node][DRA][OCPFeatureGate:DynamicResourceAllocation]", | |||||||||||||||||||||||||
| o.Expect(resourceAPIGroup).NotTo(o.BeNil(), "resource.k8s.io group should exist") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| framework.Logf("Available versions for resource.k8s.io: %v", resourceAPIGroup.Versions) | ||||||||||||||||||||||||||
| // Verify only v1 is in the list | ||||||||||||||||||||||||||
| expectedVersions := []metav1.GroupVersionForDiscovery{ | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| GroupVersion: "resource.k8s.io/v1", | ||||||||||||||||||||||||||
| Version: "v1", | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| v1 := metav1.GroupVersionForDiscovery{GroupVersion: "resource.k8s.io/v1", Version: "v1"} | ||||||||||||||||||||||||||
| v1beta2 := metav1.GroupVersionForDiscovery{GroupVersion: "resource.k8s.io/v1beta2", Version: "v1beta2"} | ||||||||||||||||||||||||||
| o.Expect(resourceAPIGroup.Versions).To(o.ContainElement(v1), "v1 should be available") | ||||||||||||||||||||||||||
| for _, version := range resourceAPIGroup.Versions { | ||||||||||||||||||||||||||
| o.Expect(version.Version).NotTo(o.HavePrefix("v1alpha"), "alpha resource.k8s.io APIs should not be served") | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+48
to
50
Contributor
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. nit: this loop is redundant with the allowlist on the next line, which already rejects any alpha version. Fine to keep for the clearer failure message, just noting it. |
||||||||||||||||||||||||||
| o.Expect(resourceAPIGroup.Versions).To(o.Equal(expectedVersions), "only v1 should be available") | ||||||||||||||||||||||||||
| o.Expect(resourceAPIGroup.Versions).To(o.HaveEach(o.BeElementOf(v1, v1beta2)), "only v1 and v1beta2 should be available") | ||||||||||||||||||||||||||
|
Contributor
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. This allows v1beta2 on every cluster, including Default feature set, so the regression guard this test was added for is lost (see the original intent in 76bcca7: "verify beta and alpha DRA APIs are disabled"). v1beta2 is only meant to appear when Since the gate is readable from the cluster, we can allow v1beta2 only when it is enabled. This still passes at every step of the sequencing in openshift/api#3004 (comment) (gate registered but disabled today, enabled in TPNU later):
Suggested change
I would not require v1beta2 when the gate is on, since the mapping above is scoped to kube 1.36 and hosted control planes are handled separately in openshift/hypershift#9518. Same pattern is already used in |
||||||||||||||||||||||||||
| o.Expect(resourceAPIGroup.PreferredVersion.Version).To(o.Equal("v1"), "v1 should be the preferred version") | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: the rename makes Sippy / Component Readiness treat this as a new test, so the old name's history is lost. The rename is justified since the assertion changed, just calling it out. The old name is also still present in
pkg/test/ginkgo/test_summaries.jsonat L11666.Two housekeeping items so tide can pick this up:
jira/valid-referenceis currently blocking. The companions both useOCPBUGS-112638, soOCPBUGS-112638: allow v1beta2 DRA API in discovery testwould match.