From 815fa61ee7d0eba1fcc012cdf629b3798b24581e Mon Sep 17 00:00:00 2001 From: Bill Farner Date: Thu, 1 Dec 2016 12:39:50 -0800 Subject: [PATCH] Group plugin: remove unintended side-effect of pretend commit Signed-off-by: Bill Farner --- pkg/plugin/group/group.go | 3 +-- pkg/plugin/group/integration_test.go | 29 ++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/plugin/group/group.go b/pkg/plugin/group/group.go index 521ceb081..2a78c90c2 100644 --- a/pkg/plugin/group/group.go +++ b/pkg/plugin/group/group.go @@ -103,9 +103,8 @@ func (p *plugin) CommitGroup(config group.Spec, pretend bool) (string, error) { panic("Invalid empty allocation method") } - p.groups.put(config.ID, &groupContext{supervisor: supervisor, scaled: scaled, settings: settings}) - if !pretend { + p.groups.put(config.ID, &groupContext{supervisor: supervisor, scaled: scaled, settings: settings}) go supervisor.Run() } diff --git a/pkg/plugin/group/integration_test.go b/pkg/plugin/group/integration_test.go index 77ffb4191..def619e44 100644 --- a/pkg/plugin/group/integration_test.go +++ b/pkg/plugin/group/integration_test.go @@ -424,7 +424,6 @@ func TestInstanceAndFlavorChange(t *testing.T) { desc, err := grp.CommitGroup(updated, true) require.NoError(t, err) - require.Equal(t, "Performing a rolling update on 3 instances", desc) _, err = grp.CommitGroup(updated, false) @@ -461,7 +460,6 @@ func TestFlavorChange(t *testing.T) { desc, err := grp.CommitGroup(updated, true) require.NoError(t, err) - require.Equal(t, "Performing a rolling update on 3 instances", desc) require.NoError(t, grp.FreeGroup(id)) @@ -562,3 +560,30 @@ func TestUpdateFailsWhenInstanceIsUnhealthy(t *testing.T) { require.Equal(t, 1, badUpdateInstanaces) require.NoError(t, grp.FreeGroup(id)) } + +func TestNoSideEffectsFromPretendCommit(t *testing.T) { + // Tests that internal state is not modified by a GroupCommit with Pretend=true. + + plugin := newTestInstancePlugin() + grp := NewGroupPlugin(pluginLookup(pluginName, plugin), flavorPluginLookup, 1*time.Millisecond) + + desc, err := grp.CommitGroup(minions, true) + require.NoError(t, err) + require.Equal(t, "Managing 3 instances", desc) + + desc, err = grp.CommitGroup(minions, true) + require.NoError(t, err) + require.Equal(t, "Managing 3 instances", desc) + + err = grp.FreeGroup(id) + require.Error(t, err) + require.Equal(t, "Group 'testGroup' is not being watched", err.Error()) + + err = grp.DestroyGroup(id) + require.Error(t, err) + require.Equal(t, "Group 'testGroup' is not being watched", err.Error()) + + desc, err = grp.CommitGroup(minions, true) + require.NoError(t, err) + require.Equal(t, "Managing 3 instances", desc) +}