Skip to content

CLOUDP-302068: Refactoring: propagate env vars via parameters #1676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 10, 2025
Merged

CLOUDP-302068: Refactoring: propagate env vars via parameters #1676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package main
import (
"fmt"
"os"

"sigs.k8s.io/controller-runtime/pkg/cache"

mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
"github.com/mongodb/mongodb-kubernetes-operator/controllers"
"github.com/mongodb/mongodb-kubernetes-operator/controllers/construct"
"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/envvar"
"go.uber.org/zap"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -56,7 +58,14 @@ func main() {
log.Sugar().Fatalf("Failed to configure logger: %v", err)
}

if !hasRequiredVariables(log, construct.AgentImageEnv, construct.VersionUpgradeHookImageEnv, construct.ReadinessProbeImageEnv) {
if !hasRequiredVariables(
log,
construct.MongodbRepoUrlEnv,
construct.MongodbImageEnv,
Comment on lines +63 to +64
Copy link
Collaborator Author

@m1kola m1kola Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the most significant change in this PR: rest is moving things around and updating tests where applicable.

These env vars were not required before, but from the code it seems like they actually are required. So I'm adding them here. Would be good to get a sanity check here. Am I missing something? Is it actually possible and reasonable to run without them?

construct.AgentImageEnv,
construct.VersionUpgradeHookImageEnv,
construct.ReadinessProbeImageEnv,
) {
os.Exit(1)
}

Expand Down Expand Up @@ -99,7 +108,15 @@ func main() {
}

// Setup Controller.
if err = controllers.NewReconciler(mgr).SetupWithManager(mgr); err != nil {
if err = controllers.NewReconciler(
mgr,
os.Getenv(construct.MongodbRepoUrlEnv),
os.Getenv(construct.MongodbImageEnv),
envvar.GetEnvOrDefault(construct.MongoDBImageTypeEnv, construct.DefaultImageType),
os.Getenv(construct.AgentImageEnv),
os.Getenv(construct.VersionUpgradeHookImageEnv),
os.Getenv(construct.ReadinessProbeImageEnv),
).SetupWithManager(mgr); err != nil {
log.Sugar().Fatalf("Unable to create controller: %v", err)
}
// +kubebuilder:scaffold:builder
Expand Down
108 changes: 8 additions & 100 deletions controllers/construct/build_statefulset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package construct

import (
"os"
"reflect"
"testing"

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

func init() {
os.Setenv(VersionUpgradeHookImageEnv, "version-upgrade-hook-image")
}

func newTestReplicaSet() mdbv1.MongoDBCommunity {
return mdbv1.MongoDBCommunity{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -40,12 +35,8 @@ func newTestReplicaSet() mdbv1.MongoDBCommunity {
}

func TestMultipleCalls_DoNotCauseSideEffects(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-community-server")
t.Setenv(AgentImageEnv, "agent-image")

mdb := newTestReplicaSet()
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, os.Getenv(AgentImageEnv), true)
stsFunc := BuildMongoDBReplicaSetStatefulSetModificationFunction(&mdb, &mdb, "fake-mongodbImage", "fake-agentImage", "fake-versionUpgradeHookImage", "fake-readinessProbeImage", true)
sts := &appsv1.StatefulSet{}

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

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

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

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

assertStatefulSetIsBuiltCorrectly(t, mdb, sts)
}

func TestGetMongoDBImage(t *testing.T) {
type testConfig struct {
setArgs func(t *testing.T)
version string
expectedImage string
}
tests := map[string]testConfig{
"Default UBI8 Community image": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-community-server")
},
version: "6.0.5",
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubi8",
},
"Overridden UBI8 Enterprise image": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-enterprise-server")
},
version: "6.0.5",
expectedImage: "docker.io/mongodb/mongodb-enterprise-server:6.0.5-ubi8",
},
"Overridden UBI8 Enterprise image from Quay": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "quay.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-enterprise-server")
},
version: "6.0.5",
expectedImage: "quay.io/mongodb/mongodb-enterprise-server:6.0.5-ubi8",
},
"Overridden Ubuntu Community image": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-community-server")
t.Setenv(MongoDBImageType, "ubuntu2204")
},
version: "6.0.5",
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubuntu2204",
},
"Overridden UBI Community image": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io/mongodb")
t.Setenv(MongodbImageEnv, "mongodb-community-server")
t.Setenv(MongoDBImageType, "ubi8")
},
version: "6.0.5",
expectedImage: "docker.io/mongodb/mongodb-community-server:6.0.5-ubi8",
},
"Docker Inc images": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "docker.io")
t.Setenv(MongodbImageEnv, "mongo")
},
version: "6.0.5",
expectedImage: "docker.io/mongo:6.0.5",
},
"Deprecated AppDB images defined the old way": {
setArgs: func(t *testing.T) {
t.Setenv(MongodbRepoUrl, "quay.io")
t.Setenv(MongodbImageEnv, "mongodb/mongodb-enterprise-appdb-database-ubi")
// In this example, we intentionally don't use the suffix from the env. variable and let users
// define it in the version instead. There are some known customers who do this.
// This is a backwards compatibility case.
t.Setenv(MongoDBImageType, "will-be-ignored")
},

version: "5.0.14-ent",
expectedImage: "quay.io/mongodb/mongodb-enterprise-appdb-database-ubi:5.0.14-ent",
},
}
for testName := range tests {
t.Run(testName, func(t *testing.T) {
testConfig := tests[testName]
testConfig.setArgs(t)
image := getMongoDBImage(testConfig.version)
assert.Equal(t, testConfig.expectedImage, image)
})
}
}

func TestMongod_Container(t *testing.T) {
c := container.New(mongodbContainer("4.2", []corev1.VolumeMount{}, mdbv1.NewMongodConfiguration()))
const mongodbImageMock = "fake-mongodbImage"
c := container.New(mongodbContainer(mongodbImageMock, []corev1.VolumeMount{}, mdbv1.NewMongodConfiguration()))

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

t.Run("Image is correct", func(t *testing.T) {
assert.Equal(t, getMongoDBImage("4.2"), c.Image)
assert.Equal(t, mongodbImageMock, c.Image)
})

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

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

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

initContainer := sts.Spec.Template.Spec.InitContainers[0]
assert.Equal(t, versionUpgradeHookName, initContainer.Name)
assert.Equal(t, "version-upgrade-hook-image", initContainer.Image)
assert.Equal(t, "fake-versionUpgradeHookImage", initContainer.Image)
assert.Len(t, initContainer.VolumeMounts, 1)
if !managedSecurityContext {
assert.NotNil(t, sts.Spec.Template.Spec.InitContainers[0].SecurityContext)
Expand Down
58 changes: 21 additions & 37 deletions controllers/construct/mongodbstatefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ package construct

import (
"fmt"
"github.com/mongodb/mongodb-kubernetes-operator/pkg/readiness/config"
"os"
"strconv"
"strings"

"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/envvar"
"github.com/mongodb/mongodb-kubernetes-operator/pkg/readiness/config"

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

// Environment variables used to configure the MongoDB StatefulSet.
const (
MongodbRepoUrlEnv = "MONGODB_REPO_URL"
MongodbImageEnv = "MONGODB_IMAGE"
MongoDBImageTypeEnv = "MDB_IMAGE_TYPE"
AgentImageEnv = "AGENT_IMAGE"
VersionUpgradeHookImageEnv = "VERSION_UPGRADE_HOOK_IMAGE"
ReadinessProbeImageEnv = "READINESS_PROBE_IMAGE"
)

const (
AgentName = "mongodb-agent"
MongodbName = "mongod"
Expand All @@ -42,18 +50,12 @@ const (
mongodbDatabaseServiceAccountName = "mongodb-database"
agentHealthStatusFilePathValue = "/var/log/mongodb-mms-automation/healthstatus/agent-health-status.json"

MongodbRepoUrl = "MONGODB_REPO_URL"
OfficialMongodbEnterpriseServerImageName = "mongodb-enterprise-server"

headlessAgentEnv = "HEADLESS_AGENT"
podNamespaceEnv = "POD_NAMESPACE"
automationConfigEnv = "AUTOMATION_CONFIG_MAP"
AgentImageEnv = "AGENT_IMAGE"
MongodbImageEnv = "MONGODB_IMAGE"
MongoDBImageType = "MDB_IMAGE_TYPE"
MongoDBAssumeEnterpriseEnv = "MDB_ASSUME_ENTERPRISE"
VersionUpgradeHookImageEnv = "VERSION_UPGRADE_HOOK_IMAGE"
ReadinessProbeImageEnv = "READINESS_PROBE_IMAGE"

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

upgradeInitContainer = podtemplatespec.WithInitContainer(versionUpgradeHookName, versionUpgradeHookInit([]corev1.VolumeMount{hooksVolumeMount}))
readinessInitContainer = podtemplatespec.WithInitContainer(ReadinessProbeContainerName, readinessProbeInit([]corev1.VolumeMount{scriptsVolumeMount}))
upgradeInitContainer = podtemplatespec.WithInitContainer(versionUpgradeHookName, versionUpgradeHookInit([]corev1.VolumeMount{hooksVolumeMount}, versionUpgradeHookImage))
readinessInitContainer = podtemplatespec.WithInitContainer(ReadinessProbeContainerName, readinessProbeInit([]corev1.VolumeMount{scriptsVolumeMount}, readinessProbeImage))
scriptsVolumeMod = podtemplatespec.WithVolume(scriptsVolume)
hooksVolumeMod = podtemplatespec.WithVolume(hooksVolume)

Expand Down Expand Up @@ -243,7 +245,7 @@ func BuildMongoDBReplicaSetStatefulSetModificationFunction(mdb MongoDBStatefulSe
podtemplatespec.WithVolume(keyFileVolume),
podtemplatespec.WithServiceAccount(mongodbDatabaseServiceAccountName),
podtemplatespec.WithContainer(AgentName, mongodbAgentContainer(mdb.AutomationConfigSecretName(), mongodbAgentVolumeMounts, agentLogLevel, agentLogFile, agentMaxLogFileDurationHours, agentImage)),
podtemplatespec.WithContainer(MongodbName, mongodbContainer(mdb.GetMongoDBVersion(nil), mongodVolumeMounts, mdb.GetMongodConfiguration())),
podtemplatespec.WithContainer(MongodbName, mongodbContainer(mongodbImage, mongodVolumeMounts, mdb.GetMongodConfiguration())),
upgradeInitContainer,
readinessInitContainer,
),
Expand Down Expand Up @@ -312,12 +314,12 @@ func mongodbAgentContainer(automationConfigSecretName string, volumeMounts []cor
)
}

func versionUpgradeHookInit(volumeMount []corev1.VolumeMount) container.Modification {
func versionUpgradeHookInit(volumeMount []corev1.VolumeMount, versionUpgradeHookImage string) container.Modification {
_, containerSecurityContext := podtemplatespec.WithDefaultSecurityContextsModifications()
return container.Apply(
container.WithName(versionUpgradeHookName),
container.WithCommand([]string{"cp", "version-upgrade-hook", "/hooks/version-upgrade"}),
container.WithImage(os.Getenv(VersionUpgradeHookImageEnv)),
container.WithImage(versionUpgradeHookImage),
container.WithResourceRequirements(resourcerequirements.Defaults()),
container.WithImagePullPolicy(corev1.PullAlways),
container.WithVolumeMounts(volumeMount),
Expand Down Expand Up @@ -351,38 +353,20 @@ func logsPvc(logsVolumeName string) persistentvolumeclaim.Modification {

// readinessProbeInit returns a modification function which will add the readiness probe container.
// this container will copy the readiness probe binary into the /opt/scripts directory.
func readinessProbeInit(volumeMount []corev1.VolumeMount) container.Modification {
func readinessProbeInit(volumeMount []corev1.VolumeMount, readinessProbeImage string) container.Modification {
_, containerSecurityContext := podtemplatespec.WithDefaultSecurityContextsModifications()
return container.Apply(
container.WithName(ReadinessProbeContainerName),
container.WithCommand([]string{"cp", "/probes/readinessprobe", "/opt/scripts/readinessprobe"}),
container.WithImage(os.Getenv(ReadinessProbeImageEnv)),
container.WithImage(readinessProbeImage),
container.WithImagePullPolicy(corev1.PullAlways),
container.WithVolumeMounts(volumeMount),
container.WithResourceRequirements(resourcerequirements.Defaults()),
containerSecurityContext,
)
}

func getMongoDBImage(version string) string {
repoUrl := os.Getenv(MongodbRepoUrl)
imageType := envvar.GetEnvOrDefault(MongoDBImageType, DefaultImageType)

if strings.HasSuffix(repoUrl, "/") {
repoUrl = strings.TrimRight(repoUrl, "/")
}
mongoImageName := os.Getenv(MongodbImageEnv)
for _, officialUrl := range OfficialMongodbRepoUrls {
if repoUrl == officialUrl {
return fmt.Sprintf("%s/%s:%s-%s", repoUrl, mongoImageName, version, imageType)
}
}

// This is the old images backwards compatibility code path.
return fmt.Sprintf("%s/%s:%s", repoUrl, mongoImageName, version)
}

func mongodbContainer(version string, volumeMounts []corev1.VolumeMount, additionalMongoDBConfig mdbv1.MongodConfiguration) container.Modification {
func mongodbContainer(mongodbImage string, volumeMounts []corev1.VolumeMount, additionalMongoDBConfig mdbv1.MongodConfiguration) container.Modification {
filePath := additionalMongoDBConfig.GetDBDataDir() + "/" + automationMongodConfFileName
mongoDbCommand := fmt.Sprintf(`
if [ -e "/hooks/version-upgrade" ]; then
Expand All @@ -408,7 +392,7 @@ exec mongod -f %s;

return container.Apply(
container.WithName(MongodbName),
container.WithImage(getMongoDBImage(version)),
container.WithImage(mongodbImage),
container.WithResourceRequirements(resourcerequirements.Defaults()),
container.WithCommand(containerCommand),
// The official image provides both CMD and ENTRYPOINT. We're reusing the former and need to replace
Expand Down
5 changes: 3 additions & 2 deletions controllers/mongodb_cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ package controllers

import (
"context"
"testing"

mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1"
kubeClient "github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/client"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"testing"
)

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

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

secret, err := r.client.GetSecret(ctx, mdb.AgentCertificatePemSecretNamespacedName())
assert.NoError(t, err)
Expand Down
Loading
Loading