Skip to content

Commit 0bf85b7

Browse files
pablintinopablintino
committed
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 <git@amail.pablintino.eu>
1 parent 21813ff commit 0bf85b7

3 files changed

Lines changed: 130 additions & 102 deletions

File tree

‎pkg/monitortestlibrary/platformidentification/types.go‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import (
1212
exutil "github.com/openshift/origin/test/extended/util"
1313
kapierrs "k8s.io/apimachinery/pkg/api/errors"
1414
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
16+
"k8s.io/apimachinery/pkg/runtime/schema"
1517
"k8s.io/apimachinery/pkg/util/sets"
18+
"k8s.io/client-go/dynamic"
1619
"k8s.io/client-go/kubernetes"
1720
"k8s.io/client-go/rest"
1821
)
@@ -149,10 +152,13 @@ func BuildClusterData(ctx context.Context, clientConfig *rest.Config) (ClusterDa
149152
clusterData.CloudZone = kNodes.Items[0].Labels[`topology.kubernetes.io/zone`]
150153
}
151154

152-
if mcClient, err := machineconfigclient.NewForConfig(clientConfig); err != nil {
155+
dynClient, err := dynamic.NewForConfig(clientConfig)
156+
if err != nil {
157+
errors = append(errors, err)
158+
} else if mcClient, err := machineconfigclient.NewForConfig(clientConfig); err != nil {
153159
errors = append(errors, err)
154160
} else {
155-
osImageStreams, err := getOSImageStreams(mcClient)
161+
osImageStreams, err := getOSImageStreams(mcClient, dynClient)
156162
clusterData.OS = OS{
157163
OSImageStreams: osImageStreams,
158164
}
@@ -193,17 +199,50 @@ func getClusterVersions(versions *configv1.ClusterVersionList) []string {
193199
return cvs
194200
}
195201

196-
func getOSImageStreams(mc machineconfigclient.Interface) (OSImageStreams, error) {
202+
// GetOSImageStreamDefaultStream fetches the OSImageStream "cluster" singleton using the
203+
// dynamic client, trying v1 first and falling back to v1alpha1. It returns the
204+
// status.defaultStream value. Returns ("", nil) if the resource is not found.
205+
func GetOSImageStreamDefaultStream(dc dynamic.Interface) (string, error) {
206+
var obj *unstructured.Unstructured
207+
var err error
208+
for _, version := range []string{"v1", "v1alpha1"} {
209+
osisGVR := schema.GroupVersionResource{
210+
Group: "machineconfiguration.openshift.io",
211+
Version: version,
212+
Resource: "osimagestreams",
213+
}
214+
obj, err = dc.Resource(osisGVR).Get(context.TODO(), "cluster", metav1.GetOptions{})
215+
if err == nil || !kapierrs.IsNotFound(err) {
216+
break
217+
}
218+
}
219+
if err != nil {
220+
if kapierrs.IsNotFound(err) {
221+
return "", nil
222+
}
223+
return "", fmt.Errorf("error getting OSImageStream singleton: %v", err)
224+
}
225+
if obj == nil {
226+
return "", fmt.Errorf("OSImageStream singleton returned nil object")
227+
}
228+
defaultStream, found, nestedErr := unstructured.NestedString(obj.Object, "status", "defaultStream")
229+
if nestedErr != nil || !found {
230+
return "", fmt.Errorf("error reading OSImageStream status.defaultStream: %v", nestedErr)
231+
}
232+
return defaultStream, nil
233+
}
234+
235+
func getOSImageStreams(mc machineconfigclient.Interface, dc dynamic.Interface) (OSImageStreams, error) {
197236
var osImageStreams OSImageStreams
198237
var errs []error
199238

200-
osImageStream, err := mc.MachineconfigurationV1alpha1().OSImageStreams().Get(context.TODO(), "cluster", metav1.GetOptions{})
239+
// TODO @pablintino MCO-2320. Remove this call to platformidentification.GetOSImageStreamDefaultStream
240+
// and replace it with a bare machineConfigClient.MachineconfigurationV1().OSImageStreams().Get()
241+
defaultStream, err := GetOSImageStreamDefaultStream(dc)
201242
if err != nil {
202-
if !kapierrs.IsNotFound(err) {
203-
errs = append(errs, fmt.Errorf("error getting OSImageStream singleton: %v", err))
204-
}
243+
errs = append(errs, err)
205244
} else {
206-
osImageStreams.Default = osImageStream.Status.DefaultStream
245+
osImageStreams.Default = defaultStream
207246
}
208247

209248
mcpList, err := mc.MachineconfigurationV1().MachineConfigPools().List(context.TODO(), metav1.ListOptions{})

‎pkg/monitortestlibrary/platformidentification/types_test.go‎

Lines changed: 74 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import (
66
"testing"
77

88
mcapiv1 "github.com/openshift/api/machineconfiguration/v1"
9-
mcapiv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
109
machineconfigclient "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
1110
mcv1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1"
12-
mcv1alpha1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1"
1311
kapierrs "k8s.io/apimachinery/pkg/api/errors"
1412
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1514
"k8s.io/apimachinery/pkg/runtime/schema"
15+
"k8s.io/client-go/dynamic"
1616
)
1717

1818
// Minimal fakes: embed the real interfaces (nil-valued) and override only
@@ -22,16 +22,12 @@ import (
2222

2323
type fakeMCClient struct {
2424
machineconfigclient.Interface
25-
v1 *fakeMCv1
26-
v1alpha1 *fakeMCv1alpha1
25+
v1 *fakeMCv1
2726
}
2827

2928
func (f *fakeMCClient) MachineconfigurationV1() mcv1.MachineconfigurationV1Interface {
3029
return f.v1
3130
}
32-
func (f *fakeMCClient) MachineconfigurationV1alpha1() mcv1alpha1.MachineconfigurationV1alpha1Interface {
33-
return f.v1alpha1
34-
}
3531

3632
type fakeMCv1 struct {
3733
mcv1.MachineconfigurationV1Interface
@@ -42,15 +38,6 @@ func (f *fakeMCv1) MachineConfigPools() mcv1.MachineConfigPoolInterface {
4238
return f.pools
4339
}
4440

45-
type fakeMCv1alpha1 struct {
46-
mcv1alpha1.MachineconfigurationV1alpha1Interface
47-
streams *fakeOSImageStreams
48-
}
49-
50-
func (f *fakeMCv1alpha1) OSImageStreams() mcv1alpha1.OSImageStreamInterface {
51-
return f.streams
52-
}
53-
5441
type fakeMCPools struct {
5542
mcv1.MachineConfigPoolInterface
5643
list *mcapiv1.MachineConfigPoolList
@@ -61,27 +48,34 @@ func (f *fakeMCPools) List(_ context.Context, _ metav1.ListOptions) (*mcapiv1.Ma
6148
return f.list, f.err
6249
}
6350

64-
type fakeOSImageStreams struct {
65-
mcv1alpha1.OSImageStreamInterface
66-
obj *mcapiv1alpha1.OSImageStream
67-
err error
68-
}
69-
70-
func (f *fakeOSImageStreams) Get(_ context.Context, _ string, _ metav1.GetOptions) (*mcapiv1alpha1.OSImageStream, error) {
71-
return f.obj, f.err
72-
}
73-
74-
func newFakeMCClient(osImageStream *mcapiv1alpha1.OSImageStream, osImageStreamErr error, mcpList *mcapiv1.MachineConfigPoolList, mcpErr error) *fakeMCClient {
51+
func newFakeMCClient(mcpList *mcapiv1.MachineConfigPoolList, mcpErr error) *fakeMCClient {
7552
return &fakeMCClient{
7653
v1: &fakeMCv1{
7754
pools: &fakeMCPools{list: mcpList, err: mcpErr},
7855
},
79-
v1alpha1: &fakeMCv1alpha1{
80-
streams: &fakeOSImageStreams{obj: osImageStream, err: osImageStreamErr},
81-
},
8256
}
8357
}
8458

59+
type fakeDynClient struct {
60+
dynamic.Interface
61+
obj *unstructured.Unstructured
62+
err error
63+
}
64+
65+
func (f *fakeDynClient) Resource(_ schema.GroupVersionResource) dynamic.NamespaceableResourceInterface {
66+
return &fakeDynResource{obj: f.obj, err: f.err}
67+
}
68+
69+
type fakeDynResource struct {
70+
dynamic.NamespaceableResourceInterface
71+
obj *unstructured.Unstructured
72+
err error
73+
}
74+
75+
func (f *fakeDynResource) Get(_ context.Context, _ string, _ metav1.GetOptions, _ ...string) (*unstructured.Unstructured, error) {
76+
return f.obj, f.err
77+
}
78+
8579
func mcp(name, osImageStreamName string) mcapiv1.MachineConfigPool {
8680
return mcapiv1.MachineConfigPool{
8781
ObjectMeta: metav1.ObjectMeta{Name: name},
@@ -95,11 +89,17 @@ func mcpList(pools ...mcapiv1.MachineConfigPool) *mcapiv1.MachineConfigPoolList
9589
return &mcapiv1.MachineConfigPoolList{Items: pools}
9690
}
9791

98-
func osImageStreamSingleton(defaultStream string) *mcapiv1alpha1.OSImageStream {
99-
return &mcapiv1alpha1.OSImageStream{
100-
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
101-
Status: mcapiv1alpha1.OSImageStreamStatus{
102-
DefaultStream: defaultStream,
92+
func unstructuredOSImageStream(defaultStream string) *unstructured.Unstructured {
93+
return &unstructured.Unstructured{
94+
Object: map[string]interface{}{
95+
"apiVersion": "machineconfiguration.openshift.io/v1",
96+
"kind": "OSImageStream",
97+
"metadata": map[string]interface{}{
98+
"name": "cluster",
99+
},
100+
"status": map[string]interface{}{
101+
"defaultStream": defaultStream,
102+
},
103103
},
104104
}
105105
}
@@ -109,7 +109,8 @@ func TestGetOSImageStreams(t *testing.T) {
109109

110110
tests := []struct {
111111
name string
112-
client *fakeMCClient
112+
mcClient *fakeMCClient
113+
dynClient *fakeDynClient
113114
wantDefault string
114115
wantControlPlaneMachineConfigPool string
115116
wantWorkerMachineConfigPool string
@@ -118,10 +119,9 @@ func TestGetOSImageStreams(t *testing.T) {
118119
wantAdditionalNil bool
119120
}{
120121
{
121-
name: "all streams populated with master and worker MCPs",
122-
client: newFakeMCClient(
123-
osImageStreamSingleton("rhel-9.6"),
124-
nil,
122+
name: "all streams populated with master and worker MCPs",
123+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
124+
mcClient: newFakeMCClient(
125125
mcpList(
126126
mcp("master", "rhel-9.6"),
127127
mcp("worker", "rhel-9.6"),
@@ -134,10 +134,9 @@ func TestGetOSImageStreams(t *testing.T) {
134134
wantAdditionalNil: true,
135135
},
136136
{
137-
name: "master and worker have different streams",
138-
client: newFakeMCClient(
139-
osImageStreamSingleton("rhel-9.6"),
140-
nil,
137+
name: "master and worker have different streams",
138+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
139+
mcClient: newFakeMCClient(
141140
mcpList(
142141
mcp("master", "rhel-9.6"),
143142
mcp("worker", "rhel-10.0"),
@@ -150,10 +149,9 @@ func TestGetOSImageStreams(t *testing.T) {
150149
wantAdditionalNil: true,
151150
},
152151
{
153-
name: "additional MCPs with unique stream names",
154-
client: newFakeMCClient(
155-
osImageStreamSingleton("rhel-9.6"),
156-
nil,
152+
name: "additional MCPs with unique stream names",
153+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
154+
mcClient: newFakeMCClient(
157155
mcpList(
158156
mcp("master", "rhel-9.6"),
159157
mcp("worker", "rhel-9.6"),
@@ -168,10 +166,9 @@ func TestGetOSImageStreams(t *testing.T) {
168166
wantAdditional: []string{"rhel-10.0", "rhel-10.1"},
169167
},
170168
{
171-
name: "additional MCPs with stream name matching default are deduplicated",
172-
client: newFakeMCClient(
173-
osImageStreamSingleton("rhel-9.6"),
174-
nil,
169+
name: "additional MCPs with stream name matching default are deduplicated",
170+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
171+
mcClient: newFakeMCClient(
175172
mcpList(
176173
mcp("master", "rhel-9.6"),
177174
mcp("worker", "rhel-9.6"),
@@ -185,10 +182,9 @@ func TestGetOSImageStreams(t *testing.T) {
185182
wantAdditionalNil: true,
186183
},
187184
{
188-
name: "additional MCPs with stream name matching master/worker are deduplicated",
189-
client: newFakeMCClient(
190-
osImageStreamSingleton("rhel-9.6"),
191-
nil,
185+
name: "additional MCPs with stream name matching master/worker are deduplicated",
186+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
187+
mcClient: newFakeMCClient(
192188
mcpList(
193189
mcp("master", "rhel-10.0"),
194190
mcp("worker", "rhel-10.1"),
@@ -204,10 +200,9 @@ func TestGetOSImageStreams(t *testing.T) {
204200
wantAdditional: []string{"rhel-10.2"},
205201
},
206202
{
207-
name: "OSImageStream singleton not found is not an error",
208-
client: newFakeMCClient(
209-
nil,
210-
notFoundErr,
203+
name: "OSImageStream singleton not found is not an error",
204+
dynClient: &fakeDynClient{err: notFoundErr},
205+
mcClient: newFakeMCClient(
211206
mcpList(
212207
mcp("master", "rhel-9.6"),
213208
mcp("worker", "rhel-9.6"),
@@ -219,10 +214,9 @@ func TestGetOSImageStreams(t *testing.T) {
219214
wantAdditionalNil: true,
220215
},
221216
{
222-
name: "OSImageStream singleton non-404 error is returned",
223-
client: newFakeMCClient(
224-
nil,
225-
fmt.Errorf("internal server error"),
217+
name: "OSImageStream singleton non-404 error is returned",
218+
dynClient: &fakeDynClient{err: fmt.Errorf("internal server error")},
219+
mcClient: newFakeMCClient(
226220
mcpList(
227221
mcp("master", "rhel-9.6"),
228222
mcp("worker", "rhel-9.6"),
@@ -235,42 +229,38 @@ func TestGetOSImageStreams(t *testing.T) {
235229
wantAdditionalNil: true,
236230
},
237231
{
238-
name: "MCP list error is returned",
239-
client: newFakeMCClient(
240-
osImageStreamSingleton("rhel-9.6"),
241-
nil,
232+
name: "MCP list error is returned",
233+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
234+
mcClient: newFakeMCClient(
242235
nil,
243236
fmt.Errorf("failed to list MCPs"),
244237
),
245238
wantErr: true,
246239
wantDefault: "rhel-9.6",
247240
},
248241
{
249-
name: "both OSImageStream and MCP errors are joined",
250-
client: newFakeMCClient(
251-
nil,
252-
fmt.Errorf("osimagestream error"),
242+
name: "both OSImageStream and MCP errors are joined",
243+
dynClient: &fakeDynClient{err: fmt.Errorf("osimagestream error")},
244+
mcClient: newFakeMCClient(
253245
nil,
254246
fmt.Errorf("mcp error"),
255247
),
256248
wantErr: true,
257249
},
258250
{
259-
name: "no MCPs returns empty streams",
260-
client: newFakeMCClient(
261-
osImageStreamSingleton("rhel-9.6"),
262-
nil,
251+
name: "no MCPs returns empty streams",
252+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
253+
mcClient: newFakeMCClient(
263254
mcpList(),
264255
nil,
265256
),
266257
wantDefault: "rhel-9.6",
267258
wantAdditionalNil: true,
268259
},
269260
{
270-
name: "empty stream names on MCPs",
271-
client: newFakeMCClient(
272-
osImageStreamSingleton("rhel-9.6"),
273-
nil,
261+
name: "empty stream names on MCPs",
262+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
263+
mcClient: newFakeMCClient(
274264
mcpList(
275265
mcp("master", ""),
276266
mcp("worker", ""),
@@ -281,10 +271,9 @@ func TestGetOSImageStreams(t *testing.T) {
281271
wantAdditionalNil: true,
282272
},
283273
{
284-
name: "additional MCPs with empty stream names are excluded",
285-
client: newFakeMCClient(
286-
osImageStreamSingleton("rhel-9.6"),
287-
nil,
274+
name: "additional MCPs with empty stream names are excluded",
275+
dynClient: &fakeDynClient{obj: unstructuredOSImageStream("rhel-9.6")},
276+
mcClient: newFakeMCClient(
288277
mcpList(
289278
mcp("master", "rhel-9.6"),
290279
mcp("worker", "rhel-9.6"),
@@ -302,7 +291,7 @@ func TestGetOSImageStreams(t *testing.T) {
302291

303292
for _, tt := range tests {
304293
t.Run(tt.name, func(t *testing.T) {
305-
got, err := getOSImageStreams(tt.client)
294+
got, err := getOSImageStreams(tt.mcClient, tt.dynClient)
306295

307296
if tt.wantErr && err == nil {
308297
t.Fatal("expected error, got nil")

0 commit comments

Comments
 (0)