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.

Sign up for GitHub

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

Set expectations when upscaling a StatefulSet #1813

Merged
merged 1 commit into from
Sep 27, 2019
Merged

Set expectations when upscaling a StatefulSet #1813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions pkg/controller/elasticsearch/driver/nodes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func (d *defaultDriver) reconcileNodeSpecs(
observedState: observedState,
esState: esState,
upscaleStateBuilder: &upscaleStateBuilder{},
expectations: d.Expectations,
}
actualStatefulSets, err = HandleUpscaleAndSpecChanges(upscaleCtx, actualStatefulSets, expectedResources)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/elasticsearch/driver/upscale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/expectations"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/nodespec"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/observer"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings"
Expand All @@ -26,6 +27,7 @@ type upscaleCtx struct {
observedState observer.State
esState ESState
upscaleStateBuilder *upscaleStateBuilder
expectations *expectations.Expectations
}

// HandleUpscaleAndSpecChanges reconciles expected NodeSpec resources.
Expand Down Expand Up @@ -55,7 +57,7 @@ func HandleUpscaleAndSpecChanges(
if _, err := common.ReconcileService(ctx.k8sClient, ctx.scheme, &res.HeadlessService, &ctx.es); err != nil {
return nil, err
}
reconciled, err := sset.ReconcileStatefulSet(ctx.k8sClient, ctx.scheme, ctx.es, res.StatefulSet)
reconciled, err := sset.ReconcileStatefulSet(ctx.k8sClient, ctx.scheme, ctx.es, res.StatefulSet, ctx.expectations)
if err != nil {
return nil, err
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/elasticsearch/driver/upscale_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/expectations"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/name"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/nodespec"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/observer"
Expand Down Expand Up @@ -44,6 +45,7 @@ func TestHandleUpscaleAndSpecChanges(t *testing.T) {
observedState: observer.State{},
esState: nil,
upscaleStateBuilder: &upscaleStateBuilder{},
expectations: expectations.NewExpectations(),
}
expectedResources := nodespec.ResourcesList{
{
Expand Down Expand Up @@ -124,6 +126,8 @@ func TestHandleUpscaleAndSpecChanges(t *testing.T) {
require.NoError(t, k8sClient.Get(types.NamespacedName{Namespace: "ns", Name: "sset2"}, &sset2))
require.Equal(t, common.Int32(10), sset2.Spec.Replicas)
require.Equal(t, updatedStatefulSets[1], sset2)
// expectations should have been set
require.NotEmpty(t, ctx.expectations.GetGenerations())

// apply a spec change
actualStatefulSets = sset.StatefulSetList{sset1, sset2}
Expand Down
10 changes: 9 additions & 1 deletion pkg/controller/elasticsearch/sset/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package sset

import (
"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/expectations"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/hash"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/reconciler"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
Expand All @@ -14,7 +15,7 @@ import (
)

// ReconcileStatefulSet creates or updates the expected StatefulSet.
func ReconcileStatefulSet(c k8s.Client, scheme *runtime.Scheme, es v1alpha1.Elasticsearch, expected appsv1.StatefulSet) (appsv1.StatefulSet, error) {
func ReconcileStatefulSet(c k8s.Client, scheme *runtime.Scheme, es v1alpha1.Elasticsearch, expected appsv1.StatefulSet, expectations *expectations.Expectations) (appsv1.StatefulSet, error) {
var reconciled appsv1.StatefulSet
err := reconciler.ReconcileResource(reconciler.Params{
Client: c,
Expand All @@ -31,6 +32,13 @@ func ReconcileStatefulSet(c k8s.Client, scheme *runtime.Scheme, es v1alpha1.Elas
UpdateReconciled: func() {
expected.DeepCopyInto(&reconciled)
},
PostUpdate: func() {
if expectations != nil {
// expect the reconciled StatefulSet to be there in the cache for next reconciliations,
// to prevent assumptions based on the wrong replica count
expectations.ExpectGeneration(reconciled.ObjectMeta)
}
},
})
return reconciled, err
}
Expand Down
35 changes: 22 additions & 13 deletions pkg/controller/elasticsearch/sset/reconcile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/expectations"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/hash"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
)
Expand Down Expand Up @@ -49,29 +50,34 @@ func TestReconcileStatefulSet(t *testing.T) {
updatedSset.Labels[hash.TemplateHashLabelName] = "updated"

tests := []struct {
name string
c k8s.Client
expected v1.StatefulSet
name string
c k8s.Client
expected v1.StatefulSet
wantExpectationsUpdated bool
}{
{
name: "create new sset",
c: k8s.WrapClient(fake.NewFakeClient()),
expected: ssetSample,
name: "create new sset",
c: k8s.WrapClient(fake.NewFakeClient()),
expected: ssetSample,
wantExpectationsUpdated: false,
},
{
name: "no update on existing sset",
c: k8s.WrapClient(fake.NewFakeClient(&ssetSample)),
expected: ssetSample,
name: "no update on existing sset",
c: k8s.WrapClient(fake.NewFakeClient(&ssetSample)),
expected: ssetSample,
wantExpectationsUpdated: false,
},
{
name: "update on sset with different template hash",
c: k8s.WrapClient(fake.NewFakeClient(&ssetSample)),
expected: updatedSset,
name: "update on sset with different template hash",
c: k8s.WrapClient(fake.NewFakeClient(&ssetSample)),
expected: updatedSset,
wantExpectationsUpdated: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
returned, err := ReconcileStatefulSet(tt.c, scheme.Scheme, es, tt.expected)
exp := expectations.NewExpectations()
returned, err := ReconcileStatefulSet(tt.c, scheme.Scheme, es, tt.expected, exp)
require.NoError(t, err)

// expect owner ref to be set to the es resource
Expand All @@ -80,6 +86,9 @@ func TestReconcileStatefulSet(t *testing.T) {
err = controllerutil.SetControllerReference(&es, metaObj, scheme.Scheme)
require.NoError(t, err)

// check expectations were updated
require.Equal(t, tt.wantExpectationsUpdated, len(exp.GetGenerations()) != 0)

// returned sset should match the expected one
require.Equal(t, tt.expected, returned)
// and be stored in the apiserver
Expand Down