Skip to content

Commit e403261

Browse files
m1kolafealebenpae
m1kola
authored and
fealebenpae
committed
CLOUDP-302068: Refactor reading image environment variables (#4128)
Moves reading image environment variables into a single place: `ReadImagesFromEnvironment` is responsible for that. We call it in `main.go` and pass the result (`imagesMap`) into the controllers. We have to pass the whole `imagesMap` instead of selected images to the controllers because controllers need to be able to read certain environment variables from their `RELATED_IMAGE_` counterparts, and image versions are determined at reconciliation time. For example, `MONGODB_IMAGE` for version `5.0.18` could be read from `RELATED_IMAGE_MONGODB_IMAGE_5_0_18_ubi8`. The general idea is that we only pass the whole `imagesMap` into controllers and their immediate helpers such as `ShardedClusterReconcileHelper`. Further down the call hierarchy, we want to pass only specific images. This helps with unit testing as we can just provide a required mock/fake value instead of heavily relying on `mock.InitDefaultEnvVariables()` in smaller units. StatefulSet constructors (`./controllers/operator/construct`) are now free of reading images: we pass specific images from the controllers into the constructors via options. For example, `OpsManagerStatefulSetOptions` now has `InitOpsManagerImage` and `OpsManagerImage` options. There are still `FIXME(Mikalai)` in this commit. These will be addressed in the next iteration where we will push more environment variables to `main.go`.
1 parent 34a7e23 commit e403261

26 files changed

+438
-332
lines changed

‎controllers/operator/appdbreplicaset_controller.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package operator
33
import (
44
"context"
55
"fmt"
6-
"os"
76
"path"
87
"sort"
98
"strconv"
@@ -32,7 +31,7 @@ import (
3231
"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/scale"
3332

3433
mdbcv1_controllers "github.com/mongodb/mongodb-kubernetes-operator/controllers"
35-
mdbconstruct "github.com/mongodb/mongodb-kubernetes-operator/controllers/construct"
34+
mcoConstruct "github.com/mongodb/mongodb-kubernetes-operator/controllers/construct"
3635
kubernetesClient "github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/client"
3736
appsv1 "k8s.io/api/apps/v1"
3837
corev1 "k8s.io/api/core/v1"
@@ -117,13 +116,16 @@ type ReconcileAppDbReplicaSet struct {
117116
memberClusters []multicluster.MemberCluster
118117
stateStore *StateStore[AppDBDeploymentState]
119118
deploymentState *AppDBDeploymentState
119+
120+
imageUrls construct.ImageUrls
120121
}
121122

122-
func NewAppDBReplicaSetReconciler(ctx context.Context, appDBSpec omv1.AppDBSpec, commonController *ReconcileCommonController, omConnectionFactory om.ConnectionFactory, omAnnotations map[string]string, globalMemberClustersMap map[string]client.Client, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
123+
func NewAppDBReplicaSetReconciler(ctx context.Context, imageUrls construct.ImageUrls, appDBSpec omv1.AppDBSpec, commonController *ReconcileCommonController, omConnectionFactory om.ConnectionFactory, omAnnotations map[string]string, globalMemberClustersMap map[string]client.Client, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
123124
reconciler := &ReconcileAppDbReplicaSet{
124125
ReconcileCommonController: commonController,
125126
omConnectionFactory: omConnectionFactory,
126127
centralClient: commonController.client,
128+
imageUrls: imageUrls,
127129
}
128130

129131
if err := reconciler.initializeStateStore(ctx, appDBSpec, omAnnotations, log); err != nil {
@@ -551,30 +553,40 @@ func (r *ReconcileAppDbReplicaSet) ReconcileAppDB(ctx context.Context, opsManage
551553
}
552554
}
553555

554-
appdbOpts := construct.AppDBStatefulSetOptions{}
556+
// FIXME(Mikalai): move env var read up the call stack
557+
version := env.ReadOrDefault(construct.InitAppdbVersionEnv, "latest")
558+
559+
appdbOpts := construct.AppDBStatefulSetOptions{
560+
InitAppDBImage: construct.ContainerImage(r.imageUrls, util.InitAppdbImageUrlEnv, version),
561+
MongodbImage: construct.GetOfficialImage(r.imageUrls, opsManager.Spec.AppDB.Version, opsManager.GetAnnotations()),
562+
}
555563
if architectures.IsRunningStaticArchitecture(opsManager.Annotations) {
556564
if !rs.PodSpec.IsAgentImageOverridden() {
557565
// Because OM is not available when starting AppDB, we read the version from the mapping
558566
// We plan to change this in the future, but for the sake of simplicity we leave it that way for the moment
559567
// It avoids unnecessary reconciles, race conditions...
560-
appdbOpts.AgentVersion, err = r.getAgentVersion(nil, opsManager.Spec.Version, true, log)
568+
agentVersion, err := r.getAgentVersion(nil, opsManager.Spec.Version, true, log)
561569
if err != nil {
562570
log.Errorf("Impossible to get agent version, please override the agent image by providing a pod template")
563571
return r.updateStatus(ctx, opsManager, workflow.Failed(xerrors.Errorf("Failed to get agent version: %w. Please use spec.statefulSet to supply proper Agent version", err)), log)
564572
}
573+
574+
appdbOpts.AgentImage = construct.ContainerImage(r.imageUrls, architectures.MdbAgentImageRepo, agentVersion)
565575
}
566576
} else {
567577
// instead of using a hard-coded monitoring version, we use the "newest" one based on the release.json.
568578
// This ensures we need to care less about CVEs compared to the prior older hardcoded versions.
569-
appdbOpts.LegacyMonitoringAgent, err = r.getAgentVersion(nil, opsManager.Spec.Version, true, log)
579+
legacyMonitoringAgentVersion, err := r.getAgentVersion(nil, opsManager.Spec.Version, true, log)
580+
if err != nil {
581+
return r.updateStatus(ctx, opsManager, workflow.Failed(xerrors.Errorf("Error reading monitoring agent version: %w", err)), log, appDbStatusOption)
582+
}
583+
584+
appdbOpts.LegacyMonitoringAgentImage = construct.ContainerImage(r.imageUrls, mcoConstruct.AgentImageEnv, legacyMonitoringAgentVersion)
570585

571586
// AgentImageEnv contains the full container image uri e.g. quay.io/mongodb/mongodb-agent-ubi:107.0.0.8502-1
572587
// In non-static containers we don't ask OM for the correct version, therefore we just rely on the provided
573588
// environment variable.
574-
appdbOpts.AgentVersion = os.Getenv(mdbconstruct.AgentImageEnv)
575-
if err != nil {
576-
return r.updateStatus(ctx, opsManager, workflow.Failed(xerrors.Errorf("Error reading monitoring agent version: %w", err)), log, appDbStatusOption)
577-
}
589+
appdbOpts.AgentImage = r.imageUrls[mcoConstruct.AgentImageEnv]
578590
}
579591

580592
workflowStatus := r.ensureTLSSecretAndCreatePEMIfNeeded(ctx, opsManager, log)

‎controllers/operator/appdbreplicaset_controller_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -757,9 +757,6 @@ func buildAutomationConfigForAppDb(ctx context.Context, builder *omv1.OpsManager
757757
if err != nil {
758758
return automationconfig.AutomationConfig{}, err
759759
}
760-
if err != nil {
761-
return automationconfig.AutomationConfig{}, err
762-
}
763760
return reconciler.buildAppDbAutomationConfig(ctx, opsManager, acType, UnusedPrometheusConfiguration, multicluster.LegacyCentralClusterName, zap.S())
764761
}
765762

@@ -776,13 +773,13 @@ func checkDeploymentEqualToPublished(t *testing.T, expected automationconfig.Aut
776773

777774
func newAppDbReconciler(ctx context.Context, c client.Client, opsManager *omv1.MongoDBOpsManager, omConnectionFactoryFunc om.ConnectionFactory, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
778775
commonController := NewReconcileCommonController(ctx, c)
779-
return NewAppDBReplicaSetReconciler(ctx, opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, nil, zap.S())
776+
return NewAppDBReplicaSetReconciler(ctx, nil, opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, nil, zap.S())
780777
}
781778

782779
func newAppDbMultiReconciler(ctx context.Context, c client.Client, opsManager *omv1.MongoDBOpsManager, memberClusterMap map[string]client.Client, log *zap.SugaredLogger, omConnectionFactoryFunc om.ConnectionFactory) (*ReconcileAppDbReplicaSet, error) {
783780
_ = c.Update(ctx, opsManager)
784781
commonController := NewReconcileCommonController(ctx, c)
785-
return NewAppDBReplicaSetReconciler(ctx, opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, memberClusterMap, log)
782+
return NewAppDBReplicaSetReconciler(ctx, nil, opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, memberClusterMap, log)
786783
}
787784

788785
func TestChangingFCVAppDB(t *testing.T) {

‎controllers/operator/authentication_test.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestX509CanBeEnabled_WhenThereAreOnlyTlsDeployments_ReplicaSet(t *testing.T
4848

4949
addKubernetesTlsResources(ctx, kubeClient, rs)
5050

51-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
51+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
5252
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
5353
}
5454

@@ -58,7 +58,7 @@ func TestX509ClusterAuthentication_CanBeEnabled_IfX509AuthenticationIsEnabled_Re
5858
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
5959
addKubernetesTlsResources(ctx, kubeClient, rs)
6060

61-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
61+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
6262
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
6363
}
6464

@@ -91,7 +91,7 @@ func TestUpdateOmAuthentication_NoAuthenticationEnabled(t *testing.T) {
9191
processNames := []string{"my-rs-0", "my-rs-1", "my-rs-2"}
9292

9393
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
94-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
94+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
9595
r.updateOmAuthentication(ctx, conn, processNames, rs, "", "", "", false, zap.S())
9696

9797
ac, _ := conn.ReadAutomationConfig()
@@ -112,7 +112,7 @@ func TestUpdateOmAuthentication_EnableX509_TlsNotEnabled(t *testing.T) {
112112
rs.Spec.Security.TLSConfig.Enabled = true
113113

114114
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
115-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
115+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
116116
status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, conn, []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
117117

118118
assert.True(t, status.IsOK(), "configuring both options at once should not result in a failed status")
@@ -124,7 +124,7 @@ func TestUpdateOmAuthentication_EnableX509_WithTlsAlreadyEnabled(t *testing.T) {
124124
rs := DefaultReplicaSetBuilder().SetName("my-rs").SetMembers(3).EnableTLS().Build()
125125
omConnectionFactory := om.NewCachedOMConnectionFactoryWithInitializedConnection(om.NewMockedOmConnection(deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rs)))
126126
kubeClient := mock.NewDefaultFakeClientWithOMConnectionFactory(omConnectionFactory, rs)
127-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
127+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
128128
status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
129129

130130
assert.True(t, status.IsOK(), "configuring x509 when tls has already been enabled should not result in a failed status")
@@ -139,7 +139,7 @@ func TestUpdateOmAuthentication_AuthenticationIsNotConfigured_IfAuthIsNotSet(t *
139139

140140
omConnectionFactory := om.NewCachedOMConnectionFactoryWithInitializedConnection(om.NewMockedOmConnection(deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rs)))
141141
kubeClient := mock.NewDefaultFakeClientWithOMConnectionFactory(omConnectionFactory, rs)
142-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
142+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
143143

144144
status, _ := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
145145
assert.True(t, status.IsOK(), "no authentication should have been configured")
@@ -162,7 +162,7 @@ func TestUpdateOmAuthentication_DoesNotDisableAuth_IfAuthIsNotSet(t *testing.T)
162162
Build()
163163

164164
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
165-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
165+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
166166

167167
addKubernetesTlsResources(ctx, kubeClient, rs)
168168

@@ -175,7 +175,7 @@ func TestUpdateOmAuthentication_DoesNotDisableAuth_IfAuthIsNotSet(t *testing.T)
175175

176176
rs.Spec.Security.Authentication = nil
177177

178-
reconciler = newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
178+
reconciler = newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
179179

180180
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
181181

@@ -197,7 +197,7 @@ func TestCanConfigureAuthenticationDisabled_WithNoModes(t *testing.T) {
197197
Build()
198198

199199
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
200-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
200+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
201201

202202
addKubernetesTlsResources(ctx, kubeClient, rs)
203203

@@ -209,7 +209,7 @@ func TestUpdateOmAuthentication_EnableX509_FromEmptyDeployment(t *testing.T) {
209209
rs := DefaultReplicaSetBuilder().SetName("my-rs").SetMembers(3).EnableTLS().EnableAuth().EnableX509().Build()
210210
omConnectionFactory := om.NewCachedOMConnectionFactoryWithInitializedConnection(om.NewMockedOmConnection(om.NewDeployment()))
211211
kubeClient := mock.NewDefaultFakeClientWithOMConnectionFactory(omConnectionFactory, rs)
212-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
212+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
213213
createAgentCSRs(t, ctx, 1, r.client, certsv1.CertificateApproved)
214214

215215
status, isMultiStageReconciliation := r.updateOmAuthentication(ctx, omConnectionFactory.GetConnection(), []string{"my-rs-0", "my-rs-1", "my-rs-2"}, rs, "", "", "", false, zap.S())
@@ -229,7 +229,7 @@ func TestX509AgentUserIsCorrectlyConfigured(t *testing.T) {
229229

230230
// configure x509/tls resources
231231
addKubernetesTlsResources(ctx, kubeClient, rs)
232-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
232+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
233233

234234
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
235235

@@ -265,7 +265,7 @@ func TestScramAgentUserIsCorrectlyConfigured(t *testing.T) {
265265

266266
assert.NoError(t, err)
267267

268-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
268+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
269269

270270
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
271271

@@ -295,7 +295,7 @@ func TestScramAgentUser_IsNotOverridden(t *testing.T) {
295295
}
296296
})
297297

298-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
298+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
299299

300300
checkReconcileSuccessful(ctx, t, reconciler, rs, kubeClient)
301301

@@ -314,7 +314,7 @@ func TestX509InternalClusterAuthentication_CanBeEnabledWithScram_ReplicaSet(t *t
314314
Build()
315315

316316
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
317-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
317+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
318318
addKubernetesTlsResources(ctx, r.client, rs)
319319

320320
checkReconcileSuccessful(ctx, t, r, rs, kubeClient)
@@ -367,7 +367,7 @@ func TestConfigureLdapDeploymentAuthentication_WithScramAgentAuthentication(t *t
367367
Build()
368368

369369
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
370-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
370+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
371371
data := map[string]string{
372372
"password": "LITZTOd6YiCV8j",
373373
}
@@ -424,7 +424,7 @@ func TestConfigureLdapDeploymentAuthentication_WithCustomRole(t *testing.T) {
424424
Build()
425425

426426
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
427-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
427+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
428428
data := map[string]string{
429429
"password": "LITZTOd6YiCV8j",
430430
}
@@ -478,7 +478,7 @@ func TestConfigureLdapDeploymentAuthentication_WithAuthzQueryTemplate_AndUserToD
478478
Build()
479479

480480
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
481-
r := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
481+
r := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
482482
data := map[string]string{
483483
"password": "LITZTOd6YiCV8j",
484484
}
@@ -741,7 +741,7 @@ func TestInvalidPEM_SecretDoesNotContainKey(t *testing.T) {
741741
Build()
742742

743743
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
744-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
744+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
745745
addKubernetesTlsResources(ctx, kubeClient, rs)
746746

747747
// Replace the secret with an empty one
@@ -796,7 +796,7 @@ func Test_NoExternalDomainPresent(t *testing.T) {
796796
rs.Spec.ExternalAccessConfiguration = &mdbv1.ExternalAccessConfiguration{ExternalDomain: ptr.To("foo")}
797797

798798
kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
799-
reconciler := newReplicaSetReconciler(ctx, kubeClient, false, omConnectionFactory.GetConnectionFunc)
799+
reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, false, omConnectionFactory.GetConnectionFunc)
800800
addKubernetesTlsResources(ctx, kubeClient, rs)
801801

802802
secret := &corev1.Secret{}

0 commit comments

Comments
 (0)