Use pointer field for priority in member options (#1610)

* Use pointer field for priority in member options

* Ignore G115 for probes

* Skip leftover overflows

* Ignore G115 at golangi.yml level

* Sort members by rs member id

Author: mircea-cosbuc

.golangci.yml

@@ -28,7 +28,10 @@ linters:
     - rowserrcheck
     - gosec
     - unconvert
-
+linters-settings:
+  gosec:
+    excludes:
+      - G115
 run:
   modules-download-mode: mod
   # timeout for analysis, e.g. 30s, 5m, default is 1m

go.mod

@@ -17,6 +17,7 @@ require (
 	k8s.io/api v0.27.12
 	k8s.io/apimachinery v0.27.12
 	k8s.io/client-go v0.27.12
+	k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0
 	sigs.k8s.io/controller-runtime v0.15.3
 	sigs.k8s.io/yaml v1.4.0
 )
@@ -83,7 +84,6 @@ require (
 	k8s.io/component-base v0.27.12 // indirect
 	k8s.io/klog/v2 v2.90.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
-	k8s.io/utils v0.0.0-20230209194617-a36077c30491 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
 )

go.sum

@@ -311,6 +311,8 @@ k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5F
 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg=
 k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPBjNSSOMowRZxxsY=
 k8s.io/utils v0.0.0-20230209194617-a36077c30491/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
+k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak=
+k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 sigs.k8s.io/controller-runtime v0.15.3 h1:L+t5heIaI3zeejoIyyvLQs5vTVu/67IU2FfisVzFlBc=
 sigs.k8s.io/controller-runtime v0.15.3/go.mod h1:kp4jckA4vTx281S/0Yk2LFEEQe67mjg+ev/yknv47Ds=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=

pkg/automationconfig/automation_config.go

@@ -283,7 +283,7 @@ type ReplicaSetMember struct {
 	// is different in AC from the CR(CR don't support float) - hence all the members are declared
 	// separately
 	Votes    *int              `json:"votes,omitempty"`
-	Priority float32           `json:"priority,omitempty"`
+	Priority *float32          `json:"priority,omitempty"`
 	Tags     map[string]string `json:"tags,omitempty"`
 }
 
@@ -294,7 +294,7 @@ func newReplicaSetMember(name string, id int, horizons ReplicaSetHorizons, isArb
 	// ensure that the number of voting members in the replica set is not more than 7
 	// as this is the maximum number of voting members.
 	votes := 0
-	priority := 0.0
+	priority := float32(0.0)
 
 	if isVotingMember {
 		votes = 1
@@ -307,7 +307,7 @@ func newReplicaSetMember(name string, id int, horizons ReplicaSetHorizons, isArb
 		ArbiterOnly: isArbiter,
 		Horizons:    horizons,
 		Votes:       &votes,
-		Priority:    float32(priority),
+		Priority:    &priority,
 	}
 }
 

pkg/automationconfig/automation_config_builder.go

@@ -6,8 +6,9 @@ import (
 	"strings"
 
 	"github.com/blang/semver"
-
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/versions"
+
+	"k8s.io/utils/ptr"
 )
 
 type Topology string
@@ -351,7 +352,7 @@ func (b *Builder) Build() (AutomationConfig, error) {
 		if len(b.memberOptions) > i {
 			// override the member options if explicitly specified in the spec
 			members[i].Votes = b.memberOptions[i].Votes
-			members[i].Priority = b.memberOptions[i].GetPriority()
+			members[i].Priority = ptr.To(b.memberOptions[i].GetPriority())
 			members[i].Tags = b.memberOptions[i].Tags
 		}
 	}

pkg/automationconfig/automation_config_test.go

@@ -477,6 +477,23 @@ func TestAreEqual(t *testing.T) {
 		assert.NoError(t, err)
 		assert.False(t, areEqual)
 	})
+
+	t.Run("Automation Configs with nil and zero values are not equal", func(t *testing.T) {
+		votes := 1
+		priority := "0.0"
+		firstBuilder := NewBuilder().SetName("name0").SetMongoDBVersion("mdbVersion0").SetOptions(Options{DownloadBase: "downloadBase0"}).SetDomain("domain0").SetMembers(2).SetAuth(Auth{Disabled: true})
+		firstBuilder.SetMemberOptions([]MemberOptions{MemberOptions{Votes: &votes, Priority: &priority}})
+		firstAc, _ := firstBuilder.Build()
+		firstAc.Version = 2
+		secondBuilder := NewBuilder().SetName("name0").SetMongoDBVersion("mdbVersion0").SetOptions(Options{DownloadBase: "downloadBase0"}).SetDomain("domain0").SetMembers(2).SetAuth(Auth{Disabled: true})
+		secondBuilder.SetMemberOptions([]MemberOptions{MemberOptions{Votes: &votes, Priority: nil}})
+		secondAc, _ := secondBuilder.Build()
+		secondAc.Version = 2
+
+		areEqual, err := AreEqual(firstAc, secondAc)
+		assert.NoError(t, err)
+		assert.False(t, areEqual)
+	})
 }
 
 func TestValidateFCV(t *testing.T) {

test/e2e/mongodbtests/mongodbtests.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strconv"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -36,11 +36,6 @@ func SkipTestIfLocal(t *testing.T, msg string, f func(t *testing.T)) {
 	t.Run(msg, f)
 }
 
-func strPointer(n float32) *string {
-	s := strconv.FormatFloat(float64(n), 'f', -1, 64)
-	return &s
-}
-
 // StatefulSetBecomesReady ensures that the underlying stateful set
 // reaches the running state.
 func StatefulSetBecomesReady(ctx context.Context, mdb *mdbv1.MongoDBCommunity, opts ...wait.Configuration) func(t *testing.T) {
@@ -435,10 +430,13 @@ func AutomationConfigHasVoteTagPriorityConfigured(ctx context.Context, mdb *mdbv
 
 	return func(t *testing.T) {
 		currentAc := getAutomationConfig(ctx, t, mdb)
-		rsMemebers := currentAc.ReplicaSets
+		rsMembers := currentAc.ReplicaSets
+		sort.Slice(rsMembers[0].Members, func(i, j int) bool {
+			return rsMembers[0].Members[i].Id < rsMembers[0].Members[j].Id
+		})
 
-		for _, m := range rsMemebers[0].Members {
-			acMemberOptions = append(acMemberOptions, automationconfig.MemberOptions{Votes: m.Votes, Priority: strPointer(m.Priority), Tags: m.Tags})
+		for _, m := range rsMembers[0].Members {
+			acMemberOptions = append(acMemberOptions, automationconfig.MemberOptions{Votes: m.Votes, Priority: floatPtrTostringPtr(m.Priority), Tags: m.Tags})
 		}
 		assert.ElementsMatch(t, memberOptions, acMemberOptions)
 	}
@@ -825,3 +823,11 @@ func AddUserToMongoDBCommunity(ctx context.Context, mdb *mdbv1.MongoDBCommunity,
 		}
 	}
 }
+
+func floatPtrTostringPtr(floatPtr *float32) *string {
+	if floatPtr != nil {
+		stringValue := fmt.Sprintf("%.1f", *floatPtr)
+		return &stringValue
+	}
+	return nil
+}

test/e2e/replica_set/replica_set_test.go

@@ -70,7 +70,7 @@ func TestReplicaSet(t *testing.T) {
 		{
 			Votes:    intPtr(1),
 			Tags:     map[string]string{"foo2": "bar2"},
-			Priority: strPtr("1"),
+			Priority: strPtr("1.0"),
 		},
 		{
 			Votes:    intPtr(1),