Skip to content

Commit 85ad151

Browse files
author
Nikolas De Giorgis
authored
CLOUDP-64701: ensure automation config version bump (#60)
1 parent 58f5911 commit 85ad151

File tree

10 files changed

+147
-24
lines changed

10 files changed

+147
-24
lines changed

‎pkg/automationconfig/automation_config_builder.go‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package automationconfig
22

33
import (
4+
"bytes"
5+
"encoding/json"
46
"fmt"
57
)
68

@@ -21,7 +23,7 @@ type Builder struct {
2123
fcv string
2224
topology Topology
2325
mongodbVersion string
24-
26+
previousAC AutomationConfig
2527
// MongoDB installable versions
2628
versions []MongoDbVersionConfig
2729
}
@@ -74,12 +76,11 @@ func (b *Builder) SetMongoDBVersion(version string) *Builder {
7476
return b
7577
}
7678

77-
func (b *Builder) SetAutomationConfigVersion(version int) *Builder {
78-
b.version = version
79+
func (b *Builder) SetPreviousAutomationConfig(previousAC AutomationConfig) *Builder {
80+
b.previousAC = previousAC
7981
return b
8082
}
81-
82-
func (b *Builder) Build() AutomationConfig {
83+
func (b *Builder) Build() (AutomationConfig, error) {
8384
hostnames := make([]string, b.members)
8485
for i := 0; i < b.members; i++ {
8586
hostnames[i] = fmt.Sprintf("%s-%d.%s", b.name, i, b.domain)
@@ -93,8 +94,8 @@ func (b *Builder) Build() AutomationConfig {
9394
members[i] = newReplicaSetMember(process, i)
9495
}
9596

96-
return AutomationConfig{
97-
Version: b.version,
97+
currentAc := AutomationConfig{
98+
Version: b.previousAC.Version,
9899
Processes: processes,
99100
ReplicaSets: []ReplicaSet{
100101
{
@@ -107,6 +108,26 @@ func (b *Builder) Build() AutomationConfig {
107108
Options: Options{DownloadBase: "/var/lib/mongodb-mms-automation"},
108109
Auth: DisabledAuth(),
109110
}
111+
112+
// Here we compare the bytes of the two automationconfigs,
113+
// we can't use reflect.DeepEqual() as it treats nil entries as different from empty ones,
114+
// and in the AutomationConfig Struct we use omitempty to set empty field to nil
115+
// The agent requires the nil value we provide, otherwise the agent attempts to configure authentication.
116+
117+
newAcBytes, err := json.Marshal(b.previousAC)
118+
if err != nil {
119+
return AutomationConfig{}, err
120+
}
121+
122+
currentAcBytes, err := json.Marshal(currentAc)
123+
if err != nil {
124+
return AutomationConfig{}, err
125+
}
126+
127+
if bytes.Compare(newAcBytes, currentAcBytes) != 0 {
128+
currentAc.Version += 1
129+
}
130+
return currentAc, nil
110131
}
111132

112133
func toHostName(name string, index int) string {

‎pkg/automationconfig/automation_config_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ func defaultMongoDbVersion(version string) MongoDbVersionConfig {
2626

2727
func TestBuildAutomationConfig(t *testing.T) {
2828

29-
ac := NewBuilder().
29+
ac, err := NewBuilder().
3030
SetName("my-rs").
3131
SetDomain("my-ns.svc.cluster.local").
3232
SetMongoDBVersion("4.2.0").
33-
SetAutomationConfigVersion(1).
3433
SetMembers(3).
3534
SetFCV("4.0").
3635
Build()
3736

37+
assert.NoError(t, err)
3838
assert.Len(t, ac.Processes, 3)
3939
assert.Equal(t, 1, ac.Version)
4040

@@ -63,15 +63,15 @@ func TestBuildAutomationConfig(t *testing.T) {
6363

6464
func TestMongoDbVersions(t *testing.T) {
6565

66-
ac := NewBuilder().
66+
ac, err := NewBuilder().
6767
SetName("my-rs").
6868
SetDomain("my-ns.svc.cluster.local").
6969
SetMongoDBVersion("4.2.0").
70-
SetAutomationConfigVersion(1).
7170
SetMembers(3).
7271
AddVersion(defaultMongoDbVersion("4.2.0")).
7372
Build()
7473

74+
assert.NoError(t, err)
7575
assert.Len(t, ac.Processes, 3)
7676
assert.Len(t, ac.Versions, 1)
7777
assert.Len(t, ac.Versions[0].Builds, 1)
@@ -90,59 +90,59 @@ func TestMongoDbVersions(t *testing.T) {
9090
},
9191
)
9292

93-
ac = NewBuilder().
93+
ac, err = NewBuilder().
9494
SetName("my-rs").
9595
SetDomain("my-ns.svc.cluster.local").
9696
SetMongoDBVersion("4.2.0").
97-
SetAutomationConfigVersion(1).
9897
SetMembers(3).
9998
AddVersion(defaultMongoDbVersion("4.2.0")).
10099
AddVersion(version2).
101100
Build()
102101

102+
assert.NoError(t, err)
103103
assert.Len(t, ac.Processes, 3)
104104
assert.Len(t, ac.Versions, 2)
105105
assert.Len(t, ac.Versions[0].Builds, 1)
106106
assert.Len(t, ac.Versions[1].Builds, 2)
107107
}
108108

109109
func TestHasOptions(t *testing.T) {
110-
ac := NewBuilder().
110+
ac, err := NewBuilder().
111111
SetName("my-rs").
112112
SetDomain("my-ns.svc.cluster.local").
113113
SetMongoDBVersion("4.2.0").
114-
SetAutomationConfigVersion(1).
115114
SetMembers(3).
116115
Build()
117116

117+
assert.NoError(t, err)
118118
assert.Equal(t, ac.Options.DownloadBase, "/var/lib/mongodb-mms-automation")
119119
}
120120

121121
func TestModulesNotNil(t *testing.T) {
122122
// We make sure the .Modules is initialized as an empty list of strings
123123
// or it will dumped as null attribute in json.
124-
ac := NewBuilder().
124+
ac, err := NewBuilder().
125125
SetName("my-rs").
126126
SetDomain("my-ns.svc.cluster.local").
127127
SetMongoDBVersion("4.2.0").
128-
SetAutomationConfigVersion(1).
129128
SetMembers(3).
130129
AddVersion(defaultMongoDbVersion("4.3.2")).
131130
Build()
132131

132+
assert.NoError(t, err)
133133
assert.NotNil(t, ac.Versions[0].Builds[0].Modules)
134134
}
135135

136136
func TestProcessHasPortSetToDefault(t *testing.T) {
137-
ac := NewBuilder().
137+
ac, err := NewBuilder().
138138
SetName("my-rs").
139139
SetDomain("my-ns.svc.cluster.local").
140140
SetMongoDBVersion("4.2.0").
141-
SetAutomationConfigVersion(1).
142141
SetMembers(3).
143142
AddVersion(defaultMongoDbVersion("4.3.2")).
144143
Build()
145144

145+
assert.NoError(t, err)
146146
assert.Len(t, ac.Processes, 3)
147147
assert.Equal(t, ac.Processes[0].Args26.Net.Port, 27017)
148148
assert.Equal(t, ac.Processes[1].Args26.Net.Port, 27017)

‎pkg/controller/mongodb/mongodb_controller.go‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/apimachinery/pkg/runtime"
2626
"k8s.io/apimachinery/pkg/runtime/schema"
2727
"k8s.io/apimachinery/pkg/types"
28+
k8sClient "sigs.k8s.io/controller-runtime/pkg/client"
2829
"sigs.k8s.io/controller-runtime/pkg/controller"
2930
"sigs.k8s.io/controller-runtime/pkg/handler"
3031
"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -130,7 +131,6 @@ func (r *ReplicaSetReconciler) Reconcile(request reconcile.Request) (reconcile.R
130131
return reconcile.Result{}, err
131132
}
132133

133-
// TODO: Read current automation config version from config map
134134
if err := r.ensureAutomationConfig(mdb); err != nil {
135135
r.log.Infof("error creating automation config config map: %s", err)
136136
return reconcile.Result{}, err
@@ -265,24 +265,36 @@ func (r ReplicaSetReconciler) ensureAutomationConfig(mdb mdbv1.MongoDB) error {
265265
if err != nil {
266266
return err
267267
}
268+
268269
if err := r.client.CreateOrUpdate(&cm); err != nil {
269270
return err
270271
}
271272
return nil
272273
}
273274

274-
func buildAutomationConfig(mdb mdbv1.MongoDB, mdbVersionConfig automationconfig.MongoDbVersionConfig) automationconfig.AutomationConfig {
275+
func buildAutomationConfig(mdb mdbv1.MongoDB, mdbVersionConfig automationconfig.MongoDbVersionConfig, client mdbClient.Client) (automationconfig.AutomationConfig, error) {
275276
domain := getDomain(mdb.ServiceName(), mdb.Namespace, "")
276-
return automationconfig.NewBuilder().
277+
278+
currentAc, err := getCurrentAutomationConfig(client, mdb)
279+
if err != nil {
280+
return automationconfig.AutomationConfig{}, err
281+
}
282+
newAc, err := automationconfig.NewBuilder().
277283
SetTopology(automationconfig.ReplicaSetTopology).
278284
SetName(mdb.Name).
279285
SetDomain(domain).
280286
SetMembers(mdb.Spec.Members).
287+
SetPreviousAutomationConfig(currentAc).
281288
SetMongoDBVersion(mdb.Spec.Version).
282-
SetAutomationConfigVersion(1). // TODO: Correctly set the version
283289
SetFCV(mdb.GetFCV()).
284290
AddVersion(mdbVersionConfig).
285291
Build()
292+
293+
if err != nil {
294+
return automationconfig.AutomationConfig{}, err
295+
}
296+
297+
return newAc, nil
286298
}
287299

288300
func readVersionManifestFromDisk() (automationconfig.VersionManifest, error) {
@@ -318,12 +330,30 @@ func buildService(mdb mdbv1.MongoDB) corev1.Service {
318330
Build()
319331
}
320332

333+
func getCurrentAutomationConfig(client mdbClient.Client, mdb mdbv1.MongoDB) (automationconfig.AutomationConfig, error) {
334+
currentCm := corev1.ConfigMap{}
335+
currentAc := automationconfig.AutomationConfig{}
336+
if err := client.Get(context.TODO(), types.NamespacedName{Name: mdb.ConfigMapName(), Namespace: mdb.Namespace}, &currentCm); err != nil {
337+
// If the AC was not found we don't surface it as an error
338+
return automationconfig.AutomationConfig{}, k8sClient.IgnoreNotFound(err)
339+
340+
}
341+
if err := json.Unmarshal([]byte(currentCm.Data[AutomationConfigKey]), &currentAc); err != nil {
342+
return automationconfig.AutomationConfig{}, err
343+
}
344+
return currentAc, nil
345+
}
346+
321347
func (r ReplicaSetReconciler) buildAutomationConfigConfigMap(mdb mdbv1.MongoDB) (corev1.ConfigMap, error) {
322348
manifest, err := r.manifestProvider()
323349
if err != nil {
324350
return corev1.ConfigMap{}, fmt.Errorf("error reading version manifest from disk: %+v", err)
325351
}
326-
ac := buildAutomationConfig(mdb, manifest.BuildsForVersion(mdb.Spec.Version))
352+
353+
ac, err := buildAutomationConfig(mdb, manifest.BuildsForVersion(mdb.Spec.Version), r.client)
354+
if err != nil {
355+
return corev1.ConfigMap{}, err
356+
}
327357
acBytes, err := json.Marshal(ac)
328358
if err != nil {
329359
return corev1.ConfigMap{}, err

‎pkg/controller/mongodb/replicaset_controller_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,5 +181,48 @@ func TestService_isCorrectlyCreatedAndUpdated(t *testing.T) {
181181

182182
res, err = r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
183183
assertReconciliationSuccessful(t, res, err)
184+
}
185+
186+
func TestAutomationConfig_versionIsBumpedOnChange(t *testing.T) {
187+
mdb := newTestReplicaSet()
188+
189+
mgr := client.NewManager(&mdb)
190+
r := newReconciler(mgr, mockManifestProvider(mdb.Spec.Version))
191+
res, err := r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
192+
assertReconciliationSuccessful(t, res, err)
193+
194+
currentAc, err := getCurrentAutomationConfig(client.NewClient(mgr.GetClient()), mdb)
195+
assert.NoError(t, err)
196+
assert.Equal(t, 1, currentAc.Version)
197+
198+
mdb.Spec.Members += 1
199+
200+
_ = mgr.GetClient().Update(context.TODO(), &mdb)
201+
res, err = r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
202+
assertReconciliationSuccessful(t, res, err)
203+
204+
currentAc, err = getCurrentAutomationConfig(client.NewClient(mgr.GetClient()), mdb)
205+
assert.NoError(t, err)
206+
assert.Equal(t, 2, currentAc.Version)
184207

185208
}
209+
210+
func TestAutomationConfig_versionIsNotBumpedWithNoChanges(t *testing.T) {
211+
mdb := newTestReplicaSet()
212+
213+
mgr := client.NewManager(&mdb)
214+
r := newReconciler(mgr, mockManifestProvider(mdb.Spec.Version))
215+
res, err := r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
216+
assertReconciliationSuccessful(t, res, err)
217+
218+
currentAc, err := getCurrentAutomationConfig(client.NewClient(mgr.GetClient()), mdb)
219+
assert.NoError(t, err)
220+
assert.Equal(t, currentAc.Version, 1)
221+
222+
res, err = r.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: mdb.Namespace, Name: mdb.Name}})
223+
assertReconciliationSuccessful(t, res, err)
224+
225+
currentAc, err = getCurrentAutomationConfig(client.NewClient(mgr.GetClient()), mdb)
226+
assert.NoError(t, err)
227+
assert.Equal(t, currentAc.Version, 1)
228+
}

‎test/e2e/mongodbtests/mongodbtests.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mongodbtests
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"reflect"
78
"testing"
@@ -12,6 +13,7 @@ import (
1213
"k8s.io/apimachinery/pkg/util/wait"
1314

1415
mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/pkg/apis/mongodb/v1"
16+
"github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig"
1517
"github.com/mongodb/mongodb-kubernetes-operator/pkg/controller/mongodb"
1618
e2eutil "github.com/mongodb/mongodb-kubernetes-operator/test/e2e"
1719
f "github.com/operator-framework/operator-sdk/pkg/test"
@@ -105,6 +107,18 @@ func AutomationConfigConfigMapExists(mdb *mdbv1.MongoDB) func(t *testing.T) {
105107
}
106108
}
107109

110+
func AutomationConfigVersionHasTheExpectedVersion(mdb *mdbv1.MongoDB, expectedVersion int) func(t *testing.T) {
111+
return func(t *testing.T) {
112+
currentCm := corev1.ConfigMap{}
113+
currentAc := automationconfig.AutomationConfig{}
114+
err := f.Global.Client.Get(context.TODO(), types.NamespacedName{Name: mdb.ConfigMapName(), Namespace: mdb.Namespace}, &currentCm)
115+
assert.NoError(t, err)
116+
err = json.Unmarshal([]byte(currentCm.Data[mongodb.AutomationConfigKey]), &currentAc)
117+
assert.NoError(t, err)
118+
assert.Equal(t, expectedVersion, currentAc.Version)
119+
}
120+
}
121+
108122
// HasFeatureCompatibilityVersion verifies that the FeatureCompatibilityVersion is
109123
// set to `version`. The FCV parameter is not signaled as a non Running state, for
110124
// this reason, this function checks the value of the parameter many times, based

‎test/e2e/replica_set/replica_set_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ func TestReplicaSet(t *testing.T) {
3434
})))
3535
t.Run("MongoDB Reaches Running Phase", mongodbtests.MongoDBReachesRunningPhase(&mdb))
3636
t.Run("Test Basic Connectivity", mongodbtests.BasicConnectivity(&mdb))
37+
t.Run("AutomationConfig has the correct version", mongodbtests.AutomationConfigVersionHasTheExpectedVersion(&mdb, 1))
3738
t.Run("Test Status Was Updated", mongodbtests.Status(&mdb,
3839
mdbv1.MongoDBStatus{
3940
MongoURI: mdb.MongoURI(),

‎test/e2e/replica_set_change_version/replica_set_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ func TestReplicaSetUpgradeVersion(t *testing.T) {
2828
t.Run("Config Map Was Correctly Created", mongodbtests.AutomationConfigConfigMapExists(&mdb))
2929
t.Run("Stateful Set Reaches Ready State", mongodbtests.StatefulSetIsReady(&mdb))
3030
t.Run("MongoDB Reaches Running Phase", mongodbtests.MongoDBReachesRunningPhase(&mdb))
31+
t.Run("AutomationConfig has the correct version", mongodbtests.AutomationConfigVersionHasTheExpectedVersion(&mdb, 1))
3132
t.Run("Test Basic Connectivity", mongodbtests.BasicConnectivity(&mdb))
3233
t.Run("Test Status Was Updated", mongodbtests.Status(&mdb,
3334
mdbv1.MongoDBStatus{
@@ -41,6 +42,8 @@ func TestReplicaSetUpgradeVersion(t *testing.T) {
4142
t.Run("Test Version can be upgraded", mongodbtests.ChangeVersion(&mdb, "4.0.8"))
4243
t.Run("StatefulSet has OnDelete update strategy", mongodbtests.StatefulSetHasUpdateStrategy(&mdb, appsv1.OnDeleteStatefulSetStrategyType))
4344
t.Run("Stateful Set Reaches Ready State, after Upgrading", mongodbtests.StatefulSetIsUpdated(&mdb))
45+
t.Run("AutomationConfig's version has been increased", mongodbtests.AutomationConfigVersionHasTheExpectedVersion(&mdb, 2))
46+
4447
},
4548
))
4649
t.Run("StatefulSet has RollingUpgrade restart strategy", mongodbtests.StatefulSetHasUpdateStrategy(&mdb, appsv1.RollingUpdateStatefulSetStrategyType))
@@ -51,6 +54,8 @@ func TestReplicaSetUpgradeVersion(t *testing.T) {
5154
t.Run("Test Version can be downgraded", mongodbtests.ChangeVersion(&mdb, "4.0.6"))
5255
t.Run("StatefulSet has OnDelete restart strategy", mongodbtests.StatefulSetHasUpdateStrategy(&mdb, appsv1.OnDeleteStatefulSetStrategyType))
5356
t.Run("Stateful Set Reaches Ready State, after Upgrading", mongodbtests.StatefulSetIsUpdated(&mdb))
57+
t.Run("AutomationConfig's version has been increased", mongodbtests.AutomationConfigVersionHasTheExpectedVersion(&mdb, 3))
58+
5459
},
5560
))
5661
t.Run("StatefulSet has RollingUpgrade restart strategy", mongodbtests.StatefulSetHasUpdateStrategy(&mdb, appsv1.RollingUpdateStatefulSetStrategyType))

0 commit comments

Comments
 (0)