Skip to content

Commit 6c0eaed

Browse files
committed
pkg/payload/precondition/clusterversion: New GiantHop update precondition
From our docs [1]: Due to fundamental Kubernetes design, all OpenShift Container Platform updates between minor versions must be serialized. You must update from OpenShift Container Platform <4.y> to <4.y+1>, and then to <4.y+2>. You cannot update from OpenShift Container Platform <4.y> to <4.y+2> directly. However, administrators who want to update between two even-numbered minor versions can do so incurring only a single reboot of non-control plane hosts. This commit adds a new precondition that enforces that policy, so cluster admins who run '--to-image ...' don't hop straight from 4.y.z to 4.(y+2).z' or similar without realizing that they were outpacing testing and policy. [1]: https://docs.openshift.com/container-platform/4.17/updating/updating_a_cluster/control-plane-only-update.html
1 parent 75706cd commit 6c0eaed

File tree

3 files changed

+195
-0
lines changed

3 files changed

+195
-0
lines changed

pkg/cvo/cvo.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,7 @@ func hasReachedLevel(cv *configv1.ClusterVersion, desired configv1.Update) bool
981981
func (optr *Operator) defaultPreconditionChecks() precondition.List {
982982
return []precondition.Precondition{
983983
preconditioncv.NewRollback(optr.cvLister),
984+
preconditioncv.NewGiantHop(optr.cvLister),
984985
preconditioncv.NewUpgradeable(optr.cvLister),
985986
preconditioncv.NewRecommendedUpdate(optr.cvLister),
986987
}
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package clusterversion
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/blang/semver/v4"
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+
12+
"github.com/openshift/cluster-version-operator/pkg/payload/precondition"
13+
)
14+
15+
// GiantHop blocks giant hops from the version that is currently being reconciled.
16+
type GiantHop struct {
17+
key string
18+
lister configv1listers.ClusterVersionLister
19+
}
20+
21+
// NewGiantHop returns a new GiantHop precondition check.
22+
func NewGiantHop(lister configv1listers.ClusterVersionLister) *GiantHop {
23+
return &GiantHop{
24+
key: "version",
25+
lister: lister,
26+
}
27+
}
28+
29+
// Name returns Name for the precondition.
30+
func (p *GiantHop) Name() string { return "ClusterVersionGiantHop" }
31+
32+
// Run runs the GiantHop precondition, blocking giant hops from the
33+
// version that is currently being reconciled. It returns a
34+
// PreconditionError when possible.
35+
func (p *GiantHop) Run(ctx context.Context, releaseContext precondition.ReleaseContext) error {
36+
cv, err := p.lister.Get(p.key)
37+
if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) {
38+
return nil
39+
}
40+
if err != nil {
41+
return &precondition.Error{
42+
Nested: err,
43+
Reason: "UnknownError",
44+
Message: err.Error(),
45+
Name: p.Name(),
46+
}
47+
}
48+
49+
currentVersion, err := semver.Parse(cv.Status.Desired.Version)
50+
if err != nil {
51+
return &precondition.Error{
52+
Nested: err,
53+
Reason: "InvalidCurrentVersion",
54+
Message: err.Error(),
55+
Name: p.Name(),
56+
NonBlockingWarning: true, // do not block on issues that require an update to fix
57+
}
58+
}
59+
60+
targetVersion, err := semver.Parse(releaseContext.DesiredVersion)
61+
if err != nil {
62+
return &precondition.Error{
63+
Nested: err,
64+
Reason: "InvalidDesiredVersion",
65+
Message: err.Error(),
66+
Name: p.Name(),
67+
}
68+
}
69+
70+
if targetVersion.Major > currentVersion.Major {
71+
return &precondition.Error{
72+
Reason: "MajorVersionUpdate",
73+
Message: fmt.Sprintf("%s has a larger major version than the current target %s (%d > %d), and only updates within the current major version are supported.", targetVersion, currentVersion, targetVersion.Major, currentVersion.Major),
74+
Name: p.Name(),
75+
}
76+
}
77+
78+
if targetVersion.Minor > currentVersion.Minor+1 {
79+
return &precondition.Error{
80+
Reason: "MultipleMinorVersionsUpdate",
81+
Message: fmt.Sprintf("%s is more than one minor version beyond the current target %s (%d.%d > %d.(%d+1)), and only updates within the current minor version or to the next minor version are supported.", targetVersion, currentVersion, targetVersion.Major, targetVersion.Minor, currentVersion.Major, currentVersion.Minor),
82+
Name: p.Name(),
83+
}
84+
}
85+
86+
return nil
87+
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package clusterversion
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
configv1 "github.com/openshift/api/config/v1"
8+
"github.com/openshift/cluster-version-operator/pkg/payload/precondition"
9+
)
10+
11+
func TestGiantHopRun(t *testing.T) {
12+
ctx := context.Background()
13+
14+
tests := []struct {
15+
name string
16+
clusterVersion configv1.ClusterVersion
17+
expected string
18+
}{
19+
{
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: "",
34+
},
35+
{
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: "",
50+
},
51+
{
52+
name: "major version",
53+
clusterVersion: configv1.ClusterVersion{
54+
Spec: configv1.ClusterVersionSpec{
55+
DesiredUpdate: &configv1.Update{
56+
Version: "2.0.0",
57+
},
58+
},
59+
Status: configv1.ClusterVersionStatus{
60+
Desired: configv1.Release{
61+
Version: "1.0.0",
62+
},
63+
},
64+
},
65+
expected: "2.0.0 has a larger major version than the current target 1.0.0 (2 > 1), and only updates within the current major version are supported.",
66+
},
67+
{
68+
name: "two minor versions",
69+
clusterVersion: configv1.ClusterVersion{
70+
Spec: configv1.ClusterVersionSpec{
71+
DesiredUpdate: &configv1.Update{
72+
Version: "1.2.0",
73+
},
74+
},
75+
Status: configv1.ClusterVersionStatus{
76+
Desired: configv1.Release{
77+
Version: "1.0.0",
78+
},
79+
},
80+
},
81+
expected: "1.2.0 is more than one minor version beyond the current target 1.0.0 (1.2 > 1.(0+1)), and only updates within the current minor version or to the next minor version are supported.",
82+
},
83+
}
84+
85+
for _, tc := range tests {
86+
t.Run(tc.name, func(t *testing.T) {
87+
tc.clusterVersion.ObjectMeta.Name = "version"
88+
cvLister := fakeClusterVersionLister(t, &tc.clusterVersion)
89+
instance := NewGiantHop(cvLister)
90+
91+
err := instance.Run(ctx, precondition.ReleaseContext{
92+
DesiredVersion: tc.clusterVersion.Spec.DesiredUpdate.Version,
93+
})
94+
switch {
95+
case err != nil && len(tc.expected) == 0:
96+
t.Error(err)
97+
case err != nil && err.Error() == tc.expected:
98+
case err != nil && err.Error() != tc.expected:
99+
t.Error(err)
100+
case err == nil && len(tc.expected) == 0:
101+
case err == nil && len(tc.expected) != 0:
102+
t.Error(err)
103+
}
104+
105+
})
106+
}
107+
}

0 commit comments

Comments
 (0)