Skip to content

Commit 6e53d3c

Browse files
m1kolam1kola
authored
Refactoring: propagate env vars via parameters (#1676)
1 parent 7a5cd08 commit 6e53d3c

File tree

9 files changed

+222
-244
lines changed

9 files changed

+222
-244
lines changed

‎cmd/manager/main.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ package main
33
import (
44
"fmt"
55
"os"
6+
67
"sigs.k8s.io/controller-runtime/pkg/cache"
78

89
mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
910
"github.com/mongodb/mongodb-kubernetes-operator/controllers"
1011
"github.com/mongodb/mongodb-kubernetes-operator/controllers/construct"
12+
"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/envvar"
1113
"go.uber.org/zap"
1214
"k8s.io/apimachinery/pkg/runtime"
1315
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -56,7 +58,14 @@ func main() {
5658
log.Sugar().Fatalf("Failed to configure logger: %v", err)
5759
}
5860

59-
if !hasRequiredVariables(log, construct.AgentImageEnv, construct.VersionUpgradeHookImageEnv, construct.ReadinessProbeImageEnv) {
61+
if !hasRequiredVariables(
62+
log,
63+
construct.MongodbRepoUrlEnv,
64+
construct.MongodbImageEnv,
65+
construct.AgentImageEnv,
66+
construct.VersionUpgradeHookImageEnv,
67+
construct.ReadinessProbeImageEnv,
68+
) {
6069
os.Exit(1)
6170
}
6271

@@ -99,7 +108,15 @@ func main() {
99108
}
100109

101110
// Setup Controller.
102-
if err = controllers.NewReconciler(mgr).SetupWithManager(mgr); err != nil {
111+
if err = controllers.NewReconciler(
112+
mgr,
113+
os.Getenv(construct.MongodbRepoUrlEnv),
114+
os.Getenv(construct.MongodbImageEnv),
115+
envvar.GetEnvOrDefault(construct.MongoDBImageTypeEnv, construct.DefaultImageType),
116+
os.Getenv(construct.AgentImageEnv),
117+
os.Getenv(construct.VersionUpgradeHookImageEnv),
118+
os.Getenv(construct.ReadinessProbeImageEnv),
119+
).SetupWithManager(mgr); err != nil {
103120
log.Sugar().Fatalf("Unable to create controller: %v", err)
104121
}
105122
// +kubebuilder:scaffold:builder

‎controllers/construct/build_statefulset_test.go‎

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

33
import (
4-
"os"
54
"reflect"
65
"testing"
76

@@ -21,10 +20,6 @@ import (
2120
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2221
)
2322

24-
func init() {
25-
os.Setenv(VersionUpgradeHookImageEnv, "version-upgrade-hook-image")
26-
}
27-
2823
func newTestReplicaSet() mdbv1.MongoDBCommunity {
2924
return mdbv1.MongoDBCommunity{
3025
ObjectMeta: metav1.ObjectMeta{
@@ -40,12 +35,8 @@ func newTestReplicaSet() mdbv1.MongoDBCommunity {
4035
}
4136

4237
func TestMultipleCalls_DoNotCauseSideEffects(t *testing.T) {
43-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
44-
t.Setenv(MongodbImageEnv, "mongodb-community-server")
45-
t.Setenv(AgentImageEnv, "agent-image")
46-
4738
mdb := newTestReplicaSet()
48-
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, os.Getenv(AgentImageEnv), true)
39+
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, "fake-mongodbImage", "fake-agentImage", "fake-versionUpgradeHookImage", "fake-readinessProbeImage", true)
4940
sts := &appsv1.StatefulSet{}
5041

5142
t.Run("1st Call", func(t *testing.T) {
@@ -63,103 +54,20 @@ func TestMultipleCalls_DoNotCauseSideEffects(t *testing.T) {
6354
}
6455

6556
func TestManagedSecurityContext(t *testing.T) {
66-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
67-
t.Setenv(MongodbImageEnv, "mongodb-community-server")
68-
t.Setenv(AgentImageEnv, "agent-image")
6957
t.Setenv(podtemplatespec.ManagedSecurityContextEnv, "true")
7058

7159
mdb := newTestReplicaSet()
72-
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, os.Getenv(AgentImageEnv), true)
60+
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, "fake-mongodbImage", "fake-agentImage", "fake-versionUpgradeHookImage", "fake-readinessProbeImage", true)
7361

7462
sts := &appsv1.StatefulSet{}
7563
stsFunc(sts)
7664

7765
assertStatefulSetIsBuiltCorrectly(t, mdb, sts)
7866
}
7967

80-
func TestGetMongoDBImage(t *testing.T) {
81-
type testConfig struct {
82-
setArgs func(t *testing.T)
83-
version string
84-
expectedImage string
85-
}
86-
tests := map[string]testConfig{
87-
"Default UBI8 Community image": {
88-
setArgs: func(t *testing.T) {
89-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
90-
t.Setenv(MongodbImageEnv, "mongodb-community-server")
91-
},
92-
version: "6.0.5",
93-
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubi8",
94-
},
95-
"Overridden UBI8 Enterprise image": {
96-
setArgs: func(t *testing.T) {
97-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
98-
t.Setenv(MongodbImageEnv, "mongodb-enterprise-server")
99-
},
100-
version: "6.0.5",
101-
expectedImage: "docker.io/mongodb/mongodb-enterprise-server:6.0.5-ubi8",
102-
},
103-
"Overridden UBI8 Enterprise image from Quay": {
104-
setArgs: func(t *testing.T) {
105-
t.Setenv(MongodbRepoUrl, "quay.io/mongodb")
106-
t.Setenv(MongodbImageEnv, "mongodb-enterprise-server")
107-
},
108-
version: "6.0.5",
109-
expectedImage: "quay.io/mongodb/mongodb-enterprise-server:6.0.5-ubi8",
110-
},
111-
"Overridden Ubuntu Community image": {
112-
setArgs: func(t *testing.T) {
113-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
114-
t.Setenv(MongodbImageEnv, "mongodb-community-server")
115-
t.Setenv(MongoDBImageType, "ubuntu2204")
116-
},
117-
version: "6.0.5",
118-
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubuntu2204",
119-
},
120-
"Overridden UBI Community image": {
121-
setArgs: func(t *testing.T) {
122-
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
123-
t.Setenv(MongodbImageEnv, "mongodb-community-server")
124-
t.Setenv(MongoDBImageType, "ubi8")
125-
},
126-
version: "6.0.5",
127-
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubi8",
128-
},
129-
"Docker Inc images": {
130-
setArgs: func(t *testing.T) {
131-
t.Setenv(MongodbRepoUrl, "docker.io")
132-
t.Setenv(MongodbImageEnv, "mongo")
133-
},
134-
version: "6.0.5",
135-
expectedImage: "docker.io/mongo:6.0.5",
136-
},
137-
"Deprecated AppDB images defined the old way": {
138-
setArgs: func(t *testing.T) {
139-
t.Setenv(MongodbRepoUrl, "quay.io")
140-
t.Setenv(MongodbImageEnv, "mongodb/mongodb-enterprise-appdb-database-ubi")
141-
// In this example, we intentionally don't use the suffix from the env. variable and let users
142-
// define it in the version instead. There are some known customers who do this.
143-
// This is a backwards compatibility case.
144-
t.Setenv(MongoDBImageType, "will-be-ignored")
145-
},
146-
147-
version: "5.0.14-ent",
148-
expectedImage: "quay.io/mongodb/mongodb-enterprise-appdb-database-ubi:5.0.14-ent",
149-
},
150-
}
151-
for testName := range tests {
152-
t.Run(testName, func(t *testing.T) {
153-
testConfig := tests[testName]
154-
testConfig.setArgs(t)
155-
image := getMongoDBImage(testConfig.version)
156-
assert.Equal(t, testConfig.expectedImage, image)
157-
})
158-
}
159-
}
160-
16168
func TestMongod_Container(t *testing.T) {
162-
c := container.New(mongodbContainer("4.2", []corev1.VolumeMount{}, mdbv1.NewMongodConfiguration()))
69+
const mongodbImageMock = "fake-mongodbImage"
70+
c := container.New(mongodbContainer(mongodbImageMock, []corev1.VolumeMount{}, mdbv1.NewMongodConfiguration()))
16371

16472
t.Run("Has correct Env vars", func(t *testing.T) {
16573
assert.Len(t, c.Env, 1)
@@ -168,7 +76,7 @@ func TestMongod_Container(t *testing.T) {
16876
})
16977

17078
t.Run("Image is correct", func(t *testing.T) {
171-
assert.Equal(t, getMongoDBImage("4.2"), c.Image)
79+
assert.Equal(t, mongodbImageMock, c.Image)
17280
})
17381

17482
t.Run("Resource requirements are correct", func(t *testing.T) {
@@ -210,7 +118,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDBCommunity,
210118
}
211119

212120
agentContainer := sts.Spec.Template.Spec.Containers[0]
213-
assert.Equal(t, "agent-image", agentContainer.Image)
121+
assert.Equal(t, "fake-agentImage", agentContainer.Image)
214122
probe := agentContainer.ReadinessProbe
215123
assert.True(t, reflect.DeepEqual(probes.New(DefaultReadiness()), *probe))
216124
assert.Equal(t, probes.New(DefaultReadiness()).FailureThreshold, probe.FailureThreshold)
@@ -231,7 +139,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDBCommunity,
231139
assertContainsVolumeMountWithName(t, agentContainer.VolumeMounts, "my-rs-keyfile")
232140

233141
mongodContainer := sts.Spec.Template.Spec.Containers[1]
234-
assert.Equal(t, "docker.io/mongodb/mongodb-community-server:6.0.5-ubi8", mongodContainer.Image)
142+
assert.Equal(t, "fake-mongodbImage", mongodContainer.Image)
235143
assert.Len(t, mongodContainer.VolumeMounts, 6)
236144
if !managedSecurityContext {
237145
assert.NotNil(t, sts.Spec.Template.Spec.Containers[1].SecurityContext)
@@ -248,7 +156,7 @@ func assertStatefulSetIsBuiltCorrectly(t *testing.T, mdb mdbv1.MongoDBCommunity,
248156

249157
initContainer := sts.Spec.Template.Spec.InitContainers[0]
250158
assert.Equal(t, versionUpgradeHookName, initContainer.Name)
251-
assert.Equal(t, "version-upgrade-hook-image", initContainer.Image)
159+
assert.Equal(t, "fake-versionUpgradeHookImage", initContainer.Image)
252160
assert.Len(t, initContainer.VolumeMounts, 1)
253161
if !managedSecurityContext {
254162
assert.NotNil(t, sts.Spec.Template.Spec.InitContainers[0].SecurityContext)

‎controllers/construct/mongodbstatefulset.go‎

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ package construct
22

33
import (
44
"fmt"
5-
"github.com/mongodb/mongodb-kubernetes-operator/pkg/readiness/config"
65
"os"
76
"strconv"
8-
"strings"
97

10-
"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/envvar"
8+
"github.com/mongodb/mongodb-kubernetes-operator/pkg/readiness/config"
119

1210
"github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig"
1311
"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/container"
@@ -28,6 +26,16 @@ var (
2826
OfficialMongodbRepoUrls = []string{"docker.io/mongodb", "quay.io/mongodb"}
2927
)
3028

29+
// Environment variables used to configure the MongoDB StatefulSet.
30+
const (
31+
MongodbRepoUrlEnv = "MONGODB_REPO_URL"
32+
MongodbImageEnv = "MONGODB_IMAGE"
33+
MongoDBImageTypeEnv = "MDB_IMAGE_TYPE"
34+
AgentImageEnv = "AGENT_IMAGE"
35+
VersionUpgradeHookImageEnv = "VERSION_UPGRADE_HOOK_IMAGE"
36+
ReadinessProbeImageEnv = "READINESS_PROBE_IMAGE"
37+
)
38+
3139
const (
3240
AgentName = "mongodb-agent"
3341
MongodbName = "mongod"
@@ -42,18 +50,12 @@ const (
4250
mongodbDatabaseServiceAccountName = "mongodb-database"
4351
agentHealthStatusFilePathValue = "/var/log/mongodb-mms-automation/healthstatus/agent-health-status.json"
4452

45-
MongodbRepoUrl = "MONGODB_REPO_URL"
4653
OfficialMongodbEnterpriseServerImageName = "mongodb-enterprise-server"
4754

4855
headlessAgentEnv = "HEADLESS_AGENT"
4956
podNamespaceEnv = "POD_NAMESPACE"
5057
automationConfigEnv = "AUTOMATION_CONFIG_MAP"
51-
AgentImageEnv = "AGENT_IMAGE"
52-
MongodbImageEnv = "MONGODB_IMAGE"
53-
MongoDBImageType = "MDB_IMAGE_TYPE"
5458
MongoDBAssumeEnterpriseEnv = "MDB_ASSUME_ENTERPRISE"
55-
VersionUpgradeHookImageEnv = "VERSION_UPGRADE_HOOK_IMAGE"
56-
ReadinessProbeImageEnv = "READINESS_PROBE_IMAGE"
5759

5860
automationMongodConfFileName = "automation-mongod.conf"
5961
keyfileFilePath = "/var/lib/mongodb-mms-automation/authentication/keyfile"
@@ -123,7 +125,7 @@ type MongoDBStatefulSetOwner interface {
123125
// BuildMongoDBReplicaSetStatefulSetModificationFunction builds the parts of the replica set that are common between every resource that implements
124126
// MongoDBStatefulSetOwner.
125127
// It doesn't configure TLS or additional containers/env vars that the statefulset might need.
126-
func BuildMongoDBReplicaSetStatefulSetModificationFunction(mdb MongoDBStatefulSetOwner, scaler scale.ReplicaSetScaler, agentImage string, withInitContainers bool) statefulset.Modification {
128+
func BuildMongoDBReplicaSetStatefulSetModificationFunction(mdb MongoDBStatefulSetOwner, scaler scale.ReplicaSetScaler, mongodbImage, agentImage, versionUpgradeHookImage, readinessProbeImage string, withInitContainers bool) statefulset.Modification {
127129
labels := map[string]string{
128130
"app": mdb.ServiceName(),
129131
}
@@ -174,8 +176,8 @@ func BuildMongoDBReplicaSetStatefulSetModificationFunction(mdb MongoDBStatefulSe
174176
scriptsVolume = statefulset.CreateVolumeFromEmptyDir("agent-scripts")
175177
scriptsVolumeMount := statefulset.CreateVolumeMount(scriptsVolume.Name, "/opt/scripts", statefulset.WithReadOnly(false))
176178

177-
upgradeInitContainer = podtemplatespec.WithInitContainer(versionUpgradeHookName, versionUpgradeHookInit([]corev1.VolumeMount{hooksVolumeMount}))
178-
readinessInitContainer = podtemplatespec.WithInitContainer(ReadinessProbeContainerName, readinessProbeInit([]corev1.VolumeMount{scriptsVolumeMount}))
179+
upgradeInitContainer = podtemplatespec.WithInitContainer(versionUpgradeHookName, versionUpgradeHookInit([]corev1.VolumeMount{hooksVolumeMount}, versionUpgradeHookImage))
180+
readinessInitContainer = podtemplatespec.WithInitContainer(ReadinessProbeContainerName, readinessProbeInit([]corev1.VolumeMount{scriptsVolumeMount}, readinessProbeImage))
179181
scriptsVolumeMod = podtemplatespec.WithVolume(scriptsVolume)
180182
hooksVolumeMod = podtemplatespec.WithVolume(hooksVolume)
181183

@@ -243,7 +245,7 @@ func BuildMongoDBReplicaSetStatefulSetModificationFunction(mdb MongoDBStatefulSe
243245
podtemplatespec.WithVolume(keyFileVolume),
244246
podtemplatespec.WithServiceAccount(mongodbDatabaseServiceAccountName),
245247
podtemplatespec.WithContainer(AgentName, mongodbAgentContainer(mdb.AutomationConfigSecretName(), mongodbAgentVolumeMounts, agentLogLevel, agentLogFile, agentMaxLogFileDurationHours, agentImage)),
246-
podtemplatespec.WithContainer(MongodbName, mongodbContainer(mdb.GetMongoDBVersion(nil), mongodVolumeMounts, mdb.GetMongodConfiguration())),
248+
podtemplatespec.WithContainer(MongodbName, mongodbContainer(mongodbImage, mongodVolumeMounts, mdb.GetMongodConfiguration())),
247249
upgradeInitContainer,
248250
readinessInitContainer,
249251
),
@@ -312,12 +314,12 @@ func mongodbAgentContainer(automationConfigSecretName string, volumeMounts []cor
312314
)
313315
}
314316

315-
func versionUpgradeHookInit(volumeMount []corev1.VolumeMount) container.Modification {
317+
func versionUpgradeHookInit(volumeMount []corev1.VolumeMount, versionUpgradeHookImage string) container.Modification {
316318
_, containerSecurityContext := podtemplatespec.WithDefaultSecurityContextsModifications()
317319
return container.Apply(
318320
container.WithName(versionUpgradeHookName),
319321
container.WithCommand([]string{"cp", "version-upgrade-hook", "/hooks/version-upgrade"}),
320-
container.WithImage(os.Getenv(VersionUpgradeHookImageEnv)),
322+
container.WithImage(versionUpgradeHookImage),
321323
container.WithResourceRequirements(resourcerequirements.Defaults()),
322324
container.WithImagePullPolicy(corev1.PullAlways),
323325
container.WithVolumeMounts(volumeMount),
@@ -351,38 +353,20 @@ func logsPvc(logsVolumeName string) persistentvolumeclaim.Modification {
351353

352354
// readinessProbeInit returns a modification function which will add the readiness probe container.
353355
// this container will copy the readiness probe binary into the /opt/scripts directory.
354-
func readinessProbeInit(volumeMount []corev1.VolumeMount) container.Modification {
356+
func readinessProbeInit(volumeMount []corev1.VolumeMount, readinessProbeImage string) container.Modification {
355357
_, containerSecurityContext := podtemplatespec.WithDefaultSecurityContextsModifications()
356358
return container.Apply(
357359
container.WithName(ReadinessProbeContainerName),
358360
container.WithCommand([]string{"cp", "/probes/readinessprobe", "/opt/scripts/readinessprobe"}),
359-
container.WithImage(os.Getenv(ReadinessProbeImageEnv)),
361+
container.WithImage(readinessProbeImage),
360362
container.WithImagePullPolicy(corev1.PullAlways),
361363
container.WithVolumeMounts(volumeMount),
362364
container.WithResourceRequirements(resourcerequirements.Defaults()),
363365
containerSecurityContext,
364366
)
365367
}
366368

367-
func getMongoDBImage(version string) string {
368-
repoUrl := os.Getenv(MongodbRepoUrl)
369-
imageType := envvar.GetEnvOrDefault(MongoDBImageType, DefaultImageType)
370-
371-
if strings.HasSuffix(repoUrl, "/") {
372-
repoUrl = strings.TrimRight(repoUrl, "/")
373-
}
374-
mongoImageName := os.Getenv(MongodbImageEnv)
375-
for _, officialUrl := range OfficialMongodbRepoUrls {
376-
if repoUrl == officialUrl {
377-
return fmt.Sprintf("%s/%s:%s-%s", repoUrl, mongoImageName, version, imageType)
378-
}
379-
}
380-
381-
// This is the old images backwards compatibility code path.
382-
return fmt.Sprintf("%s/%s:%s", repoUrl, mongoImageName, version)
383-
}
384-
385-
func mongodbContainer(version string, volumeMounts []corev1.VolumeMount, additionalMongoDBConfig mdbv1.MongodConfiguration) container.Modification {
369+
func mongodbContainer(mongodbImage string, volumeMounts []corev1.VolumeMount, additionalMongoDBConfig mdbv1.MongodConfiguration) container.Modification {
386370
filePath := additionalMongoDBConfig.GetDBDataDir() + "/" + automationMongodConfFileName
387371
mongoDbCommand := fmt.Sprintf(`
388372
if [ -e "/hooks/version-upgrade" ]; then
@@ -408,7 +392,7 @@ exec mongod -f %s;
408392

409393
return container.Apply(
410394
container.WithName(MongodbName),
411-
container.WithImage(getMongoDBImage(version)),
395+
container.WithImage(mongodbImage),
412396
container.WithResourceRequirements(resourcerequirements.Defaults()),
413397
container.WithCommand(containerCommand),
414398
// The official image provides both CMD and ENTRYPOINT. We're reusing the former and need to replace

‎controllers/mongodb_cleanup_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package controllers
22

33
import (
44
"context"
5+
"testing"
6+
57
mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
68
kubeClient "github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/client"
79
"github.com/stretchr/testify/assert"
810
corev1 "k8s.io/api/core/v1"
911
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10-
"testing"
1112
)
1213

1314
func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) {
@@ -140,7 +141,7 @@ func TestReplicaSetReconcilerCleanupPemSecret(t *testing.T) {
140141
err := createAgentCertPemSecret(ctx, client, mdb, "CERT", "KEY", "")
141142
assert.NoError(t, err)
142143

143-
r := NewReconciler(mgr)
144+
r := NewReconciler(mgr, "fake-mongodbRepoUrl", "fake-mongodbImage", "ubi8", "fake-agentImage", "fake-versionUpgradeHookImage", "fake-readinessProbeImage")
144145

145146
secret, err := r.client.GetSecret(ctx, mdb.AgentCertificatePemSecretNamespacedName())
146147
assert.NoError(t, err)

0 commit comments

Comments
 (0)