Skip to content

Commit

Permalink
Change upgrade path validation for 8.0 to only allow 7.17 (elastic#5261
Browse files Browse the repository at this point in the history
…) (elastic#5269)

This sets the minimal supported version for 8.0 to 7.17. It also changes the validation on version upgrade to take the .status.Version into account instead of the spec.Version (which it still uses as a fallback in case we do not have a valid .status.Version). The rationale behind that is that we want to make sure that users do update the version of the spec in quick succession without waiting for the rollout of the upgrade to complete thereby unwittingly circumventing the version restriction.
  • Loading branch information
pebrc committed Jan 20, 2022
1 parent caa3ece commit 90652d0
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
28 changes: 24 additions & 4 deletions pkg/controller/elasticsearch/validation/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,11 @@ func noDowngrades(current, proposed esv1.Elasticsearch) field.ErrorList {

func validUpgradePath(current, proposed esv1.Elasticsearch) field.ErrorList {
var errs field.ErrorList
currentVer, err := version.Parse(current.Spec.Version)
if err != nil {
// this should not happen, since this is the already persisted version
errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), current.Spec.Version, parseStoredVersionErrMsg))
currentVer, ferr := currentVersion(current)
if ferr != nil {
errs = append(errs, ferr)
}

proposedVer, err := version.Parse(proposed.Spec.Version)
if err != nil {
errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), proposed.Spec.Version, parseVersionErrMsg))
Expand All @@ -298,6 +298,26 @@ func validUpgradePath(current, proposed esv1.Elasticsearch) field.ErrorList {
return errs
}

func currentVersion(current esv1.Elasticsearch) (version.Version, *field.Error) {
// we do not have a version in the status let's use the version in the current spec instead which will not reflect
// actually running Pods but which is still better than no validation.
if current.Status.Version == "" {
currentVer, err := version.Parse(current.Spec.Version)
if err != nil {
// this should not happen, since this is the already persisted version
return version.Version{}, field.Invalid(field.NewPath("spec").Child("version"), current.Spec.Version, parseStoredVersionErrMsg)
}
return currentVer, nil
}
// if available use the status version which reflects the lowest version currently running in the cluster
currentVer, err := version.Parse(current.Status.Version)
if err != nil {
// this should not happen, since this is the version from the spec copied to the status by the operator
return version.Version{}, field.Invalid(field.NewPath("status").Child("version"), current.Status.Version, parseStoredVersionErrMsg)
}
return currentVer, nil
}

func validMonitoring(es esv1.Elasticsearch) field.ErrorList {
return stackmon.Validate(&es, es.Spec.Version)
}
16 changes: 15 additions & 1 deletion pkg/controller/elasticsearch/validation/validations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"

"github.com/stretchr/testify/assert"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand Down Expand Up @@ -382,6 +381,21 @@ func Test_validUpgradePath(t *testing.T) {
proposed: es("7.1.0"),
expectErrors: false,
},
{
name: "not yet fully upgraded rejected",
current: esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "foo",
},
Spec: esv1.ElasticsearchSpec{Version: "7.17.0"},
Status: esv1.ElasticsearchStatus{
Version: "7.16.2",
},
},
proposed: es("8.0.0"),
expectErrors: true, // still running at least one node with 7.16.2
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/elasticsearch/version/supported_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func technicallySupportedVersions(v version.Version) *version.MinMaxVersion {
}
case 8:
return &version.MinMaxVersion{
// 7.4.0 is the lowest version that offers a direct upgrade path to 8.0
Min: version.MustParse("7.4.0"),
// 7.17.0 is the lowest version that offers a direct upgrade path to 8.0
Min: version.MinFor(7, 17, 0), // allow snapshot builds here for testing
Max: version.MustParse("8.99.99"),
}
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestSupportedVersions(t *testing.T) {
v: version.MustParse("8.0.0"),
},
supported: []version.Version{
version.MustParse("7.4.0"),
version.MustParse("7.17.0"),
version.MustParse("8.9.0"),
},
unsupported: []version.Version{
Expand Down

0 comments on commit 90652d0

Please sign in to comment.