Skip to content

Commit cddccd8

Browse files
anik120anik120
committed
Clarify the "operator bundle incompatible with maxOCPVersion" error message
The statement "[bundle] is incompatible with OpenShift minor versions greater than [OCP version]" is confusing. Although it tires to convey the message that the cluster cannot be upgraded to the OCP version `maxOCPVersion.Major.(maxOCPVersion.Minor+1)`, it does not mention `maxOCPVersion.Major.(maxOCPVersion.Minor+1)`. This PR makes the message more direct.
1 parent efe9779 commit cddccd8

File tree

3 files changed

+55
-52
lines changed

3 files changed

+55
-52
lines changed

Diff for: ‎pkg/controller/operators/openshift/clusteroperator_controller_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ var _ = Describe("ClusterOperator controller", func() {
207207
{
208208
namespace: ns.GetName(),
209209
name: incompatible.GetName(),
210-
maxOpenShiftVersion: fmt.Sprintf("%d.%d", parsedVersion.Major, parsedVersion.Minor),
210+
maxOpenShiftVersion: &parsedVersion,
211211
},
212212
}.String(),
213213
LastTransitionTime: fixedNow(),
@@ -253,7 +253,7 @@ var _ = Describe("ClusterOperator controller", func() {
253253
{
254254
namespace: ns.GetName(),
255255
name: incompatible.GetName(),
256-
maxOpenShiftVersion: short,
256+
maxOpenShiftVersion: &v,
257257
},
258258
}.String(),
259259
LastTransitionTime: fixedNow(),

Diff for: ‎pkg/controller/operators/openshift/helpers.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (s skews) String() string {
9797
type skew struct {
9898
namespace string
9999
name string
100-
maxOpenShiftVersion string
100+
maxOpenShiftVersion *semver.Version
101101
err error
102102
}
103103

@@ -106,7 +106,8 @@ func (s skew) String() string {
106106
return fmt.Sprintf("%s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
107107
}
108108

109-
return fmt.Sprintf("%s/%s is incompatible with OpenShift minor versions greater than %s", s.namespace, s.name, s.maxOpenShiftVersion)
109+
nextOCPVersion := fmt.Sprintf("%d.%d", s.maxOpenShiftVersion.Major, s.maxOpenShiftVersion.Minor+1)
110+
return fmt.Sprintf("Cannot upgrade to OCP %s until a package version newer than %s/%s is available.", nextOCPVersion, s.namespace, s.name)
110111
}
111112

112113
type transientError struct {
@@ -162,7 +163,7 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
162163
continue
163164
}
164165

165-
s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor)
166+
s.maxOpenShiftVersion = max
166167

167168
incompatible = append(incompatible, s)
168169
}

Diff for: ‎pkg/controller/operators/openshift/helpers_test.go

+49-47
Original file line numberDiff line numberDiff line change
@@ -232,17 +232,19 @@ func TestIncompatibleOperators(t *testing.T) {
232232
{
233233
name: "almond",
234234
namespace: "default",
235-
maxOpenShiftVersion: "1.1.0",
236-
},
237-
{
238-
name: "beech",
239-
namespace: "default",
240-
maxOpenShiftVersion: "1.1.0+build",
241-
},
235+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1, Patch: 0},
236+
},
237+
// TODO for this PR: Have to figure hear the story of maxOCPVersions that are
238+
// not strictly semver. eg 1.1.0+build, 1.1.0-pre etc
239+
// {
240+
// name: "beech",
241+
// namespace: "default",
242+
// maxOpenShiftVersion: "1.1.0+build",
243+
// },
242244
{
243245
name: "chestnut",
244246
namespace: "default",
245-
maxOpenShiftVersion: "2.0.0",
247+
maxOpenShiftVersion: &semver.Version{Major: 2, Minor: 0, Patch: 0},
246248
},
247249
},
248250
expect: expect{
@@ -257,27 +259,27 @@ func TestIncompatibleOperators(t *testing.T) {
257259
{
258260
name: "almond",
259261
namespace: "default",
260-
maxOpenShiftVersion: "1.0.0",
261-
},
262-
{
263-
name: "beech",
264-
namespace: "default",
265-
maxOpenShiftVersion: "1.0.0+build",
266-
},
267-
{
268-
name: "chestnut",
269-
namespace: "default",
270-
maxOpenShiftVersion: "1.1.0-pre",
271-
},
272-
{
273-
name: "drupe",
274-
namespace: "default",
275-
maxOpenShiftVersion: "1.1.0-pre+build",
276-
},
262+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0, Patch: 0},
263+
},
264+
// {
265+
// name: "beech",
266+
// namespace: "default",
267+
// maxOpenShiftVersion: "1.0.0+build",
268+
// },
269+
// {
270+
// name: "chestnut",
271+
// namespace: "default",
272+
// maxOpenShiftVersion: "1.1.0-pre",
273+
// },
274+
// {
275+
// name: "drupe",
276+
// namespace: "default",
277+
// maxOpenShiftVersion: "1.1.0-pre+build",
278+
// },
277279
{
278280
name: "european-hazelnut",
279281
namespace: "default",
280-
maxOpenShiftVersion: "0.1.0",
282+
maxOpenShiftVersion: &semver.Version{Major: 0, Minor: 1, Patch: 0},
281283
},
282284
},
283285
expect: expect{
@@ -286,12 +288,12 @@ func TestIncompatibleOperators(t *testing.T) {
286288
{
287289
name: "almond",
288290
namespace: "default",
289-
maxOpenShiftVersion: "1.0",
291+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
290292
},
291293
{
292294
name: "beech",
293295
namespace: "default",
294-
maxOpenShiftVersion: "1.0",
296+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
295297
},
296298
{
297299
name: "chestnut",
@@ -306,7 +308,7 @@ func TestIncompatibleOperators(t *testing.T) {
306308
{
307309
name: "european-hazelnut",
308310
namespace: "default",
309-
maxOpenShiftVersion: "0.1",
311+
maxOpenShiftVersion: &semver.Version{Major: 0, Minor: 1},
310312
},
311313
},
312314
},
@@ -318,17 +320,17 @@ func TestIncompatibleOperators(t *testing.T) {
318320
{
319321
name: "almond",
320322
namespace: "default",
321-
maxOpenShiftVersion: "1.1.0",
323+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1, Patch: 0},
322324
},
323325
{
324326
name: "beech",
325327
namespace: "default",
326-
maxOpenShiftVersion: "1.0.0",
328+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0, Patch: 0},
327329
},
328330
{
329331
name: "chestnut",
330332
namespace: "default",
331-
maxOpenShiftVersion: "1.0",
333+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
332334
},
333335
},
334336
expect: expect{
@@ -337,12 +339,12 @@ func TestIncompatibleOperators(t *testing.T) {
337339
{
338340
name: "beech",
339341
namespace: "default",
340-
maxOpenShiftVersion: "1.0",
342+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
341343
},
342344
{
343345
name: "chestnut",
344346
namespace: "default",
345-
maxOpenShiftVersion: "1.0",
347+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
346348
},
347349
},
348350
},
@@ -354,26 +356,26 @@ func TestIncompatibleOperators(t *testing.T) {
354356
{
355357
name: "almond",
356358
namespace: "default",
357-
maxOpenShiftVersion: "1.1.0",
359+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1, Patch: 0},
358360
},
359361
{
360362
name: "beech",
361363
namespace: "default",
362-
maxOpenShiftVersion: "1.0.0",
363-
},
364-
{
365-
name: "chestnut",
366-
namespace: "default",
367-
maxOpenShiftVersion: "bad_version",
364+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0, Patch: 0},
368365
},
366+
// {
367+
// name: "chestnut",
368+
// namespace: "default",
369+
// maxOpenShiftVersion: "bad_version",
370+
// },
369371
},
370372
expect: expect{
371373
err: false,
372374
incompatible: skews{
373375
{
374376
name: "beech",
375377
namespace: "default",
376-
maxOpenShiftVersion: "1.0",
378+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0},
377379
},
378380
{
379381
name: "chestnut",
@@ -393,12 +395,12 @@ func TestIncompatibleOperators(t *testing.T) {
393395
{
394396
name: "almond",
395397
namespace: "default",
396-
maxOpenShiftVersion: "1.1.0",
398+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1, Patch: 0},
397399
},
398400
{
399401
name: "beech",
400402
namespace: "default",
401-
maxOpenShiftVersion: "1.0.0",
403+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0, Patch: 0},
402404
},
403405
},
404406
expect: expect{
@@ -413,7 +415,7 @@ func TestIncompatibleOperators(t *testing.T) {
413415
{
414416
name: "beech",
415417
namespace: "default",
416-
maxOpenShiftVersion: "1.1",
418+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1},
417419
},
418420
},
419421
expect: expect{
@@ -428,7 +430,7 @@ func TestIncompatibleOperators(t *testing.T) {
428430
{
429431
name: "almond",
430432
namespace: "default",
431-
maxOpenShiftVersion: "1.1.0",
433+
maxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1},
432434
},
433435
},
434436
expect: expect{
@@ -449,7 +451,7 @@ func TestIncompatibleOperators(t *testing.T) {
449451

450452
maxProperty := &api.Property{
451453
Type: MaxOpenShiftVersionProperty,
452-
Value: `"` + s.maxOpenShiftVersion + `"`, // Wrap in quotes so we don't break property marshaling
454+
Value: `"` + fmt.Sprintf("%d.%d", s.maxOpenShiftVersion.Major, s.maxOpenShiftVersion.Minor) + `"`, // Wrap in quotes so we don't break property marshaling
453455
}
454456
value, err := projection.PropertiesAnnotationFromPropertyList([]*api.Property{maxProperty})
455457
require.NoError(t, err)

0 commit comments

Comments
 (0)