Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions test/extended/node/dra.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Copy link
Copy Markdown
Contributor

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.json at L11666.

Two housekeeping items so tide can pick this up:

  • The PR title lacks the Jira key, and jira/valid-reference is currently blocking. The companions both use OCPBUGS-112638, so OCPBUGS-112638: allow v1beta2 DRA API in discovery test would match.
  • The PR title still describes the earlier TPNU-skip approach ("fix alpha/beta DRA API test if TPNU"). The commit message is accurate; the title should match it.

g.By("discovering available API versions for resource.k8s.io group")
discoveryClient := oc.AdminKubeClient().Discovery()
apiGroup, err := discoveryClient.ServerResourcesForGroupVersion("resource.k8s.io/v1")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand All @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 DRADeviceTaintRules is enabled, per the CKASO mapping:
https://github.com/openshift/cluster-kube-apiserver-operator/blob/b0783a2f85920adde7fad1915154f28dd6fe368b/pkg/operator/configobservation/apienablement/observe_runtime_config.go#L19-L20

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
o.Expect(resourceAPIGroup.Versions).To(o.HaveEach(o.BeElementOf(v1, v1beta2)), "only v1 and v1beta2 should be available")
featureGate, err := oc.AdminConfigClient().ConfigV1().FeatureGates().Get(ctx, "cluster", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), "should be able to read the cluster FeatureGate")
allowed := []metav1.GroupVersionForDiscovery{v1}
for _, details := range featureGate.Status.FeatureGates {
for _, enabled := range details.Enabled {
if enabled.Name == "DRADeviceTaintRules" {
allowed = append(allowed, v1beta2)
}
}
}
o.Expect(resourceAPIGroup.Versions).To(o.HaveEach(o.BeElementOf(allowed)), "only v1 (and v1beta2 when DRADeviceTaintRules is enabled) should be available")

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
https://github.com/openshift/origin/blob/1437257c1a/test/extended/operators/crd_must_be_stable.go#L68

o.Expect(resourceAPIGroup.PreferredVersion.Version).To(o.Equal("v1"), "v1 should be the preferred version")
})

Expand Down