Merge pull request #232 from arangodb/bugfix/immutable-field-fix

Applying defaults after immutable fields have been reset
arangodb · Aug 7, 2018 · 6e960fe · 6e960fe
2 parents 8c9fee8 + 523f11e
commit 6e960fe
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 5 deletions.
diff --git a/pkg/apis/deployment/v1alpha/deployment_spec_test.go b/pkg/apis/deployment/v1alpha/deployment_spec_test.go
@@ -45,22 +45,39 @@ func TestDeploymentSpecSetDefaults(t *testing.T) {
 
 func TestDeploymentSpecResetImmutableFields(t *testing.T) {
 	tests := []struct {
-		Original DeploymentSpec
-		Target   DeploymentSpec
-		Expected DeploymentSpec
-		Result   []string
+		Original      DeploymentSpec
+		Target        DeploymentSpec
+		Expected      DeploymentSpec
+		ApplyDefaults bool
+		Result        []string
 	}{
 		// Valid "changes"
 		{
 			DeploymentSpec{Image: util.NewString("foo")},
 			DeploymentSpec{Image: util.NewString("foo2")},
 			DeploymentSpec{Image: util.NewString("foo2")},
+			false,
+			nil,
+		},
+		{
+			DeploymentSpec{Image: util.NewString("foo")},
+			DeploymentSpec{Image: util.NewString("foo2")},
+			DeploymentSpec{Image: util.NewString("foo2")},
+			true,
 			nil,
 		},
 		{
 			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullAlways)},
 			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullNever)},
 			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullNever)},
+			false,
+			nil,
+		},
+		{
+			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullAlways)},
+			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullNever)},
+			DeploymentSpec{ImagePullPolicy: util.NewPullPolicy(v1.PullNever)},
+			true,
 			nil,
 		},
 
@@ -69,12 +86,31 @@ func TestDeploymentSpecResetImmutableFields(t *testing.T) {
 			DeploymentSpec{Mode: NewMode(DeploymentModeSingle)},
 			DeploymentSpec{Mode: NewMode(DeploymentModeCluster)},
 			DeploymentSpec{Mode: NewMode(DeploymentModeSingle)},
+			false,
 			[]string{"mode"},
 		},
+		{
+			DeploymentSpec{Mode: NewMode(DeploymentModeSingle)},
+			DeploymentSpec{Mode: NewMode(DeploymentModeCluster)},
+			DeploymentSpec{Mode: NewMode(DeploymentModeSingle)},
+			true,
+			[]string{"mode", "agents.count"},
+		},
 	}
 
 	for _, test := range tests {
+		if test.ApplyDefaults {
+			test.Original.SetDefaults("foo")
+			test.Expected.SetDefaults("foo")
+			test.Target.SetDefaultsFrom(test.Original)
+			test.Target.SetDefaults("foo")
+		}
 		result := test.Original.ResetImmutableFields(&test.Target)
+		if test.ApplyDefaults {
+			if len(result) > 0 {
+				test.Target.SetDefaults("foo")
+			}
+		}
 		assert.Equal(t, test.Result, result)
 		assert.Equal(t, test.Expected, test.Target)
 	}

diff --git a/pkg/apis/deployment/v1alpha/server_group_spec.go b/pkg/apis/deployment/v1alpha/server_group_spec.go
@@ -135,7 +135,7 @@ func (s *ServerGroupSpec) SetDefaults(group ServerGroup, used bool, mode Deploym
 			s.Count = util.NewInt(3)
 		}
 	} else if s.GetCount() > 0 && !used {
-		s.Count = util.NewInt(0)
+		s.Count = nil
 	}
 	if _, found := s.Resources.Requests[v1.ResourceStorage]; !found {
 		switch group {

diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go
@@ -292,6 +292,7 @@ func (d *Deployment) handleArangoDeploymentUpdatedEvent() error {
 	resetFields := specBefore.ResetImmutableFields(&newAPIObject.Spec)
 	if len(resetFields) > 0 {
 		log.Debug().Strs("fields", resetFields).Msg("Found modified immutable fields")
+		newAPIObject.Spec.SetDefaults(d.apiObject.GetName())
 	}
 	if err := newAPIObject.Spec.Validate(); err != nil {
 		d.CreateEvent(k8sutil.NewErrorEvent("Validation failed", err, d.apiObject))