Skip to content

Commit 405d4cf

Browse files
helderjshelderjs
authored
CLOUDP-278048: Configurable sync period for independent custom resources (#1858)
1 parent 6d9a918 commit 405d4cf

File tree

9 files changed

+89
-45
lines changed

9 files changed

+89
-45
lines changed

‎cmd/manager/main.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"log"
2424
"os"
2525
"strings"
26+
"time"
2627

2728
"github.com/go-logr/zapr"
2829
"go.uber.org/zap"
@@ -51,6 +52,8 @@ const (
5152
objectDeletionProtectionDefault = true
5253
subobjectDeletionProtectionDefault = false
5354
subobjectDeletionProtectionMessage = "Note: sub-object deletion protection is IGNORED because it does not work deterministically."
55+
independentSyncPeriod = 15 // time in minutes
56+
minimumIndependentSyncPeriod = 5 // time in minutes
5457
)
5558

5659
func main() {
@@ -72,7 +75,7 @@ func main() {
7275
setupLog := logger.Named("setup").Sugar()
7376
setupLog.Info("starting with configuration", zap.Any("config", config), zap.Any("version", version.Version))
7477

75-
mgr, err := operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), akoScheme).
78+
mgr, err := operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), akoScheme, time.Duration(minimumIndependentSyncPeriod)*time.Minute).
7679
WithConfig(ctrl.GetConfigOrDie()).
7780
WithNamespaces(collection.Keys(config.WatchedNamespaces)...).
7881
WithLogger(logger).
@@ -82,6 +85,7 @@ func main() {
8285
WithAtlasDomain(config.AtlasDomain).
8386
WithAPISecret(config.GlobalAPISecret).
8487
WithDeletionProtection(config.ObjectDeletionProtection).
88+
WithIndependentSyncPeriod(time.Duration(config.IndependentSyncPeriod) * time.Minute).
8589
Build(ctx)
8690
if err != nil {
8791
setupLog.Error(err, "unable to start operator")
@@ -107,6 +111,7 @@ type Config struct {
107111
LogEncoder string
108112
ObjectDeletionProtection bool
109113
SubObjectDeletionProtection bool
114+
IndependentSyncPeriod int
110115
FeatureFlags *featureflags.FeatureFlags
111116
}
112117

@@ -128,6 +133,12 @@ func parseConfiguration() Config {
128133
"when a Custom Resource is deleted")
129134
flag.BoolVar(&config.SubObjectDeletionProtection, subobjectDeletionProtectionFlag, subobjectDeletionProtectionDefault, "Defines if the operator overwrites "+
130135
"(and consequently delete) subresources that were not previously created by the operator. "+subobjectDeletionProtectionMessage)
136+
flag.IntVar(
137+
&config.IndependentSyncPeriod,
138+
"independent-sync-period",
139+
independentSyncPeriod,
140+
fmt.Sprintf("The default time, in minutes, between reconciliations for independent custom resources. (default %d, minimum %d)", independentSyncPeriod, minimumIndependentSyncPeriod),
141+
)
131142
appVersion := flag.Bool("v", false, "prints application version")
132143
flag.Parse()
133144

‎pkg/controller/atlasdatabaseuser/atlasdatabaseuser_controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package atlasdatabaseuser
1919
import (
2020
"context"
2121
"fmt"
22+
"time"
2223

2324
"go.uber.org/zap"
2425
corev1 "k8s.io/api/core/v1"
@@ -65,6 +66,7 @@ type AtlasDatabaseUserReconciler struct {
6566
ObjectDeletionProtection bool
6667
SubObjectDeletionProtection bool
6768
FeaturePreviewOIDCAuthEnabled bool
69+
independentSyncPeriod time.Duration
6870

6971
dbUserService dbuser.AtlasUsersService
7072
deploymentService deployment.AtlasDeploymentsService
@@ -203,7 +205,7 @@ func (r *AtlasDatabaseUserReconciler) ready(ctx *workflow.Context, atlasDatabase
203205
EnsureStatusOption(status.AtlasDatabaseUserPasswordVersion(passwordVersion))
204206

205207
if atlasDatabaseUser.Spec.ExternalProjectRef != nil {
206-
return workflow.Requeue(workflow.StandaloneResourceRequeuePeriod).ReconcileResult()
208+
return workflow.Requeue(r.independentSyncPeriod).ReconcileResult()
207209
}
208210

209211
return workflow.OK().ReconcileResult()
@@ -275,6 +277,7 @@ func NewAtlasDatabaseUserReconciler(
275277
predicates []predicate.Predicate,
276278
atlasProvider atlas.Provider,
277279
deletionProtection bool,
280+
independentSyncPeriod time.Duration,
278281
featureFlags *featureflags.FeatureFlags,
279282
logger *zap.Logger,
280283
) *AtlasDatabaseUserReconciler {
@@ -287,5 +290,6 @@ func NewAtlasDatabaseUserReconciler(
287290
AtlasProvider: atlasProvider,
288291
ObjectDeletionProtection: deletionProtection,
289292
FeaturePreviewOIDCAuthEnabled: featureFlags.IsFeaturePresent(featureflags.FeatureOIDC),
293+
independentSyncPeriod: independentSyncPeriod,
290294
}
291295
}

‎pkg/controller/atlasdatabaseuser/atlasdatabaseuser_controller_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"reflect"
77
"testing"
8+
"time"
89

910
"github.com/google/go-cmp/cmp"
1011
"github.com/google/go-cmp/cmp/cmpopts"
@@ -385,7 +386,7 @@ func TestReady(t *testing.T) {
385386
api.TrueCondition(api.DatabaseUserReadyType),
386387
},
387388
},
388-
"don't requeue when it's a standalone resource": {
389+
"requeue when it's a standalone resource": {
389390
dbUser: &akov2.AtlasDatabaseUser{
390391
ObjectMeta: metav1.ObjectMeta{
391392
Name: "user1",
@@ -408,7 +409,7 @@ func TestReady(t *testing.T) {
408409
},
409410
},
410411
passwordVersion: "1",
411-
expectedResult: workflow.Requeue(workflow.StandaloneResourceRequeuePeriod).ReconcileResult(),
412+
expectedResult: workflow.Requeue(10 * time.Minute).ReconcileResult(),
412413
expectedConditions: []api.Condition{
413414
api.TrueCondition(api.ReadyType),
414415
api.TrueCondition(api.DatabaseUserReadyType),
@@ -430,8 +431,9 @@ func TestReady(t *testing.T) {
430431

431432
logger := zaptest.NewLogger(t).Sugar()
432433
c := &AtlasDatabaseUserReconciler{
433-
Client: k8sClient,
434-
Log: logger,
434+
Client: k8sClient,
435+
Log: logger,
436+
independentSyncPeriod: 10 * time.Minute,
435437
}
436438
ctx := &workflow.Context{
437439
Context: context.Background(),

‎pkg/controller/workflow/result.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ import (
77
)
88

99
const (
10-
DefaultRetry = time.Second * 10
11-
StandaloneResourceRequeuePeriod = time.Minute * 15
12-
DefaultTimeout = time.Minute * 20
10+
DefaultRetry = time.Second * 10
1311
)
1412

1513
type Result struct {

‎pkg/operator/builder.go‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package operator
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"os"
78
"time"
@@ -37,9 +38,10 @@ import (
3738
)
3839

3940
const (
40-
DefaultAtlasDomain = "https://cloud.mongodb.com/"
41-
DefaultSyncPeriod = 3 * time.Hour
42-
DefaultLeaderElectionID = "06d035fb.mongodb.com"
41+
DefaultAtlasDomain = "https://cloud.mongodb.com/"
42+
DefaultSyncPeriod = 3 * time.Hour
43+
DefaultIndependentSyncPeriod = 15 * time.Minute
44+
DefaultLeaderElectionID = "06d035fb.mongodb.com"
4345
)
4446

4547
type ManagerProvider interface {
@@ -53,17 +55,19 @@ func (f ManagerProviderFunc) New(config *rest.Config, options manager.Options) (
5355
}
5456

5557
type Builder struct {
56-
managerProvider ManagerProvider
57-
scheme *runtime.Scheme
58-
59-
config *rest.Config
60-
namespaces []string
61-
logger *zap.Logger
62-
syncPeriod time.Duration
63-
metricAddress string
64-
probeAddress string
65-
leaderElection bool
66-
leaderElectionID string
58+
managerProvider ManagerProvider
59+
scheme *runtime.Scheme
60+
minimumIndependentSyncPeriod time.Duration
61+
62+
config *rest.Config
63+
namespaces []string
64+
logger *zap.Logger
65+
syncPeriod time.Duration
66+
independentSyncPeriod time.Duration
67+
metricAddress string
68+
probeAddress string
69+
leaderElection bool
70+
leaderElectionID string
6771

6872
atlasDomain string
6973
predicates []predicate.Predicate
@@ -139,6 +143,11 @@ func (b *Builder) WithDeletionProtection(deletionProtection bool) *Builder {
139143
return b
140144
}
141145

146+
func (b *Builder) WithIndependentSyncPeriod(period time.Duration) *Builder {
147+
b.independentSyncPeriod = period
148+
return b
149+
}
150+
142151
// WithSkipNameValidation skips name validation in controller-runtime
143152
// to prevent duplicate controller names.
144153
//
@@ -152,6 +161,10 @@ func (b *Builder) WithSkipNameValidation(skip bool) *Builder {
152161
func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
153162
mergeDefaults(b)
154163

164+
if b.independentSyncPeriod < b.minimumIndependentSyncPeriod {
165+
return nil, errors.New("wrong value for independentSyncPeriod. Value should be greater or equal to 5")
166+
}
167+
155168
cacheOpts := cache.Options{
156169
SyncPeriod: &b.syncPeriod,
157170
}
@@ -233,6 +246,7 @@ func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
233246
b.predicates,
234247
b.atlasProvider,
235248
b.deletionProtection,
249+
b.independentSyncPeriod,
236250
b.featureFlags,
237251
b.logger,
238252
)
@@ -310,10 +324,11 @@ func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
310324
}
311325

312326
// NewBuilder return a new Builder to construct operator controllers
313-
func NewBuilder(provider ManagerProvider, scheme *runtime.Scheme) *Builder {
327+
func NewBuilder(provider ManagerProvider, scheme *runtime.Scheme, minimumIndependentSyncPeriod time.Duration) *Builder {
314328
return &Builder{
315-
managerProvider: provider,
316-
scheme: scheme,
329+
managerProvider: provider,
330+
scheme: scheme,
331+
minimumIndependentSyncPeriod: minimumIndependentSyncPeriod,
317332
}
318333
}
319334

@@ -330,6 +345,10 @@ func mergeDefaults(b *Builder) {
330345
b.syncPeriod = DefaultSyncPeriod
331346
}
332347

348+
if b.independentSyncPeriod == 0 {
349+
b.independentSyncPeriod = DefaultIndependentSyncPeriod
350+
}
351+
333352
if b.metricAddress == "" {
334353
b.metricAddress = "0"
335354
}

‎pkg/operator/builder_test.go‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package operator
22

33
import (
44
"context"
5+
"errors"
56
"testing"
67
"time"
78

@@ -35,17 +36,14 @@ type managerMock struct {
3536
client client.Client
3637
scheme *runtime.Scheme
3738

38-
gotHealthzCheck string
39-
gotReadyzCheck string
40-
4139
opts ctrl.Options
4240
}
4341

4442
func (m *managerMock) GetCache() cache.Cache {
4543
return &informertest.FakeInformers{}
4644
}
4745

48-
func (m *managerMock) Add(runnable manager.Runnable) error {
46+
func (m *managerMock) Add(_ manager.Runnable) error {
4947
return nil
5048
}
5149

@@ -61,7 +59,7 @@ func (m *managerMock) GetScheme() *runtime.Scheme {
6159
return m.scheme
6260
}
6361

64-
func (m *managerMock) GetEventRecorderFor(name string) record.EventRecorder {
62+
func (m *managerMock) GetEventRecorderFor(_ string) record.EventRecorder {
6563
return record.NewFakeRecorder(100)
6664
}
6765

@@ -73,7 +71,7 @@ func (m *managerMock) GetFieldIndexer() client.FieldIndexer {
7371
return &informertest.FakeInformers{}
7472
}
7573

76-
func (m *managerMock) New(config *rest.Config, options manager.Options) (manager.Manager, error) {
74+
func (m *managerMock) New(_ *rest.Config, options manager.Options) (manager.Manager, error) {
7775
m.opts = options
7876
m.scheme = options.Scheme
7977
m.client = fake.NewClientBuilder().
@@ -83,13 +81,11 @@ func (m *managerMock) New(config *rest.Config, options manager.Options) (manager
8381
return m, nil
8482
}
8583

86-
func (m *managerMock) AddHealthzCheck(name string, check healthz.Checker) error {
87-
m.gotHealthzCheck = name
84+
func (m *managerMock) AddHealthzCheck(_ string, _ healthz.Checker) error {
8885
return nil
8986
}
9087

91-
func (m *managerMock) AddReadyzCheck(name string, check healthz.Checker) error {
92-
m.gotReadyzCheck = name
88+
func (m *managerMock) AddReadyzCheck(_ string, _ healthz.Checker) error {
9389
return nil
9490
}
9591

@@ -99,6 +95,7 @@ func TestBuildManager(t *testing.T) {
9995
expectedSyncPeriod time.Duration
10096
expectedClusterWideCache bool
10197
expectedNamespacedCache bool
98+
expectedError error
10299
}{
103100
"should build the manager with default values": {
104101
configure: func(b *Builder) {},
@@ -120,6 +117,7 @@ func TestBuildManager(t *testing.T) {
120117
WithNamespaces("ns1").
121118
WithLogger(zaptest.NewLogger(t)).
122119
WithSyncPeriod(time.Hour).
120+
WithIndependentSyncPeriod(15 * time.Minute).
123121
WithMetricAddress(":9090").
124122
WithProbeAddress(":9091").
125123
WithLeaderElection(true).
@@ -134,6 +132,15 @@ func TestBuildManager(t *testing.T) {
134132
expectedClusterWideCache: false,
135133
expectedNamespacedCache: true,
136134
},
135+
"should error when independentSyncPeriod is misconfigured": {
136+
configure: func(b *Builder) {
137+
b.WithIndependentSyncPeriod(4 * time.Minute)
138+
},
139+
expectedSyncPeriod: DefaultSyncPeriod,
140+
expectedClusterWideCache: false,
141+
expectedNamespacedCache: true,
142+
expectedError: errors.New("wrong value for independentSyncPeriod. Value should be greater or equal to 5"),
143+
},
137144
}
138145

139146
for name, tt := range tests {
@@ -142,16 +149,18 @@ func TestBuildManager(t *testing.T) {
142149
require.NoError(t, akov2.AddToScheme(akoScheme))
143150

144151
mgrMock := &managerMock{}
145-
builder := NewBuilder(mgrMock, akoScheme)
152+
builder := NewBuilder(mgrMock, akoScheme, 5*time.Minute)
146153
tt.configure(builder)
147154
// this is necessary for tests
148155
builder.WithSkipNameValidation(true)
149156
_, err := builder.Build(context.Background())
150-
require.NoError(t, err)
157+
require.Equal(t, tt.expectedError, err)
151158

152-
assert.Equal(t, tt.expectedSyncPeriod, *mgrMock.opts.Cache.SyncPeriod)
153-
assert.Equal(t, tt.expectedClusterWideCache, len(mgrMock.opts.Cache.ByObject) > 0)
154-
assert.Equal(t, tt.expectedNamespacedCache, len(mgrMock.opts.Cache.DefaultNamespaces) > 0)
159+
if err == nil {
160+
assert.Equal(t, tt.expectedSyncPeriod, *mgrMock.opts.Cache.SyncPeriod)
161+
assert.Equal(t, tt.expectedClusterWideCache, len(mgrMock.opts.Cache.ByObject) > 0)
162+
assert.Equal(t, tt.expectedNamespacedCache, len(mgrMock.opts.Cache.DefaultNamespaces) > 0)
163+
}
155164
})
156165
}
157166
}

‎test/helper/e2e/k8s/operator.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"sync"
7+
"time"
78

89
"github.com/go-logr/zapr"
910
. "github.com/onsi/ginkgo/v2"
@@ -50,7 +51,7 @@ func BuildManager(initCfg *Config) (manager.Manager, error) {
5051
signalCancelledCtx = ctrl.SetupSignalHandler()
5152
})
5253

53-
return operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), akoScheme).
54+
return operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), akoScheme, 5*time.Minute).
5455
WithConfig(ctrl.GetConfigOrDie()).
5556
WithNamespaces(collection.Keys(config.WatchedNamespaces)...).
5657
WithLogger(logger).

‎test/int/clusterwide/integration_suite_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
115115
logger := ctrzap.NewRaw(ctrzap.UseDevMode(true), ctrzap.WriteTo(GinkgoWriter), ctrzap.StacktraceLevel(zap.ErrorLevel))
116116
ctrl.SetLogger(zapr.NewLogger(logger))
117117

118-
mgr, err := operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), testEnv.Scheme).
118+
mgr, err := operator.NewBuilder(operator.ManagerProviderFunc(ctrl.NewManager), testEnv.Scheme, 5*time.Minute).
119119
WithConfig(testEnv.Config).
120120
WithLogger(logger).
121121
WithAtlasDomain(atlasDomain).

0 commit comments

Comments
 (0)