From 0bf85b7d9a5115d0823544b122a4739eefca3d68 Mon Sep 17 00:00:00 2001 From: Pablo Rodriguez Nava Date: Mon, 25 May 2026 15:44:50 +0200 Subject: [PATCH] MCO-2319: Temporal OSImageStream dynamic call As part of the OSImageStream graduation to v1 from v1alpha all usages, including origin, require a bump of the API and client-go to introduce the new types. Unfortunately, the latest API code is not currently compatible with the vendored kube-apiserver. To unlock the high priority bump of OSImageStreams this change moves the direc typed API call to a dynamic one. This change should be reverted to direct typed calls as soon as the kube-apiserver bump happens and the client-go and API can be safely bumped too. Signed-off-by: Pablo Rodriguez Nava --- .../platformidentification/types.go | 55 +++++- .../platformidentification/types_test.go | 159 ++++++++---------- test/extended/ci/job_names.go | 18 +- 3 files changed, 130 insertions(+), 102 deletions(-) diff --git a/pkg/monitortestlibrary/platformidentification/types.go b/pkg/monitortestlibrary/platformidentification/types.go index ed7233db8e93..e6a27f289d4a 100644 --- a/pkg/monitortestlibrary/platformidentification/types.go +++ b/pkg/monitortestlibrary/platformidentification/types.go @@ -12,7 +12,10 @@ import ( exutil "github.com/openshift/origin/test/extended/util" kapierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ) @@ -149,10 +152,13 @@ func BuildClusterData(ctx context.Context, clientConfig *rest.Config) (ClusterDa clusterData.CloudZone = kNodes.Items[0].Labels[`topology.kubernetes.io/zone`] } - if mcClient, err := machineconfigclient.NewForConfig(clientConfig); err != nil { + dynClient, err := dynamic.NewForConfig(clientConfig) + if err != nil { + errors = append(errors, err) + } else if mcClient, err := machineconfigclient.NewForConfig(clientConfig); err != nil { errors = append(errors, err) } else { - osImageStreams, err := getOSImageStreams(mcClient) + osImageStreams, err := getOSImageStreams(mcClient, dynClient) clusterData.OS = OS{ OSImageStreams: osImageStreams, } @@ -193,17 +199,50 @@ func getClusterVersions(versions *configv1.ClusterVersionList) []string { return cvs } -func getOSImageStreams(mc machineconfigclient.Interface) (OSImageStreams, error) { +// GetOSImageStreamDefaultStream fetches the OSImageStream "cluster" singleton using the +// dynamic client, trying v1 first and falling back to v1alpha1. It returns the +// status.defaultStream value. Returns ("", nil) if the resource is not found. +func GetOSImageStreamDefaultStream(dc dynamic.Interface) (string, error) { + var obj *unstructured.Unstructured + var err error + for _, version := range []string{"v1", "v1alpha1"} { + osisGVR := schema.GroupVersionResource{ + Group: "machineconfiguration.openshift.io", + Version: version, + Resource: "osimagestreams", + } + obj, err = dc.Resource(osisGVR).Get(context.TODO(), "cluster", metav1.GetOptions{}) + if err == nil || !kapierrs.IsNotFound(err) { + break + } + } + if err != nil { + if kapierrs.IsNotFound(err) { + return "", nil + } + return "", fmt.Errorf("error getting OSImageStream singleton: %v", err) + } + if obj == nil { + return "", fmt.Errorf("OSImageStream singleton returned nil object") + } + defaultStream, found, nestedErr := unstructured.NestedString(obj.Object, "status", "defaultStream") + if nestedErr != nil || !found { + return "", fmt.Errorf("error reading OSImageStream status.defaultStream: %v", nestedErr) + } + return defaultStream, nil +} + +func getOSImageStreams(mc machineconfigclient.Interface, dc dynamic.Interface) (OSImageStreams, error) { var osImageStreams OSImageStreams var errs []error - osImageStream, err := mc.MachineconfigurationV1alpha1().OSImageStreams().Get(context.TODO(), "cluster", metav1.GetOptions{}) + // TODO @pablintino MCO-2320. Remove this call to platformidentification.GetOSImageStreamDefaultStream + // and replace it with a bare machineConfigClient.MachineconfigurationV1().OSImageStreams().Get() + defaultStream, err := GetOSImageStreamDefaultStream(dc) if err != nil { - if !kapierrs.IsNotFound(err) { - errs = append(errs, fmt.Errorf("error getting OSImageStream singleton: %v", err)) - } + errs = append(errs, err) } else { - osImageStreams.Default = osImageStream.Status.DefaultStream + osImageStreams.Default = defaultStream } mcpList, err := mc.MachineconfigurationV1().MachineConfigPools().List(context.TODO(), metav1.ListOptions{}) diff --git a/pkg/monitortestlibrary/platformidentification/types_test.go b/pkg/monitortestlibrary/platformidentification/types_test.go index 7998dc60280e..4a33e60f4969 100644 --- a/pkg/monitortestlibrary/platformidentification/types_test.go +++ b/pkg/monitortestlibrary/platformidentification/types_test.go @@ -6,13 +6,13 @@ import ( "testing" mcapiv1 "github.com/openshift/api/machineconfiguration/v1" - mcapiv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1" machineconfigclient "github.com/openshift/client-go/machineconfiguration/clientset/versioned" mcv1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1" - mcv1alpha1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1" kapierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" ) // Minimal fakes: embed the real interfaces (nil-valued) and override only @@ -22,16 +22,12 @@ import ( type fakeMCClient struct { machineconfigclient.Interface - v1 *fakeMCv1 - v1alpha1 *fakeMCv1alpha1 + v1 *fakeMCv1 } func (f *fakeMCClient) MachineconfigurationV1() mcv1.MachineconfigurationV1Interface { return f.v1 } -func (f *fakeMCClient) MachineconfigurationV1alpha1() mcv1alpha1.MachineconfigurationV1alpha1Interface { - return f.v1alpha1 -} type fakeMCv1 struct { mcv1.MachineconfigurationV1Interface @@ -42,15 +38,6 @@ func (f *fakeMCv1) MachineConfigPools() mcv1.MachineConfigPoolInterface { return f.pools } -type fakeMCv1alpha1 struct { - mcv1alpha1.MachineconfigurationV1alpha1Interface - streams *fakeOSImageStreams -} - -func (f *fakeMCv1alpha1) OSImageStreams() mcv1alpha1.OSImageStreamInterface { - return f.streams -} - type fakeMCPools struct { mcv1.MachineConfigPoolInterface list *mcapiv1.MachineConfigPoolList @@ -61,27 +48,34 @@ func (f *fakeMCPools) List(_ context.Context, _ metav1.ListOptions) (*mcapiv1.Ma return f.list, f.err } -type fakeOSImageStreams struct { - mcv1alpha1.OSImageStreamInterface - obj *mcapiv1alpha1.OSImageStream - err error -} - -func (f *fakeOSImageStreams) Get(_ context.Context, _ string, _ metav1.GetOptions) (*mcapiv1alpha1.OSImageStream, error) { - return f.obj, f.err -} - -func newFakeMCClient(osImageStream *mcapiv1alpha1.OSImageStream, osImageStreamErr error, mcpList *mcapiv1.MachineConfigPoolList, mcpErr error) *fakeMCClient { +func newFakeMCClient(mcpList *mcapiv1.MachineConfigPoolList, mcpErr error) *fakeMCClient { return &fakeMCClient{ v1: &fakeMCv1{ pools: &fakeMCPools{list: mcpList, err: mcpErr}, }, - v1alpha1: &fakeMCv1alpha1{ - streams: &fakeOSImageStreams{obj: osImageStream, err: osImageStreamErr}, - }, } } +type fakeDynClient struct { + dynamic.Interface + obj *unstructured.Unstructured + err error +} + +func (f *fakeDynClient) Resource(_ schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + return &fakeDynResource{obj: f.obj, err: f.err} +} + +type fakeDynResource struct { + dynamic.NamespaceableResourceInterface + obj *unstructured.Unstructured + err error +} + +func (f *fakeDynResource) Get(_ context.Context, _ string, _ metav1.GetOptions, _ ...string) (*unstructured.Unstructured, error) { + return f.obj, f.err +} + func mcp(name, osImageStreamName string) mcapiv1.MachineConfigPool { return mcapiv1.MachineConfigPool{ ObjectMeta: metav1.ObjectMeta{Name: name}, @@ -95,11 +89,17 @@ func mcpList(pools ...mcapiv1.MachineConfigPool) *mcapiv1.MachineConfigPoolList return &mcapiv1.MachineConfigPoolList{Items: pools} } -func osImageStreamSingleton(defaultStream string) *mcapiv1alpha1.OSImageStream { - return &mcapiv1alpha1.OSImageStream{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, - Status: mcapiv1alpha1.OSImageStreamStatus{ - DefaultStream: defaultStream, +func unstructuredOSImageStream(defaultStream string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "machineconfiguration.openshift.io/v1", + "kind": "OSImageStream", + "metadata": map[string]interface{}{ + "name": "cluster", + }, + "status": map[string]interface{}{ + "defaultStream": defaultStream, + }, }, } } @@ -109,7 +109,8 @@ func TestGetOSImageStreams(t *testing.T) { tests := []struct { name string - client *fakeMCClient + mcClient *fakeMCClient + dynClient *fakeDynClient wantDefault string wantControlPlaneMachineConfigPool string wantWorkerMachineConfigPool string @@ -118,10 +119,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil bool }{ { - name: "all streams populated with master and worker MCPs", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "all streams populated with master and worker MCPs", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -134,10 +134,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "master and worker have different streams", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "master and worker have different streams", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-10.0"), @@ -150,10 +149,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "additional MCPs with unique stream names", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "additional MCPs with unique stream names", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -168,10 +166,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditional: []string{"rhel-10.0", "rhel-10.1"}, }, { - name: "additional MCPs with stream name matching default are deduplicated", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "additional MCPs with stream name matching default are deduplicated", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -185,10 +182,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "additional MCPs with stream name matching master/worker are deduplicated", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "additional MCPs with stream name matching master/worker are deduplicated", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-10.0"), mcp("worker", "rhel-10.1"), @@ -204,10 +200,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditional: []string{"rhel-10.2"}, }, { - name: "OSImageStream singleton not found is not an error", - client: newFakeMCClient( - nil, - notFoundErr, + name: "OSImageStream singleton not found is not an error", + dynClient: &fakeDynClient{err: notFoundErr}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -219,10 +214,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "OSImageStream singleton non-404 error is returned", - client: newFakeMCClient( - nil, - fmt.Errorf("internal server error"), + name: "OSImageStream singleton non-404 error is returned", + dynClient: &fakeDynClient{err: fmt.Errorf("internal server error")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -235,10 +229,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "MCP list error is returned", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "MCP list error is returned", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( nil, fmt.Errorf("failed to list MCPs"), ), @@ -246,20 +239,18 @@ func TestGetOSImageStreams(t *testing.T) { wantDefault: "rhel-9.6", }, { - name: "both OSImageStream and MCP errors are joined", - client: newFakeMCClient( - nil, - fmt.Errorf("osimagestream error"), + name: "both OSImageStream and MCP errors are joined", + dynClient: &fakeDynClient{err: fmt.Errorf("osimagestream error")}, + mcClient: newFakeMCClient( nil, fmt.Errorf("mcp error"), ), wantErr: true, }, { - name: "no MCPs returns empty streams", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "no MCPs returns empty streams", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList(), nil, ), @@ -267,10 +258,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "empty stream names on MCPs", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "empty stream names on MCPs", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", ""), mcp("worker", ""), @@ -281,10 +271,9 @@ func TestGetOSImageStreams(t *testing.T) { wantAdditionalNil: true, }, { - name: "additional MCPs with empty stream names are excluded", - client: newFakeMCClient( - osImageStreamSingleton("rhel-9.6"), - nil, + name: "additional MCPs with empty stream names are excluded", + dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")}, + mcClient: newFakeMCClient( mcpList( mcp("master", "rhel-9.6"), mcp("worker", "rhel-9.6"), @@ -302,7 +291,7 @@ func TestGetOSImageStreams(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := getOSImageStreams(tt.client) + got, err := getOSImageStreams(tt.mcClient, tt.dynClient) if tt.wantErr && err == nil { t.Fatal("expected error, got nil") diff --git a/test/extended/ci/job_names.go b/test/extended/ci/job_names.go index 366e1df33f8c..4cd3e0d9c39f 100644 --- a/test/extended/ci/job_names.go +++ b/test/extended/ci/job_names.go @@ -11,8 +11,9 @@ import ( o "github.com/onsi/gomega" v1 "github.com/openshift/api/config/v1" mcv1client "github.com/openshift/client-go/machineconfiguration/clientset/versioned" - kapierrs "k8s.io/apimachinery/pkg/api/errors" + "github.com/openshift/origin/pkg/monitortestlibrary/platformidentification" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" e2e "k8s.io/kubernetes/test/e2e/framework" e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" @@ -206,7 +207,7 @@ var _ = g.Describe("[sig-ci] [Early] prow job name", func() { e2eskipper.Skip("Cannot check RHCOS for HyperShift clusters") } - clusterIsRHCOS10 := isRHCOS10(oc.MachineConfigurationClient()) + clusterIsRHCOS10 := isRHCOS10(oc.MachineConfigurationClient(), oc.AdminDynamicClient()) // Mixed RHCOS version clusters (e.g. rhcos9-10) have nodes running both // RHCOS 9 and RHCOS 10. Validate that we see both versions across nodes. @@ -231,7 +232,7 @@ var _ = g.Describe("[sig-ci] [Early] prow job name", func() { // isRHCOS10 checks whether the cluster is running RHEL 10 by examining the worker // MCP's OSImageStream setting, falling back to the cluster-wide default stream // from the OSImageStream singleton if the MCP does not specify one. -func isRHCOS10(machineConfigClient mcv1client.Interface) bool { +func isRHCOS10(machineConfigClient mcv1client.Interface, dc dynamic.Interface) bool { mcp, err := machineConfigClient.MachineconfigurationV1().MachineConfigPools().Get(context.TODO(), "worker", metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred(), "Error getting worker MCP") @@ -239,13 +240,12 @@ func isRHCOS10(machineConfigClient mcv1client.Interface) bool { return mcp.Spec.OSImageStream.Name == "rhel-10" } - osImageStream, err := machineConfigClient.MachineconfigurationV1alpha1().OSImageStreams().Get(context.TODO(), "cluster", metav1.GetOptions{}) - if kapierrs.IsNotFound(err) { - return false - } - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting OSImageStream singleton") + // TODO @pablintino MCO-2320. Remove this call to platformidentification.GetOSImageStreamDefaultStream + // and replace it with a bare machineConfigClient.MachineconfigurationV1().OSImageStreams().Get() + defaultStream, err := platformidentification.GetOSImageStreamDefaultStream(dc) + o.Expect(err).NotTo(o.HaveOccurred(), "Error getting OSImageStream default stream") - return osImageStream.Status.DefaultStream == "rhel-10" + return defaultStream == "rhel-10" } // hasMixedRHCOSNodes scans all nodes and returns whether both RHCOS 9 and