Skip to content

Commit 54091db

Browse files
josvazgjosvazg
authored
CLOUDP-299772: Remove last skipped config (#2192)
* Clear last skipped config * Simplify Last Applied Config consumption * Clear resources migrated as full CRDs at skip time Signed-off-by: jose.vazquez <[email protected]> * Test skip clears the expected migrated embedded resource lists Signed-off-by: jose.vazquez <[email protected]> * Remove unneeded extra spec *S param * Add and use PatchLastConfigApplied with tests Signed-off-by: jose.vazquez <[email protected]> * Remove JSON trace in error output * Simplified PatchLastConfigApplied Signed-off-by: jose.vazquez <[email protected]> * Fix nil pointer access --------- Signed-off-by: jose.vazquez <[email protected]>
1 parent be33c28 commit 54091db

13 files changed

+413
-259
lines changed

‎internal/controller/atlasproject/atlasproject_controller.go

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ func (r *AtlasProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
110110
}
111111
}
112112

113-
err := customresource.ApplyLastConfigSkipped(ctx, atlasProject, r.Client)
114-
if err != nil {
115-
log.Errorw("Failed to apply last skipped config", "error", err)
116-
return workflow.Terminate(workflow.Internal, err).ReconcileResult(), nil
113+
if err := r.clearLastAppliedMigratedResources(ctx, atlasProject); err != nil {
114+
result = workflow.Terminate(workflow.Internal, err)
115+
log.Errorw("Failed to clear migrated independent resources", "error", err)
116+
return result.ReconcileResult(), nil
117117
}
118118

119119
return workflow.OK().ReconcileResult(), nil
@@ -376,3 +376,24 @@ func lastAppliedSpecFrom(atlasProject *akov2.AtlasProject) (*akov2.AtlasProjectS
376376

377377
return &lastApplied.Spec, nil
378378
}
379+
380+
func (r *AtlasProjectReconciler) clearLastAppliedMigratedResources(ctx context.Context, atlasProject *akov2.AtlasProject) error {
381+
clearedCfg, err := customresource.ParseLastConfigApplied[akov2.AtlasProjectSpec](atlasProject)
382+
if err != nil {
383+
return fmt.Errorf("failed to parse last applied config annotation: %w", err)
384+
}
385+
if clearedCfg == nil { // nothing to patch
386+
return nil
387+
}
388+
// clear all resources migrated as independent CRDs to avoid eager
389+
// reconciliation that might conflict with independent CRs and apply
390+
clearedCfg.CustomRoles = nil
391+
clearedCfg.PrivateEndpoints = nil
392+
clearedCfg.ProjectIPAccessList = nil
393+
clearedCfg.NetworkPeers = nil
394+
395+
if err := customresource.PatchLastConfigApplied(ctx, r.Client, atlasProject, clearedCfg); err != nil {
396+
return fmt.Errorf("failed to clear migrated resources in last applied config annotation: %w", err)
397+
}
398+
return nil
399+
}

‎internal/controller/atlasproject/atlasproject_controller_test.go

Lines changed: 140 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"k8s.io/apimachinery/pkg/runtime"
2424
"k8s.io/apimachinery/pkg/types"
2525
"k8s.io/client-go/tools/record"
26+
ctrl "sigs.k8s.io/controller-runtime"
2627
"sigs.k8s.io/controller-runtime/pkg/client"
2728
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2829
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
@@ -31,6 +32,7 @@ import (
3132
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api"
3233
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
3334
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/common"
35+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/project"
3436
atlas_controllers "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/atlas"
3537
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
3638
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/workflow"
@@ -418,9 +420,8 @@ func TestLastSpecFrom(t *testing.T) {
418420

419421
"should return nil when there is no last spec": {},
420422
"should return error when last spec annotation is wrong": {
421-
annotations: map[string]string{"mongodb.com/last-applied-configuration": "{wrong}"},
422-
expectedError: "error reading AtlasProject Spec from annotation [mongodb.com/last-applied-configuration]:" +
423-
" invalid character 'w' looking for beginning of object key string",
423+
annotations: map[string]string{"mongodb.com/last-applied-configuration": "{wrong}"},
424+
expectedError: "invalid character 'w' looking for beginning of object key string",
424425
},
425426
"should return last spec": {
426427
annotations: map[string]string{"mongodb.com/last-applied-configuration": "{\"name\": \"my-project\"}"},
@@ -442,6 +443,142 @@ func TestLastSpecFrom(t *testing.T) {
442443
}
443444
}
444445

446+
func TestSkipClearsMigratedResourcesLastConfig(t *testing.T) {
447+
ctx := context.Background()
448+
prj := akov2.AtlasProject{
449+
ObjectMeta: metav1.ObjectMeta{
450+
Name: "test-project",
451+
Namespace: "test-ns",
452+
Annotations: map[string]string{},
453+
},
454+
Spec: akov2.AtlasProjectSpec{
455+
Name: "test-project",
456+
PrivateEndpoints: []akov2.PrivateEndpoint{{}},
457+
CloudProviderAccessRoles: []akov2.CloudProviderAccessRole{{}},
458+
CloudProviderIntegrations: []akov2.CloudProviderIntegration{{}},
459+
AlertConfigurations: []akov2.AlertConfiguration{{}},
460+
NetworkPeers: []akov2.NetworkPeer{{}},
461+
X509CertRef: &common.ResourceRefNamespaced{},
462+
Integrations: []project.Integration{{}},
463+
EncryptionAtRest: &akov2.EncryptionAtRest{},
464+
Auditing: &akov2.Auditing{},
465+
Settings: &akov2.ProjectSettings{},
466+
CustomRoles: []akov2.CustomRole{{}},
467+
Teams: []akov2.Team{{}},
468+
BackupCompliancePolicyRef: &common.ResourceRefNamespaced{},
469+
ConnectionSecret: &common.ResourceRefNamespaced{},
470+
ProjectIPAccessList: []project.IPAccessList{{}},
471+
MaintenanceWindow: project.MaintenanceWindow{},
472+
},
473+
}
474+
prj.Annotations[customresource.AnnotationLastAppliedConfiguration] = jsonize(t, prj.Spec)
475+
prj.Annotations[customresource.ReconciliationPolicyAnnotation] = customresource.ReconciliationPolicySkip
476+
testScheme := runtime.NewScheme()
477+
require.NoError(t, akov2.AddToScheme(testScheme))
478+
require.NoError(t, corev1.AddToScheme(testScheme))
479+
k8sClient := fake.NewClientBuilder().
480+
WithScheme(testScheme).
481+
WithObjects(&prj).
482+
WithStatusSubresource(&prj).
483+
Build()
484+
485+
req := ctrl.Request{NamespacedName: types.NamespacedName{
486+
Name: prj.Name,
487+
Namespace: prj.Namespace,
488+
}}
489+
r := AtlasProjectReconciler{
490+
Client: k8sClient,
491+
Log: zaptest.NewLogger(t).Sugar(),
492+
EventRecorder: record.NewFakeRecorder(30),
493+
AtlasProvider: &atlas.TestProvider{
494+
IsCloudGovFunc: func() bool {
495+
return false
496+
},
497+
IsSupportedFunc: func() bool {
498+
return true
499+
},
500+
},
501+
}
502+
503+
result, err := r.Reconcile(ctx, req)
504+
505+
require.Equal(t, reconcile.Result{}, result)
506+
require.NoError(t, err)
507+
require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(&prj), &prj))
508+
lastApplied, err := customresource.ParseLastConfigApplied[akov2.AtlasProjectSpec](&prj)
509+
require.NoError(t, err)
510+
wantLastApplied := &akov2.AtlasProjectSpec{
511+
Name: "test-project",
512+
PrivateEndpoints: nil,
513+
CustomRoles: nil,
514+
NetworkPeers: nil,
515+
ProjectIPAccessList: nil,
516+
CloudProviderAccessRoles: []akov2.CloudProviderAccessRole{{}},
517+
CloudProviderIntegrations: []akov2.CloudProviderIntegration{{}},
518+
AlertConfigurations: []akov2.AlertConfiguration{{}},
519+
X509CertRef: &common.ResourceRefNamespaced{},
520+
Integrations: []project.Integration{{}},
521+
EncryptionAtRest: &akov2.EncryptionAtRest{},
522+
Auditing: &akov2.Auditing{},
523+
Settings: &akov2.ProjectSettings{},
524+
Teams: []akov2.Team{{}},
525+
BackupCompliancePolicyRef: &common.ResourceRefNamespaced{},
526+
ConnectionSecret: &common.ResourceRefNamespaced{},
527+
}
528+
assert.Equal(t, wantLastApplied, lastApplied)
529+
}
530+
531+
func TestSkipClearsMigratedResourcesLastConfigDoesNotPanic(t *testing.T) {
532+
ctx := context.Background()
533+
prj := akov2.AtlasProject{
534+
ObjectMeta: metav1.ObjectMeta{
535+
Name: "test-project",
536+
Namespace: "test-ns",
537+
Annotations: map[string]string{},
538+
},
539+
Spec: akov2.AtlasProjectSpec{
540+
Name: "test-project",
541+
},
542+
}
543+
prj.Annotations[customresource.ReconciliationPolicyAnnotation] = customresource.ReconciliationPolicySkip
544+
testScheme := runtime.NewScheme()
545+
require.NoError(t, akov2.AddToScheme(testScheme))
546+
require.NoError(t, corev1.AddToScheme(testScheme))
547+
k8sClient := fake.NewClientBuilder().
548+
WithScheme(testScheme).
549+
WithObjects(&prj).
550+
WithStatusSubresource(&prj).
551+
Build()
552+
553+
req := ctrl.Request{NamespacedName: types.NamespacedName{
554+
Name: prj.Name,
555+
Namespace: prj.Namespace,
556+
}}
557+
r := AtlasProjectReconciler{
558+
Client: k8sClient,
559+
Log: zaptest.NewLogger(t).Sugar(),
560+
EventRecorder: record.NewFakeRecorder(30),
561+
AtlasProvider: &atlas.TestProvider{
562+
IsCloudGovFunc: func() bool {
563+
return false
564+
},
565+
IsSupportedFunc: func() bool {
566+
return true
567+
},
568+
},
569+
}
570+
571+
result, err := r.Reconcile(ctx, req)
572+
573+
require.Equal(t, reconcile.Result{}, result)
574+
require.NoError(t, err)
575+
require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(&prj), &prj))
576+
lastApplied, err := customresource.ParseLastConfigApplied[akov2.AtlasProjectSpec](&prj)
577+
require.NoError(t, err)
578+
wantLastApplied := (*akov2.AtlasProjectSpec)(nil)
579+
assert.Equal(t, wantLastApplied, lastApplied)
580+
}
581+
445582
func jsonize(t *testing.T, obj any) string {
446583
t.Helper()
447584

‎internal/controller/atlasproject/custom_roles.go

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,6 @@ type roleController struct {
2222
service customroles.CustomRoleService
2323
}
2424

25-
func shouldCustomRolesSkipReconciling(atlasProject *akov2.AtlasProject) (bool, error) {
26-
lastSkippedSpec := akov2.AtlasProjectSpec{}
27-
lastSkippedSpecString, ok := atlasProject.Annotations[customresource.AnnotationLastSkippedConfiguration]
28-
if ok {
29-
if err := json.Unmarshal([]byte(lastSkippedSpecString), &lastSkippedSpec); err != nil {
30-
return false, fmt.Errorf("failed to parse last skipped configuration: %w", err)
31-
}
32-
33-
return len(lastSkippedSpec.CustomRoles) == 0, nil
34-
}
35-
36-
return false, nil
37-
}
38-
3925
func getLastAppliedCustomRoles(atlasProject *akov2.AtlasProject) ([]akov2.CustomRole, error) {
4026
lastAppliedSpec := akov2.AtlasProjectSpec{}
4127
lastAppliedSpecStr, ok := atlasProject.Annotations[customresource.AnnotationLastAppliedConfiguration]
@@ -76,16 +62,6 @@ func convertToInternalRoles(roles []akov2.CustomRole) []customroles.CustomRole {
7662
}
7763

7864
func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProject) workflow.Result {
79-
skipped, err := shouldCustomRolesSkipReconciling(project)
80-
if err != nil {
81-
return workflow.Terminate(workflow.Internal, err)
82-
}
83-
84-
if skipped {
85-
workflowCtx.UnsetCondition(api.ProjectCustomRolesReadyType)
86-
return workflow.OK()
87-
}
88-
8965
lastAppliedCustomRoles, err := getLastAppliedCustomRoles(project)
9066
if err != nil {
9167
return workflow.Terminate(workflow.Internal, err)

‎internal/controller/atlasproject/custom_roles_test.go

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -21,57 +21,6 @@ import (
2121
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/pointer"
2222
)
2323

24-
func TestShouldCustomRolesSkipReconciling(t *testing.T) {
25-
tests := []struct {
26-
name string
27-
annotations map[string]string
28-
expected bool
29-
shouldFail bool
30-
}{
31-
{
32-
name: "No annotations present",
33-
annotations: map[string]string{},
34-
expected: false,
35-
shouldFail: false,
36-
},
37-
{
38-
name: "Annotation present but invalid JSON",
39-
annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "invalid"},
40-
expected: false,
41-
shouldFail: true,
42-
},
43-
{
44-
name: "Annotation present with empty CustomRoles",
45-
annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "{\"CustomRoles\": []}"},
46-
expected: true,
47-
shouldFail: false,
48-
},
49-
{
50-
name: "Annotation present with non-empty CustomRoles",
51-
annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "{\"CustomRoles\": [{\"Name\": \"role1\"}]}"},
52-
expected: false,
53-
shouldFail: false,
54-
},
55-
}
56-
57-
for _, tt := range tests {
58-
t.Run(tt.name, func(t *testing.T) {
59-
atlasProject := &akov2.AtlasProject{
60-
ObjectMeta: metav1.ObjectMeta{
61-
Annotations: tt.annotations,
62-
},
63-
}
64-
result, err := shouldCustomRolesSkipReconciling(atlasProject)
65-
if tt.shouldFail {
66-
assert.Error(t, err)
67-
} else {
68-
assert.NoError(t, err)
69-
}
70-
assert.Equal(t, tt.expected, result)
71-
})
72-
}
73-
}
74-
7524
func TestEnsureCustomRoles(t *testing.T) {
7625
testRole := []akov2.CustomRole{
7726
{

‎internal/controller/atlasproject/ipaccess_list.go

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
package atlasproject
22

33
import (
4-
"encoding/json"
54
"errors"
65
"fmt"
76
"reflect"
87

98
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api"
109
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1"
1110
"github.com/mongodb/mongodb-atlas-kubernetes/v2/api/v1/status"
12-
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/customresource"
1311
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/controller/workflow"
1412
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/translation/ipaccesslist"
1513
)
@@ -130,18 +128,6 @@ func handleIPAccessList(ctx *workflow.Context, project *akov2.AtlasProject) work
130128
ctx.Log.Debug("starting ip access list processing")
131129
defer ctx.Log.Debug("finished ip access list processing")
132130

133-
skipped, err := shouldIPAccessListSkipReconciliation(project)
134-
if err != nil {
135-
return workflow.Terminate(workflow.Internal, err)
136-
}
137-
138-
if skipped {
139-
ctx.EnsureStatusOption(status.AtlasProjectExpiredIPAccessOption(nil))
140-
ctx.UnsetCondition(api.IPAccessListReadyType)
141-
142-
return workflow.OK()
143-
}
144-
145131
lastApplied, err := mapLastAppliedIPAccessList(project)
146132
if err != nil {
147133
return workflow.Terminate(workflow.Internal, fmt.Errorf("failed to get last applied configuration: %w", err))
@@ -157,20 +143,6 @@ func handleIPAccessList(ctx *workflow.Context, project *akov2.AtlasProject) work
157143
return c.reconcile()
158144
}
159145

160-
func shouldIPAccessListSkipReconciliation(atlasProject *akov2.AtlasProject) (bool, error) {
161-
lastSkippedSpec := akov2.AtlasProjectSpec{}
162-
lastSkippedSpecString, ok := atlasProject.Annotations[customresource.AnnotationLastSkippedConfiguration]
163-
if ok {
164-
if err := json.Unmarshal([]byte(lastSkippedSpecString), &lastSkippedSpec); err != nil {
165-
return false, fmt.Errorf("failed to parse last skipped configuration: %w", err)
166-
}
167-
168-
return len(lastSkippedSpec.ProjectIPAccessList) == 0, nil
169-
}
170-
171-
return false, nil
172-
}
173-
174146
func mapLastAppliedIPAccessList(atlasProject *akov2.AtlasProject) (ipaccesslist.IPAccessEntries, error) {
175147
lastApplied, err := lastAppliedSpecFrom(atlasProject)
176148
if err != nil {

0 commit comments

Comments
 (0)