Skip to content

Commit 99f83e1

Browse files
committed
fix: buggy nextY logic for maxOCPVersion
The handling logic of versions that are pre-releases by the `nextY()` func (that determines the next Y release) was erroneous. Eg: `nextY("4.16.0")` returns "4.17" correctly, but `nextY("4.16.0-rc1")` returns "4.16" (the correct value is still "4.17"). This PR fixes the `nextY` function. Also has improvement for the "not-upgradeable to next OCP" version message.
1 parent efe9779 commit 99f83e1

File tree

2 files changed

+29
-20
lines changed

2 files changed

+29
-20
lines changed

pkg/controller/operators/openshift/helpers.go

+5-20
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,7 @@ func (s skew) String() string {
105105
if s.err != nil {
106106
return fmt.Sprintf("%s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
107107
}
108-
109-
return fmt.Sprintf("%s/%s is incompatible with OpenShift minor versions greater than %s", s.namespace, s.name, s.maxOpenShiftVersion)
108+
return fmt.Sprintf("ClusterServiceVersions blocking upgrade to %s or higher. The maximum supported OCP version for %s/%s is %s", nextY(s.maxOpenShiftVersion).String(), s.namespace, s.name, s.maxOpenShiftVersion)
110109
}
111110

112111
type transientError struct {
@@ -131,11 +130,6 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
131130
return nil, fmt.Errorf("failed to determine current OpenShift Y-stream release")
132131
}
133132

134-
next, err := nextY(*current)
135-
if err != nil {
136-
return nil, err
137-
}
138-
139133
csvList := &operatorsv1alpha1.ClusterServiceVersionList{}
140134
if err := cli.List(ctx, csvList); err != nil {
141135
return nil, &transientError{fmt.Errorf("failed to list ClusterServiceVersions: %w", err)}
@@ -158,7 +152,7 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
158152
continue
159153
}
160154

161-
if max == nil || max.GTE(next) {
155+
if max == nil || max.GTE(nextY(current.String())) {
162156
continue
163157
}
164158

@@ -224,18 +218,9 @@ func getCurrentRelease() (*semver.Version, error) {
224218
return currentRelease.version, nil
225219
}
226220

227-
func nextY(v semver.Version) (semver.Version, error) {
228-
v.Build = nil // Builds are irrelevant
229-
230-
if len(v.Pre) > 0 {
231-
// Dropping pre-releases is equivalent to incrementing Y
232-
v.Pre = nil
233-
v.Patch = 0
234-
235-
return v, nil
236-
}
237-
238-
return v, v.IncrementMinor() // Sets Y=Y+1 and Z=0
221+
func nextY(version string) semver.Version {
222+
v := semver.MustParse(version)
223+
return semver.Version{Major: v.Major, Minor: v.Minor + 1} // Sets Y=Y+1
239224
}
240225

241226
const (

pkg/controller/operators/openshift/helpers_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -597,3 +597,27 @@ func TestNotCopiedSelector(t *testing.T) {
597597
})
598598
}
599599
}
600+
601+
func TestOCPVersionNextY(t *testing.T) {
602+
for _, tc := range []struct {
603+
description string
604+
inVersion string
605+
expectedVersion semver.Version
606+
}{
607+
{
608+
description: "Version: 4.16.0. Expected output: 4.17",
609+
inVersion: "4.16.0",
610+
expectedVersion: semver.MustParse("4.17.0"),
611+
},
612+
{
613+
description: "Version: 4.16.0-rc1. Expected output: 4.17",
614+
inVersion: "4.16.0-rc1",
615+
expectedVersion: semver.MustParse("4.17.0"),
616+
},
617+
} {
618+
t.Run(tc.description, func(t *testing.T) {
619+
outVersion := nextY(tc.inVersion)
620+
require.Equal(t, outVersion, tc.expectedVersion)
621+
})
622+
}
623+
}

0 commit comments

Comments
 (0)