CLOUDP-69096: Implement correct scale down logic (#190)

Author: chatton

.evergreen.yml

@@ -167,6 +167,7 @@ task_groups:
     - e2e_test_replica_set
     - e2e_test_replica_set_readiness_probe
     - e2e_test_replica_set_scale
+    - e2e_test_replica_set_scale_down
     - e2e_test_replica_set_change_version
     - e2e_test_feature_compatibility_version
     - e2e_test_feature_compatibility_version_upgrade
@@ -279,6 +280,13 @@ tasks:
         vars:
           test: replica_set_scale
 
+  - name: e2e_test_replica_set_scale_down
+    exec_timeout_secs: 3600
+    commands:
+      - func: run_e2e_test
+        vars:
+          test: replica_set_scale_down
+
   - name: e2e_test_replica_set_change_version
     commands:
       - func: run_e2e_test

agent/Dockerfile

@@ -19,9 +19,9 @@ RUN mkdir -p agent \
     && chmod -R +r /var/lib/automation/config \
     && rm agent/mongodb-agent.tar.gz \
     && rm -r mongodb-mms-automation-agent-*
-
 RUN mkdir -p /var/lib/mongodb-mms-automation/probes/ \
-    && curl --retry 3 https://readinessprobe.s3-us-west-1.amazonaws.com/readiness -o /var/lib/mongodb-mms-automation/probes/readinessprobe \
+#    && curl --retry 3 https://readinessprobe.s3-us-west-1.amazonaws.com/readiness -o /var/lib/mongodb-mms-automation/probes/readinessprobe \
+    && curl --retry 3 https://readiness-probe-scale-test.s3.amazonaws.com/readiness -o /var/lib/mongodb-mms-automation/probes/readinessprobe \
     && chmod +x /var/lib/mongodb-mms-automation/probes/readinessprobe \
     && mkdir -p /var/log/mongodb-mms-automation/ \
     && chmod -R +wr /var/log/mongodb-mms-automation/ \

cmd/testrunner/main.go

@@ -271,7 +271,8 @@ func withTest(test string) func(obj runtime.Object) {
 				"--kubeconfig",
 				"/etc/config/kubeconfig",
 				"--go-test-flags",
-				"-timeout=20m",
+				// TODO: allow this to be configurable per test, this is only large due to scale down test
+				"-timeout=60m",
 			}
 		}
 	}

deploy/crds/mongodb.com_mongodb_crd.yaml

@@ -175,18 +175,16 @@ spec:
         status:
           description: MongoDBStatus defines the observed state of MongoDB
           properties:
-            members:
+            currentStatefulSetReplicas:
+              type: integer
+            currentMongoDBMembers:
               type: integer
             message:
               type: string
             mongoUri:
               type: string
             phase:
               type: string
-          required:
-          - members
-          - mongoUri
-          - phase
           type: object
       type: object
   version: v1

deploy/operator.yaml

@@ -31,7 +31,8 @@ spec:
             - name: OPERATOR_NAME
               value: "mongodb-kubernetes-operator"
             - name: AGENT_IMAGE # The MongoDB Agent the operator will deploy to manage MongoDB deployments
-              value: quay.io/mongodb/mongodb-agent:10.19.0.6562-1
+              value: quay.io/mongodb/mongodb-agent-dev:scaledown
+#              value: quay.io/mongodb/mongodb-agent:10.19.0.6562-1
             - name: VERSION_UPGRADE_HOOK_IMAGE
               value: quay.io/mongodb/mongodb-kubernetes-operator-version-upgrade-post-start-hook:1.0.2
             - name: MONGODB_IMAGE

pkg/apis/mongodb/v1/mongodb_types.go

@@ -201,8 +201,11 @@ type AuthMode string
 type MongoDBStatus struct {
 	MongoURI string `json:"mongoUri"`
 	Phase    Phase  `json:"phase"`
-	Members  int    `json:"members"`
-	Message  string `json:"message,omitempty"`
+
+	CurrentStatefulSetReplicas int `json:"currentStatefulSetReplicas"`
+	CurrentMongoDBMembers      int `json:"currentMongoDBMembers"`
+
+	Message string `json:"message,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -220,16 +223,13 @@ type MongoDB struct {
 	Status MongoDBStatus `json:"status,omitempty"`
 }
 
-func (m MongoDB) DesiredReplicas() int {
-	return m.Spec.Members
-}
-
-func (m MongoDB) CurrentReplicas() int {
-	return m.Status.Members
-}
-
-func (m *MongoDB) ReplicasThisReconciliation() int {
-	return scale.ReplicasThisReconciliation(m)
+func (m MongoDB) AutomationConfigMembersThisReconciliation() int {
+	// determine the correct number of automation config replica set members
+	// based on our desired number, and our current number
+	return scale.ReplicasThisReconciliation(automationConfigReplicasScaler{
+		desired: m.Spec.Members,
+		current: m.Status.CurrentMongoDBMembers,
+	})
 }
 
 // MongoURI returns a mongo uri which can be used to connect to this deployment
@@ -298,6 +298,30 @@ func (m MongoDB) GetFCV() string {
 	return strings.Join(parts[:minorIndex+1], ".")
 }
 
+func (m MongoDB) DesiredReplicas() int {
+	return m.Spec.Members
+}
+
+func (m MongoDB) CurrentReplicas() int {
+	return m.Status.CurrentStatefulSetReplicas
+}
+
+func (m *MongoDB) StatefulSetReplicasThisReconciliation() int {
+	return scale.ReplicasThisReconciliation(m)
+}
+
+type automationConfigReplicasScaler struct {
+	current, desired int
+}
+
+func (a automationConfigReplicasScaler) DesiredReplicas() int {
+	return a.desired
+}
+
+func (a automationConfigReplicasScaler) CurrentReplicas() int {
+	return a.current
+}
+
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // MongoDBList contains a list of MongoDB

pkg/controller/mongodb/build_statefulset_test.go

@@ -54,7 +54,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDB, sts *app
 	assert.Equal(t, "agent-image", agentContainer.Image)
 	probe := agentContainer.ReadinessProbe
 	assert.True(t, reflect.DeepEqual(probes.New(defaultReadiness()), *probe))
-	assert.Equal(t, int32(240), probe.FailureThreshold)
+	assert.Equal(t, probes.New(defaultReadiness()).FailureThreshold, probe.FailureThreshold)
 	assert.Equal(t, int32(5), probe.InitialDelaySeconds)
 	assert.Len(t, agentContainer.VolumeMounts, 4)
 

pkg/controller/mongodb/mongodb_status_options.go

@@ -1,9 +1,8 @@
 package mongodb
 
 import (
-	"time"
-
 	mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/pkg/apis/mongodb/v1"
+	"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/result"
 	"go.uber.org/zap"
 
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/status"
@@ -16,6 +15,7 @@ type severity string
 
 const (
 	Info  severity = "INFO"
+	Debug severity = "DEBUG"
 	Warn  severity = "WARN"
 	Error severity = "ERROR"
 	None  severity = "NONE"
@@ -56,28 +56,9 @@ func (m mongoUriOption) ApplyOption(mdb *mdbv1.MongoDB) {
 }
 
 func (m mongoUriOption) GetResult() (reconcile.Result, error) {
-	return okResult()
+	return result.OK()
 }
 
-func (o *optionBuilder) withMembers(members int) *optionBuilder {
-	o.options = append(o.options,
-		membersOption{
-			members: members,
-		})
-	return o
-}
-
-type membersOption struct {
-	members int
-}
-
-func (m membersOption) ApplyOption(mdb *mdbv1.MongoDB) {
-	mdb.Status.Members = m.members
-}
-
-func (m membersOption) GetResult() (reconcile.Result, error) {
-	return okResult()
-}
 func (o *optionBuilder) withPhase(phase mdbv1.Phase, retryAfter int) *optionBuilder {
 	o.options = append(o.options,
 		phaseOption{
@@ -99,18 +80,35 @@ type messageOption struct {
 func (m messageOption) ApplyOption(mdb *mdbv1.MongoDB) {
 	mdb.Status.Message = m.message.messageString
 	if m.message.severityLevel == Error {
-		zap.S().Error(m.message)
+		zap.S().Error(m.message.messageString)
 	}
 	if m.message.severityLevel == Warn {
-		zap.S().Warn(m.message)
+		zap.S().Warn(m.message.messageString)
 	}
 	if m.message.severityLevel == Info {
-		zap.S().Info(m.message)
+		zap.S().Info(m.message.messageString)
+	}
+	if m.message.severityLevel == Debug {
+		zap.S().Debug(m.message.messageString)
 	}
 }
 
 func (m messageOption) GetResult() (reconcile.Result, error) {
-	return okResult()
+	return result.OK()
+}
+
+func (o *optionBuilder) withMongoDBMembers(members int) *optionBuilder {
+	o.options = append(o.options, mongoDBReplicasOption{
+		mongoDBMembers: members,
+	})
+	return o
+}
+
+func (o *optionBuilder) withStatefulSetReplicas(members int) *optionBuilder {
+	o.options = append(o.options, statefulSetReplicasOption{
+		replicas: members,
+	})
+	return o
 }
 
 func (o *optionBuilder) withMessage(severityLevel severity, msg string) *optionBuilder {
@@ -146,28 +144,37 @@ func (p phaseOption) ApplyOption(mdb *mdbv1.MongoDB) {
 
 func (p phaseOption) GetResult() (reconcile.Result, error) {
 	if p.phase == mdbv1.Running {
-		return okResult()
+		return result.OK()
 	}
 	if p.phase == mdbv1.Pending {
-		return retryResult(p.retryAfter)
+		return result.Retry(p.retryAfter)
 	}
 	if p.phase == mdbv1.Failed {
-		return failedResult()
+		return result.Failed()
 	}
-	return okResult()
+	return result.OK()
+}
+
+type mongoDBReplicasOption struct {
+	mongoDBMembers int
 }
 
-// helper functions which return reconciliation results which should be
-// returned from the main reconciliation loop
+func (a mongoDBReplicasOption) ApplyOption(mdb *mdbv1.MongoDB) {
+	mdb.Status.CurrentMongoDBMembers = a.mongoDBMembers
+}
+
+func (a mongoDBReplicasOption) GetResult() (reconcile.Result, error) {
+	return result.OK()
+}
 
-func okResult() (reconcile.Result, error) {
-	return reconcile.Result{}, nil
+type statefulSetReplicasOption struct {
+	replicas int
 }
 
-func retryResult(after int) (reconcile.Result, error) {
-	return reconcile.Result{Requeue: true, RequeueAfter: time.Second * time.Duration(after)}, nil
+func (s statefulSetReplicasOption) ApplyOption(mdb *mdbv1.MongoDB) {
+	mdb.Status.CurrentStatefulSetReplicas = s.replicas
 }
 
-func failedResult() (reconcile.Result, error) {
-	return retryResult(0)
+func (s statefulSetReplicasOption) GetResult() (reconcile.Result, error) {
+	return result.OK()
 }

pkg/controller/mongodb/mongodb_status_options_test.go

@@ -22,19 +22,6 @@ func TestMongoUriOption_ApplyOption(t *testing.T) {
 	assert.Equal(t, "my-uri", mdb.Status.MongoURI, "Status should be updated")
 }
 
-func TestMembersOption_ApplyOption(t *testing.T) {
-	mdb := newReplicaSet(3, "my-rs", "my-ns")
-
-	opt := membersOption{
-		members: 5,
-	}
-
-	opt.ApplyOption(&mdb)
-
-	assert.Equal(t, 3, mdb.Spec.Members, "Spec should remain unchanged")
-	assert.Equal(t, 5, mdb.Status.Members, "Status should be updated")
-}
-
 func TestOptionBuilder_RunningPhase(t *testing.T) {
 	mdb := newReplicaSet(3, "my-rs", "my-ns")