Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce a strict upgrade order between stack components #3537

Merged
merged 21 commits into from
Jul 31, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions pkg/controller/association/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package association
import (
"context"
"encoding/json"
"fmt"
"reflect"
"unsafe"

Expand Down Expand Up @@ -109,6 +110,7 @@ func ElasticsearchAuthSettings(c k8s.Client, association commonv1.Association) (

// AllowVersion returns true if the given resourceVersion is lower or equal to the associations' versions.
// For example: Kibana in version 7.8.0 cannot be deployed if its Elasticsearch association reports version 7.7.0.
// A difference in the patch version is ignored: Kibana 7.8.1 can be deployed alongside Elasticsearch 7.8.0.
sebgl marked this conversation as resolved.
Show resolved Hide resolved
// Referenced resources version is parsed from the association conf annotation.
func AllowVersion(resourceVersion version.Version, associated commonv1.Associated, logger logr.Logger, recorder record.EventRecorder) bool {
for _, assoc := range associated.GetAssociations() {
Expand All @@ -125,15 +127,17 @@ func AllowVersion(resourceVersion version.Version, associated commonv1.Associate
}
refVer, err := version.Parse(assoc.AssociationConf().Version)
if err != nil {
logger.Error(err, "Invalid version found in association conf", "association_version", assoc.AssociationConf().Version)
logger.Error(err, "Invalid version found in association configuration", "association_version", assoc.AssociationConf().Version)
return false
}
if !refVer.IsSameOrAfter(resourceVersion) {
if !refVer.IsSameOrAfterIgnoringPatch(resourceVersion) {
// the version of the referenced resource (example: Elasticsearch) is lower than
// the desired version of the reconciled resource (example: Kibana)
msg := "Delaying version deployment since an associated resource is not upgraded yet"
logger.Info(msg, "version", resourceVersion, "ref_namespace", assocRef.Namespace, "ref_name", assocRef.Name, "ref_version", refVer)
recorder.Event(associated, corev1.EventTypeWarning, events.EventReasonDelayed, msg)
logger.Info("Delaying version deployment since a referenced resource is not upgraded yet",
"version", resourceVersion, "ref_version", refVer,
"ref_type", assoc.AssociatedType(), "ref_namespace", assocRef.Namespace, "ref_name", assocRef.Name)
recorder.Event(associated, corev1.EventTypeWarning, events.EventReasonDelayed,
fmt.Sprintf("Delaying deployment of version %s since the referenced %s is not upgraded yet", resourceVersion, assoc.AssociatedType()))
return false
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/association/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func (r *Reconciler) getElasticsearch(
if apierrors.IsNotFound(err) {
// ES is not found, remove any existing backend configuration and retry in a bit.
if err := RemoveAssociationConf(r.Client, association.Associated(), association.AssociationConfAnnotationName()); err != nil && !apierrors.IsConflict(err) {
r.log(association).Error(err, "Failed to remove Elasticsearch association conf")
r.log(association).Error(err, "Failed to remove Elasticsearch association configuration")
return esv1.Elasticsearch{}, commonv1.AssociationPending, err
}
return esv1.Elasticsearch{}, commonv1.AssociationPending, nil
Expand Down
18 changes: 18 additions & 0 deletions pkg/controller/common/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,29 @@ func MustParse(version string) Version {
return *v
}

func (v *Version) Copy() *Version {
return &Version{
Major: v.Major,
Minor: v.Minor,
Patch: v.Patch,
Label: v.Label,
}
}

// IsSameOrAfter returns true if the receiver is the same version or newer than the argument. Labels are ignored.
func (v *Version) IsSameOrAfter(other Version) bool {
return v.IsSame(other) || v.IsAfter(other)
}

// IsSameOrAfterIgnoringPatch returns true if the receiver is the same version or newer than the argument,
// considering major and minor versions only (patch is ignored).
func (v *Version) IsSameOrAfterIgnoringPatch(other Version) bool {
other.Patch = 0
vCopy := v.Copy()
vCopy.Patch = 0
return vCopy.IsSameOrAfter(other)
}

// IsSameOrAfter returns true if the receiver is the same version as the argument. Labels are ignored.
func (v *Version) IsSame(other Version) bool {
return v.Major == other.Major && v.Minor == other.Minor && v.Patch == other.Patch
Expand Down
64 changes: 64 additions & 0 deletions pkg/controller/common/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,67 @@ func TestMinInStatefulSets(t *testing.T) {
})
}
}

func TestVersion_IsSameOrAfterIgnoringPatch(t *testing.T) {
tests := []struct {
name string
v Version
other Version
want bool
}{
{
name: "same version",
v: MustParse("7.8.1"),
other: MustParse("7.8.1"),
want: true,
},
{
name: "lower patch version",
v: MustParse("7.8.1"),
other: MustParse("7.8.2"),
want: true,
},
{
name: "higher patch version",
v: MustParse("7.8.2"),
other: MustParse("7.8.1"),
want: true,
},
{
name: "lower minor",
v: MustParse("7.7.1"),
other: MustParse("7.8.1"),
want: false,
},
{
name: "higher minor",
v: MustParse("7.8.1"),
other: MustParse("7.7.1"),
want: true,
},
{
name: "lower major",
v: MustParse("6.7.1"),
other: MustParse("7.7.1"),
want: false,
},
{
name: "higher major",
v: MustParse("7.7.1"),
other: MustParse("6.7.1"),
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
vCopy := tt.v.Copy()
otherCopy := tt.other.Copy()
if got := tt.v.IsSameOrAfterIgnoringPatch(tt.other); got != tt.want {
t.Errorf("IsSameOrAfterIgnoringPatch() = %v, want %v", got, tt.want)
}
// ensure v and other haven't been modified
require.Equal(t, *vCopy, tt.v)
require.Equal(t, *otherCopy, tt.other)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,19 @@ func TestReconcileEnterpriseSearch_doReconcile_AssociationDelaysVersionUpgrade(t
err = r.Client.Get(types.NamespacedName{Namespace: "ns", Name: entName.Deployment(ent.Name)}, &dep)
require.NoError(t, err)
require.Equal(t, "7.8.0", dep.Spec.Template.Labels[VersionLabelName])
// retrieve the updated ent resource
require.NoError(t, r.Client.Get(k8s.ExtractNamespacedName(&ent), &ent))

// update EnterpriseSearch to 7.8.1: this should be allowed even though Elasticsearch
// is running 7.8.0 (different patch versions)
ent.Spec.Version = "7.8.1"
err = r.Client.Update(&ent)
require.NoError(t, err)
_, err = r.doReconcile(context.Background(), ent)
require.NoError(t, err)
err = r.Client.Get(types.NamespacedName{Namespace: "ns", Name: entName.Deployment(ent.Name)}, &dep)
require.NoError(t, err)
require.Equal(t, "7.8.1", dep.Spec.Template.Labels[VersionLabelName])
}

type fakeClientStatusCall struct {
Expand Down