Skip to content

Commit 897618d

Browse files
wkingwking
authored
Revert "OCPBUGS-35994: Revert "OCPBUGS-24535: pkg/payload/precondition/clusterversion/rollback: Allow previous version within z-stream""
1 parent df4cb12 commit 897618d

File tree

3 files changed

+227
-33
lines changed

3 files changed

+227
-33
lines changed

‎pkg/cvo/cvo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ func hasReachedLevel(cv *configv1.ClusterVersion, desired configv1.Update) bool
972972

973973
func (optr *Operator) defaultPreconditionChecks() precondition.List {
974974
return []precondition.Precondition{
975-
preconditioncv.NewRollback(optr.currentVersion),
975+
preconditioncv.NewRollback(optr.cvLister),
976976
preconditioncv.NewUpgradeable(optr.cvLister),
977977
preconditioncv.NewRecommendedUpdate(optr.cvLister),
978978
}

‎pkg/payload/precondition/clusterversion/rollback.go

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,59 @@ import (
55
"fmt"
66

77
"github.com/blang/semver/v4"
8-
configv1 "github.com/openshift/api/config/v1"
8+
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
9+
apierrors "k8s.io/apimachinery/pkg/api/errors"
10+
"k8s.io/apimachinery/pkg/api/meta"
11+
"k8s.io/klog/v2"
912

1013
precondition "github.com/openshift/cluster-version-operator/pkg/payload/precondition"
1114
)
1215

13-
// currentRelease is a callback for returning the version that is currently being reconciled.
14-
type currentRelease func() configv1.Release
15-
1616
// Rollback blocks rollbacks from the version that is currently being reconciled.
1717
type Rollback struct {
18-
currentRelease
18+
key string
19+
lister configv1listers.ClusterVersionLister
1920
}
2021

2122
// NewRollback returns a new Rollback precondition check.
22-
func NewRollback(fn currentRelease) *Rollback {
23+
func NewRollback(lister configv1listers.ClusterVersionLister) *Rollback {
2324
return &Rollback{
24-
currentRelease: fn,
25+
key: "version",
26+
lister: lister,
2527
}
2628
}
2729

2830
// Name returns Name for the precondition.
29-
func (pf *Rollback) Name() string { return "ClusterVersionRollback" }
31+
func (p *Rollback) Name() string { return "ClusterVersionRollback" }
3032

3133
// Run runs the Rollback precondition, blocking rollbacks from the
3234
// version that is currently being reconciled. It returns a
3335
// PreconditionError when possible.
3436
func (p *Rollback) Run(ctx context.Context, releaseContext precondition.ReleaseContext) error {
35-
currentRelease := p.currentRelease()
36-
currentVersion, err := semver.Parse(currentRelease.Version)
37+
cv, err := p.lister.Get(p.key)
38+
if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) {
39+
return nil
40+
}
3741
if err != nil {
3842
return &precondition.Error{
3943
Nested: err,
40-
Reason: "InvalidCurrentVersion",
44+
Reason: "UnknownError",
4145
Message: err.Error(),
4246
Name: p.Name(),
4347
}
4448
}
4549

50+
currentVersion, err := semver.Parse(cv.Status.Desired.Version)
51+
if err != nil {
52+
return &precondition.Error{
53+
Nested: err,
54+
Reason: "InvalidCurrentVersion",
55+
Message: err.Error(),
56+
Name: p.Name(),
57+
NonBlockingWarning: true, // do not block on issues that require an update to fix
58+
}
59+
}
60+
4661
targetVersion, err := semver.Parse(releaseContext.DesiredVersion)
4762
if err != nil {
4863
return &precondition.Error{
@@ -54,9 +69,42 @@ func (p *Rollback) Run(ctx context.Context, releaseContext precondition.ReleaseC
5469
}
5570

5671
if targetVersion.LT(currentVersion) {
72+
var previousVersion *semver.Version
73+
var previousImage string
74+
for _, entry := range cv.Status.History {
75+
if entry.Version != currentVersion.String() || entry.Image != cv.Status.Desired.Image {
76+
version, err := semver.Parse(entry.Version)
77+
if err != nil {
78+
klog.Errorf("Precondition %q previous version %q invalid SemVer: %v", p.Name(), entry.Version, err)
79+
} else {
80+
previousVersion = &version
81+
previousImage = entry.Image
82+
}
83+
break
84+
}
85+
}
86+
87+
if previousVersion != nil {
88+
if !targetVersion.EQ(*previousVersion) {
89+
return &precondition.Error{
90+
Reason: "LowDesiredVersion",
91+
Message: fmt.Sprintf("%s is less than the current target %s, and the only supported rollback is to the cluster's previous version %s (%s)", targetVersion, currentVersion, *previousVersion, previousImage),
92+
Name: p.Name(),
93+
}
94+
}
95+
if previousVersion.Major == currentVersion.Major && previousVersion.Minor == currentVersion.Minor {
96+
klog.V(2).Infof("Precondition %q allows rollbacks from %s to the previous version %s within a z-stream", p.Name(), currentVersion, targetVersion)
97+
return nil
98+
}
99+
return &precondition.Error{
100+
Reason: "LowDesiredVersion",
101+
Message: fmt.Sprintf("%s is less than the current target %s and matches the cluster's previous version, but rollbacks that change major or minor versions are not recommended", targetVersion, currentVersion),
102+
Name: p.Name(),
103+
}
104+
}
57105
return &precondition.Error{
58106
Reason: "LowDesiredVersion",
59-
Message: fmt.Sprintf("%s is less than the current target %s, but rollbacks and downgrades are not recommended", targetVersion, currentVersion),
107+
Message: fmt.Sprintf("%s is less than the current target %s, and the cluster has no previous Semantic Version", targetVersion, currentVersion),
60108
Name: p.Name(),
61109
}
62110
}

‎pkg/payload/precondition/clusterversion/rollback_test.go

Lines changed: 166 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,40 +13,186 @@ func TestRollbackRun(t *testing.T) {
1313

1414
tests := []struct {
1515
name string
16-
currVersion string
17-
desiredVersion string
16+
clusterVersion configv1.ClusterVersion
1817
expected string
1918
}{
2019
{
21-
name: "update",
22-
currVersion: "1.0.0",
23-
desiredVersion: "1.0.1",
24-
expected: "",
20+
name: "update",
21+
clusterVersion: configv1.ClusterVersion{
22+
Spec: configv1.ClusterVersionSpec{
23+
DesiredUpdate: &configv1.Update{
24+
Version: "1.0.1",
25+
},
26+
},
27+
Status: configv1.ClusterVersionStatus{
28+
Desired: configv1.Release{
29+
Version: "1.0.0",
30+
},
31+
},
32+
},
33+
expected: "",
2534
},
2635
{
27-
name: "no change",
28-
currVersion: "1.0.0",
29-
desiredVersion: "1.0.0",
30-
expected: "",
36+
name: "no change",
37+
clusterVersion: configv1.ClusterVersion{
38+
Spec: configv1.ClusterVersionSpec{
39+
DesiredUpdate: &configv1.Update{
40+
Version: "1.0.0",
41+
},
42+
},
43+
Status: configv1.ClusterVersionStatus{
44+
Desired: configv1.Release{
45+
Version: "1.0.0",
46+
},
47+
},
48+
},
49+
expected: "",
3150
},
3251
{
33-
name: "rollback",
34-
currVersion: "1.0.1",
35-
desiredVersion: "1.0.0",
36-
expected: "1.0.0 is less than the current target 1.0.1, but rollbacks and downgrades are not recommended",
52+
name: "rollback no history",
53+
clusterVersion: configv1.ClusterVersion{
54+
Spec: configv1.ClusterVersionSpec{
55+
DesiredUpdate: &configv1.Update{
56+
Version: "1.0.0",
57+
},
58+
},
59+
Status: configv1.ClusterVersionStatus{
60+
Desired: configv1.Release{
61+
Version: "1.0.1",
62+
},
63+
},
64+
},
65+
expected: "1.0.0 is less than the current target 1.0.1, and the cluster has no previous Semantic Version",
66+
},
67+
{
68+
name: "rollback to previous in the same minor version",
69+
clusterVersion: configv1.ClusterVersion{
70+
Spec: configv1.ClusterVersionSpec{
71+
DesiredUpdate: &configv1.Update{
72+
Version: "1.0.0",
73+
},
74+
},
75+
Status: configv1.ClusterVersionStatus{
76+
Desired: configv1.Release{
77+
Version: "1.0.1",
78+
},
79+
History: []configv1.UpdateHistory{
80+
{
81+
State: configv1.CompletedUpdate,
82+
Version: "1.0.1",
83+
},
84+
{
85+
State: configv1.CompletedUpdate,
86+
Version: "1.0.0",
87+
},
88+
},
89+
},
90+
},
91+
expected: "",
92+
},
93+
{
94+
name: "rollback to previous completed with intermediate partial",
95+
clusterVersion: configv1.ClusterVersion{
96+
Spec: configv1.ClusterVersionSpec{
97+
DesiredUpdate: &configv1.Update{
98+
Version: "1.0.0",
99+
},
100+
},
101+
Status: configv1.ClusterVersionStatus{
102+
Desired: configv1.Release{
103+
Version: "1.0.2",
104+
Image: "example.com/image:1.0.2",
105+
},
106+
History: []configv1.UpdateHistory{
107+
{
108+
State: configv1.PartialUpdate,
109+
Version: "1.0.2",
110+
Image: "example.com/image:1.0.2",
111+
},
112+
{
113+
State: configv1.PartialUpdate,
114+
Version: "1.0.1",
115+
Image: "example.com/image:1.0.1",
116+
},
117+
{
118+
State: configv1.CompletedUpdate,
119+
Version: "1.0.0",
120+
Image: "example.com/image:1.0.0",
121+
},
122+
},
123+
},
124+
},
125+
expected: "1.0.0 is less than the current target 1.0.2, and the only supported rollback is to the cluster's previous version 1.0.1 (example.com/image:1.0.1)",
126+
},
127+
{
128+
name: "rollback to previous completed with intermediate image change",
129+
clusterVersion: configv1.ClusterVersion{
130+
Spec: configv1.ClusterVersionSpec{
131+
DesiredUpdate: &configv1.Update{
132+
Version: "1.0.0",
133+
},
134+
},
135+
Status: configv1.ClusterVersionStatus{
136+
Desired: configv1.Release{
137+
Version: "1.0.1",
138+
Image: "example.com/image:multi-arch",
139+
},
140+
History: []configv1.UpdateHistory{
141+
{
142+
State: configv1.CompletedUpdate,
143+
Version: "1.0.1",
144+
Image: "example.com/image:multi-arch",
145+
},
146+
{
147+
State: configv1.CompletedUpdate,
148+
Version: "1.0.1",
149+
Image: "example.com/image:single-arch",
150+
},
151+
{
152+
State: configv1.CompletedUpdate,
153+
Version: "1.0.0",
154+
},
155+
},
156+
},
157+
},
158+
expected: "1.0.0 is less than the current target 1.0.1, and the only supported rollback is to the cluster's previous version 1.0.1 (example.com/image:single-arch)",
159+
},
160+
{
161+
name: "rollback to previous in an earlier minor release",
162+
clusterVersion: configv1.ClusterVersion{
163+
Spec: configv1.ClusterVersionSpec{
164+
DesiredUpdate: &configv1.Update{
165+
Version: "1.0.0",
166+
},
167+
},
168+
Status: configv1.ClusterVersionStatus{
169+
Desired: configv1.Release{
170+
Version: "2.0.0",
171+
},
172+
History: []configv1.UpdateHistory{
173+
{
174+
State: configv1.CompletedUpdate,
175+
Version: "2.0.0",
176+
},
177+
{
178+
State: configv1.CompletedUpdate,
179+
Version: "1.0.0",
180+
},
181+
},
182+
},
183+
},
184+
expected: "1.0.0 is less than the current target 2.0.0 and matches the cluster's previous version, but rollbacks that change major or minor versions are not recommended",
37185
},
38186
}
39187

40188
for _, tc := range tests {
41189
t.Run(tc.name, func(t *testing.T) {
42-
instance := NewRollback(func() configv1.Release {
43-
return configv1.Release{
44-
Version: tc.currVersion,
45-
}
46-
})
190+
tc.clusterVersion.ObjectMeta.Name = "version"
191+
cvLister := fakeClusterVersionLister(t, &tc.clusterVersion)
192+
instance := NewRollback(cvLister)
47193

48194
err := instance.Run(ctx, precondition.ReleaseContext{
49-
DesiredVersion: tc.desiredVersion,
195+
DesiredVersion: tc.clusterVersion.Spec.DesiredUpdate.Version,
50196
})
51197
switch {
52198
case err != nil && len(tc.expected) == 0:

0 commit comments

Comments
 (0)