Issue #368 - ensure that all roles have a replica count set

* Issue #368 - ensure that roles always have a replica value

* Issue #368 - amend scale to zero test role to not have a replica count

* Issue #368 - test modification

Author: thegridman

Diff for: pkg/controller/coherencecluster/coherencecluster_controller.go

@@ -400,14 +400,22 @@ func (r *ReconcileCoherenceCluster) ensureWkaService(cluster *coherence.Coherenc
 func (r *ReconcileCoherenceCluster) getDesiredRoles(cluster *coherence.CoherenceCluster) (map[string]coherence.CoherenceRoleSpec, []string) {
 	defaultSpec := cluster.Spec.CoherenceRoleSpec
 	if len(cluster.Spec.Roles) == 0 {
-		return map[string]coherence.CoherenceRoleSpec{defaultSpec.GetRoleName(): defaultSpec}, []string{defaultSpec.GetRoleName()}
+		clone := *defaultSpec.DeepCopy()
+		clone.SetReplicas(clone.GetReplicas())
+		return map[string]coherence.CoherenceRoleSpec{clone.GetRoleName(): clone}, []string{clone.GetRoleName()}
 	}
 
 	m := make(map[string]coherence.CoherenceRoleSpec)
 	names := make([]string, len(cluster.Spec.Roles))
 	index := 0
 	for _, role := range cluster.Spec.Roles {
 		clone := role.DeepCopyWithDefaults(&defaultSpec)
+
+		// Ensure that the role specifically has a replica value set.
+		// The original yaml may not have had a replicas field but some versions of kubectl scale
+		// will not work if the field is missing.
+		clone.SetReplicas(clone.GetReplicas())
+
 		names[index] = role.GetRoleName()
 		m[names[index]] = *clone
 		index++

Diff for: pkg/controller/coherencecluster/coherencecluster_controller_test.go

@@ -16,6 +16,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	stubs "github.com/oracle/coherence-operator/pkg/fakes"
@@ -78,7 +79,7 @@ var _ = Describe("coherencecluster_controller", func() {
 				mgr.AssertCoherenceRoles(testNamespace, 1)
 				name := cluster.Spec.CoherenceRoleSpec.GetFullRoleName(cluster)
 				role := mgr.AssertCoherenceRoleExists(testNamespace, name)
-				Expect(role.Spec).To(Equal(coherence.CoherenceRoleSpec{}))
+				Expect(role.Spec).To(Equal(coherence.CoherenceRoleSpec{Replicas: pointer.Int32Ptr(coherence.DefaultReplicas)}))
 			})
 
 			It("should fire a successful CoherenceRole create event", func() {
@@ -149,7 +150,9 @@ var _ = Describe("coherencecluster_controller", func() {
 				mgr.AssertCoherenceRoles(testNamespace, 1)
 				name := roleSpec.GetFullRoleName(cluster)
 				role := mgr.AssertCoherenceRoleExists(testNamespace, name)
-				Expect(role.Spec).To(Equal(roleSpec))
+				expected := roleSpec.DeepCopy()
+				expected.SetReplicas(coherence.DefaultReplicas)
+				Expect(role.Spec).To(Equal(*expected))
 			})
 		})
 	})
@@ -220,13 +223,17 @@ var _ = Describe("coherencecluster_controller", func() {
 			It("should create a CoherenceRole for the first role", func() {
 				name := roleSpecOne.GetFullRoleName(cluster)
 				role := mgr.AssertCoherenceRoleExists(testNamespace, name)
-				Expect(role.Spec).To(Equal(roleSpecOne))
+				expected := roleSpecOne.DeepCopy()
+				expected.SetReplicas(coherence.DefaultReplicas)
+				Expect(role.Spec).To(Equal(*expected))
 			})
 
 			It("should create a CoherenceRole for the second role", func() {
 				name := roleSpecTwo.GetFullRoleName(cluster)
 				role := mgr.AssertCoherenceRoleExists(testNamespace, name)
-				Expect(role.Spec).To(Equal(roleSpecTwo))
+				expected := roleSpecTwo.DeepCopy()
+				expected.SetReplicas(coherence.DefaultReplicas)
+				Expect(role.Spec).To(Equal(*expected))
 			})
 		})
 	})
@@ -320,7 +327,9 @@ var _ = Describe("coherencecluster_controller", func() {
 					mgr.AssertCoherenceRoles(testNamespace, 1)
 					name := existingRoleSpec.GetFullRoleName(cluster)
 					role := mgr.AssertCoherenceRoleExists(testNamespace, name)
-					Expect(role.Spec).To(Equal(updatedRoleSpec))
+					expected := updatedRoleSpec.DeepCopy()
+					expected.SetReplicas(coherence.DefaultReplicas)
+					Expect(role.Spec).To(Equal(*expected))
 				})
 
 				It("should fire a successful CoherenceRole update event", func() {
@@ -426,7 +435,8 @@ var _ = Describe("coherencecluster_controller", func() {
 						Labels:    map[string]string{coherence.CoherenceClusterLabel: testClusterName},
 					},
 					Spec: coherence.CoherenceRoleSpec{
-						Role: "storage",
+						Role:     "storage",
+						Replicas: pointer.Int32Ptr(2),
 					},
 				}
 
@@ -437,7 +447,8 @@ var _ = Describe("coherencecluster_controller", func() {
 						Labels:    map[string]string{coherence.CoherenceClusterLabel: testClusterName},
 					},
 					Spec: coherence.CoherenceRoleSpec{
-						Role: "proxy",
+						Role:     "proxy",
+						Replicas: pointer.Int32Ptr(2),
 					},
 				}
 

Diff for: pkg/fakes/fake_manager.go

@@ -51,7 +51,7 @@ func NewFakeManager(initObjs ...runtime.Object) (*FakeManager, error) {
 
 	options := manager.Options{
 		Namespace:      helper.GetTestNamespace(),
-		MapperProvider: apiutil.NewDiscoveryRESTMapper,
+		MapperProvider: restMapper,
 		LeaderElection: false,
 	}
 
@@ -68,6 +68,8 @@ func NewFakeManager(initObjs ...runtime.Object) (*FakeManager, error) {
 	return &mgr, nil
 }
 
+var restMapper = apiutil.NewDiscoveryRESTMapper
+
 type FakeManager struct {
 	Scheme *runtime.Scheme
 	Client ClientWithErrors

Diff for: test/e2e/remote/scaling-to-zero-test.yaml

@@ -5,7 +5,6 @@ metadata:
 spec:
   roles:
   - role: one
-    replicas: 2
     scaling:
       policy: Parallel
     readinessProbe: