Skip to content

Commit

Permalink
configurable sync period for independent custom resources
Browse files Browse the repository at this point in the history
# Conflicts:
#	pkg/controller/atlasdatabaseuser/atlasdatabaseuser_controller_test.go
  • Loading branch information
helderjs committed Oct 11, 2024
1 parent 6d9a918 commit 686d56b
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 33 deletions.
5 changes: 5 additions & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"log"
"os"
"strings"
"time"

"github.com/go-logr/zapr"
"go.uber.org/zap"
Expand Down Expand Up @@ -51,6 +52,7 @@ const (
objectDeletionProtectionDefault = true
subobjectDeletionProtectionDefault = false
subobjectDeletionProtectionMessage = "Note: sub-object deletion protection is IGNORED because it does not work deterministically."
independentSyncPeriod = 15 // time in minutes
)

func main() {
Expand Down Expand Up @@ -82,6 +84,7 @@ func main() {
WithAtlasDomain(config.AtlasDomain).
WithAPISecret(config.GlobalAPISecret).
WithDeletionProtection(config.ObjectDeletionProtection).
WithIndependentSyncPeriod(time.Duration(config.IndependentSyncPeriod) * time.Minute).
Build(ctx)
if err != nil {
setupLog.Error(err, "unable to start operator")
Expand All @@ -107,6 +110,7 @@ type Config struct {
LogEncoder string
ObjectDeletionProtection bool
SubObjectDeletionProtection bool
IndependentSyncPeriod int
FeatureFlags *featureflags.FeatureFlags
}

Expand All @@ -128,6 +132,7 @@ func parseConfiguration() Config {
"when a Custom Resource is deleted")
flag.BoolVar(&config.SubObjectDeletionProtection, subobjectDeletionProtectionFlag, subobjectDeletionProtectionDefault, "Defines if the operator overwrites "+
"(and consequently delete) subresources that were not previously created by the operator. "+subobjectDeletionProtectionMessage)
flag.IntVar(&config.IndependentSyncPeriod, "independent-sync-period", independentSyncPeriod, "The default time, in minutes, between reconciliations for independent custom resources. (default 15, minimum 5)")
appVersion := flag.Bool("v", false, "prints application version")
flag.Parse()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package atlasdatabaseuser
import (
"context"
"fmt"
"time"

"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -65,6 +66,7 @@ type AtlasDatabaseUserReconciler struct {
ObjectDeletionProtection bool
SubObjectDeletionProtection bool
FeaturePreviewOIDCAuthEnabled bool
independentSyncPeriod time.Duration

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

if atlasDatabaseUser.Spec.ExternalProjectRef != nil {
return workflow.Requeue(workflow.StandaloneResourceRequeuePeriod).ReconcileResult()
return workflow.Requeue(r.independentSyncPeriod).ReconcileResult()
}

return workflow.OK().ReconcileResult()
Expand Down Expand Up @@ -275,6 +277,7 @@ func NewAtlasDatabaseUserReconciler(
predicates []predicate.Predicate,
atlasProvider atlas.Provider,
deletionProtection bool,
independentSyncPeriod time.Duration,
featureFlags *featureflags.FeatureFlags,
logger *zap.Logger,
) *AtlasDatabaseUserReconciler {
Expand All @@ -287,5 +290,6 @@ func NewAtlasDatabaseUserReconciler(
AtlasProvider: atlasProvider,
ObjectDeletionProtection: deletionProtection,
FeaturePreviewOIDCAuthEnabled: featureFlags.IsFeaturePresent(featureflags.FeatureOIDC),
independentSyncPeriod: independentSyncPeriod,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"reflect"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand Down Expand Up @@ -385,7 +386,7 @@ func TestReady(t *testing.T) {
api.TrueCondition(api.DatabaseUserReadyType),
},
},
"don't requeue when it's a standalone resource": {
"requeue when it's a standalone resource": {
dbUser: &akov2.AtlasDatabaseUser{
ObjectMeta: metav1.ObjectMeta{
Name: "user1",
Expand All @@ -408,7 +409,7 @@ func TestReady(t *testing.T) {
},
},
passwordVersion: "1",
expectedResult: workflow.Requeue(workflow.StandaloneResourceRequeuePeriod).ReconcileResult(),
expectedResult: workflow.Requeue(10 * time.Minute).ReconcileResult(),
expectedConditions: []api.Condition{
api.TrueCondition(api.ReadyType),
api.TrueCondition(api.DatabaseUserReadyType),
Expand All @@ -430,8 +431,9 @@ func TestReady(t *testing.T) {

logger := zaptest.NewLogger(t).Sugar()
c := &AtlasDatabaseUserReconciler{
Client: k8sClient,
Log: logger,
Client: k8sClient,
Log: logger,
independentSyncPeriod: 10 * time.Minute,
}
ctx := &workflow.Context{
Context: context.Background(),
Expand Down
4 changes: 1 addition & 3 deletions pkg/controller/workflow/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import (
)

const (
DefaultRetry = time.Second * 10
StandaloneResourceRequeuePeriod = time.Minute * 15
DefaultTimeout = time.Minute * 20
DefaultRetry = time.Second * 10
)

type Result struct {
Expand Down
41 changes: 30 additions & 11 deletions pkg/operator/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package operator

import (
"context"
"errors"
"fmt"
"os"
"time"
Expand Down Expand Up @@ -37,9 +38,12 @@ import (
)

const (
DefaultAtlasDomain = "https://cloud.mongodb.com/"
DefaultSyncPeriod = 3 * time.Hour
DefaultLeaderElectionID = "06d035fb.mongodb.com"
DefaultAtlasDomain = "https://cloud.mongodb.com/"
DefaultSyncPeriod = 3 * time.Hour
DefaultIndependentSyncPeriod = 15 * time.Minute
DefaultLeaderElectionID = "06d035fb.mongodb.com"

minimumIndependentSyncPeriod = 5 * time.Minute
)

type ManagerProvider interface {
Expand All @@ -56,14 +60,15 @@ type Builder struct {
managerProvider ManagerProvider
scheme *runtime.Scheme

config *rest.Config
namespaces []string
logger *zap.Logger
syncPeriod time.Duration
metricAddress string
probeAddress string
leaderElection bool
leaderElectionID string
config *rest.Config
namespaces []string
logger *zap.Logger
syncPeriod time.Duration
independentSyncPeriod time.Duration
metricAddress string
probeAddress string
leaderElection bool
leaderElectionID string

atlasDomain string
predicates []predicate.Predicate
Expand Down Expand Up @@ -139,6 +144,11 @@ func (b *Builder) WithDeletionProtection(deletionProtection bool) *Builder {
return b
}

func (b *Builder) WithIndependentSyncPeriod(period time.Duration) *Builder {
b.independentSyncPeriod = period
return b
}

// WithSkipNameValidation skips name validation in controller-runtime
// to prevent duplicate controller names.
//
Expand All @@ -152,6 +162,10 @@ func (b *Builder) WithSkipNameValidation(skip bool) *Builder {
func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
mergeDefaults(b)

if b.independentSyncPeriod < minimumIndependentSyncPeriod {
return nil, errors.New("wrong value for independentSyncPeriod. Value should be greater or equal to 5")
}

cacheOpts := cache.Options{
SyncPeriod: &b.syncPeriod,
}
Expand Down Expand Up @@ -233,6 +247,7 @@ func (b *Builder) Build(ctx context.Context) (manager.Manager, error) {
b.predicates,
b.atlasProvider,
b.deletionProtection,
b.independentSyncPeriod,
b.featureFlags,
b.logger,
)
Expand Down Expand Up @@ -330,6 +345,10 @@ func mergeDefaults(b *Builder) {
b.syncPeriod = DefaultSyncPeriod
}

if b.independentSyncPeriod == 0 {
b.independentSyncPeriod = DefaultIndependentSyncPeriod
}

if b.metricAddress == "" {
b.metricAddress = "0"
}
Expand Down
37 changes: 23 additions & 14 deletions pkg/operator/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package operator

import (
"context"
"errors"
"testing"
"time"

Expand Down Expand Up @@ -35,17 +36,14 @@ type managerMock struct {
client client.Client
scheme *runtime.Scheme

gotHealthzCheck string
gotReadyzCheck string

opts ctrl.Options
}

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

func (m *managerMock) Add(runnable manager.Runnable) error {
func (m *managerMock) Add(_ manager.Runnable) error {
return nil
}

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

func (m *managerMock) GetEventRecorderFor(name string) record.EventRecorder {
func (m *managerMock) GetEventRecorderFor(_ string) record.EventRecorder {
return record.NewFakeRecorder(100)
}

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

func (m *managerMock) New(config *rest.Config, options manager.Options) (manager.Manager, error) {
func (m *managerMock) New(_ *rest.Config, options manager.Options) (manager.Manager, error) {
m.opts = options
m.scheme = options.Scheme
m.client = fake.NewClientBuilder().
Expand All @@ -83,13 +81,11 @@ func (m *managerMock) New(config *rest.Config, options manager.Options) (manager
return m, nil
}

func (m *managerMock) AddHealthzCheck(name string, check healthz.Checker) error {
m.gotHealthzCheck = name
func (m *managerMock) AddHealthzCheck(_ string, _ healthz.Checker) error {
return nil
}

func (m *managerMock) AddReadyzCheck(name string, check healthz.Checker) error {
m.gotReadyzCheck = name
func (m *managerMock) AddReadyzCheck(_ string, _ healthz.Checker) error {
return nil
}

Expand All @@ -99,6 +95,7 @@ func TestBuildManager(t *testing.T) {
expectedSyncPeriod time.Duration
expectedClusterWideCache bool
expectedNamespacedCache bool
expectedError error
}{
"should build the manager with default values": {
configure: func(b *Builder) {},
Expand All @@ -120,6 +117,7 @@ func TestBuildManager(t *testing.T) {
WithNamespaces("ns1").
WithLogger(zaptest.NewLogger(t)).
WithSyncPeriod(time.Hour).
WithIndependentSyncPeriod(15 * time.Minute).
WithMetricAddress(":9090").
WithProbeAddress(":9091").
WithLeaderElection(true).
Expand All @@ -134,6 +132,15 @@ func TestBuildManager(t *testing.T) {
expectedClusterWideCache: false,
expectedNamespacedCache: true,
},
"should error when independentSyncPeriod is misconfigured": {
configure: func(b *Builder) {
b.WithIndependentSyncPeriod(4 * time.Minute)
},
expectedSyncPeriod: DefaultSyncPeriod,
expectedClusterWideCache: false,
expectedNamespacedCache: true,
expectedError: errors.New("wrong value for independentSyncPeriod. Value should be greater or equal to 5"),
},
}

for name, tt := range tests {
Expand All @@ -147,11 +154,13 @@ func TestBuildManager(t *testing.T) {
// this is necessary for tests
builder.WithSkipNameValidation(true)
_, err := builder.Build(context.Background())
require.NoError(t, err)
require.Equal(t, tt.expectedError, err)

assert.Equal(t, tt.expectedSyncPeriod, *mgrMock.opts.Cache.SyncPeriod)
assert.Equal(t, tt.expectedClusterWideCache, len(mgrMock.opts.Cache.ByObject) > 0)
assert.Equal(t, tt.expectedNamespacedCache, len(mgrMock.opts.Cache.DefaultNamespaces) > 0)
if err == nil {
assert.Equal(t, tt.expectedSyncPeriod, *mgrMock.opts.Cache.SyncPeriod)
assert.Equal(t, tt.expectedClusterWideCache, len(mgrMock.opts.Cache.ByObject) > 0)
assert.Equal(t, tt.expectedNamespacedCache, len(mgrMock.opts.Cache.DefaultNamespaces) > 0)
}
})
}
}

0 comments on commit 686d56b

Please sign in to comment.