Skip to content

Commit d9963bc

Browse files
igor-karpukhinigor-karpukhin
authored
CLOUDP-239182: Ensure teams are always cleaned up (#1481)
Ensure teams are always cleaned up
1 parent 5bb45a4 commit d9963bc

File tree

6 files changed

+259
-11
lines changed

6 files changed

+259
-11
lines changed

‎pkg/controller/atlasproject/atlasproject_controller.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (r *AtlasProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
177177

178178
workflowCtx.EnsureStatusOption(status.AtlasProjectIDOption(projectID))
179179

180-
if result = r.ensureDeletionFinalizer(workflowCtx, atlasClient, project); !result.IsOk() {
180+
if result = r.handleDeletion(workflowCtx, atlasClient, project); !result.IsOk() {
181181
setCondition(workflowCtx, status.ProjectReadyType, result)
182182
return result.ReconcileResult(), nil
183183
}
@@ -228,7 +228,7 @@ func (r *AtlasProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
228228
return workflow.OK().ReconcileResult(), nil
229229
}
230230

231-
func (r *AtlasProjectReconciler) ensureDeletionFinalizer(workflowCtx *workflow.Context, atlasClient *mongodbatlas.Client, project *akov2.AtlasProject) (result workflow.Result) {
231+
func (r *AtlasProjectReconciler) handleDeletion(workflowCtx *workflow.Context, atlasClient *mongodbatlas.Client, project *akov2.AtlasProject) (result workflow.Result) {
232232
log := workflowCtx.Log
233233

234234
if project.GetDeletionTimestamp().IsZero() {
@@ -255,6 +255,12 @@ func (r *AtlasProjectReconciler) ensureDeletionFinalizer(workflowCtx *workflow.C
255255
return result
256256
}
257257

258+
err := r.syncAssignedTeams(workflowCtx, project.ID(), project, nil)
259+
if err != nil {
260+
workflowCtx.SetConditionFalse(status.ProjectTeamsReadyType)
261+
return workflow.Terminate(workflow.TeamNotCleaned, err.Error())
262+
}
263+
258264
if err := r.deleteAtlasProject(workflowCtx.Context, atlasClient, project); err != nil {
259265
result = workflow.Terminate(workflow.Internal, err.Error())
260266
setCondition(workflowCtx, status.DeploymentReadyType, result)

‎pkg/controller/atlasproject/atlasproject_controller_test.go‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
package atlasproject
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/mock"
9+
"go.mongodb.org/atlas-sdk/v20231115008/admin"
10+
"go.mongodb.org/atlas-sdk/v20231115008/mockadmin"
11+
"go.mongodb.org/atlas/mongodbatlas"
12+
"go.uber.org/zap"
13+
corev1 "k8s.io/api/core/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
"sigs.k8s.io/controller-runtime/pkg/client"
17+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
18+
19+
atlasmocks "github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas"
20+
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
21+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status"
22+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/customresource"
23+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/watch"
24+
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/workflow"
25+
)
26+
27+
func TestAtlasProjectReconciler_handleDeletion(t *testing.T) {
28+
t.Run("Should delete team from Atlas when AtlasProject with finalizer is deleted", func(t *testing.T) {
29+
sch := runtime.NewScheme()
30+
akov2.AddToScheme(sch)
31+
corev1.AddToScheme(sch)
32+
deletionTS := metav1.Now()
33+
34+
testTeam := &akov2.AtlasTeam{
35+
TypeMeta: metav1.TypeMeta{},
36+
ObjectMeta: metav1.ObjectMeta{},
37+
Spec: akov2.TeamSpec{
38+
Name: "teamName",
39+
},
40+
Status: status.TeamStatus{
41+
ID: "teamID",
42+
Projects: []status.TeamProject{
43+
{
44+
ID: "projectID",
45+
Name: "testProject",
46+
},
47+
},
48+
},
49+
}
50+
testProject := &akov2.AtlasProject{
51+
TypeMeta: metav1.TypeMeta{
52+
Kind: "AtlasProject",
53+
APIVersion: "atlas.mongodb.com/v1",
54+
},
55+
ObjectMeta: metav1.ObjectMeta{
56+
Name: "testProject",
57+
Namespace: "default",
58+
DeletionTimestamp: &deletionTS,
59+
Finalizers: []string{customresource.FinalizerLabel},
60+
},
61+
Spec: akov2.AtlasProjectSpec{},
62+
Status: status.AtlasProjectStatus{
63+
ID: "projectID",
64+
Teams: []status.ProjectTeamStatus{
65+
{
66+
ID: testTeam.Status.ID,
67+
},
68+
},
69+
},
70+
}
71+
72+
k8sClient := fake.NewClientBuilder().
73+
WithScheme(sch).
74+
WithObjects(testProject, testTeam).Build()
75+
76+
teamsMock := &atlasmocks.TeamsClientMock{
77+
RemoveTeamFromOrganizationFunc: func(orgID string, teamID string) (*mongodbatlas.Response, error) {
78+
return nil, nil
79+
},
80+
RemoveTeamFromOrganizationRequests: map[string]struct{}{},
81+
ListFunc: func(orgID string) ([]mongodbatlas.Team, *mongodbatlas.Response, error) {
82+
return []mongodbatlas.Team{
83+
{
84+
ID: testTeam.Status.ID,
85+
Name: testTeam.Name,
86+
Usernames: nil,
87+
},
88+
}, nil, nil
89+
},
90+
RemoveTeamFromProjectFunc: func(projectID string, teamID string) (*mongodbatlas.Response, error) {
91+
return nil, nil
92+
},
93+
}
94+
95+
atlasClient := mongodbatlas.Client{
96+
Projects: &atlasmocks.ProjectsClientMock{
97+
GetProjectTeamsAssignedFunc: func(projectID string) (*mongodbatlas.TeamsAssigned, *mongodbatlas.Response, error) {
98+
return &mongodbatlas.TeamsAssigned{
99+
Links: nil,
100+
Results: []*mongodbatlas.Result{
101+
{
102+
Links: nil,
103+
RoleNames: nil,
104+
TeamID: testTeam.Status.ID,
105+
},
106+
},
107+
TotalCount: 0,
108+
}, nil, nil
109+
},
110+
DeleteFunc: func(projectID string) (*mongodbatlas.Response, error) {
111+
return nil, nil
112+
},
113+
},
114+
Teams: teamsMock,
115+
}
116+
reconciler := AtlasProjectReconciler{
117+
Client: k8sClient,
118+
ResourceWatcher: watch.ResourceWatcher{},
119+
Log: zap.S(),
120+
Scheme: sch,
121+
ObjectDeletionProtection: false,
122+
SubObjectDeletionProtection: false,
123+
AtlasProvider: &atlasmocks.TestProvider{
124+
ClientFunc: func(secretRef *client.ObjectKey, log *zap.SugaredLogger) (*mongodbatlas.Client, string, error) {
125+
return &atlasClient, "123", nil
126+
},
127+
},
128+
}
129+
130+
mockPrivateEndpointAPI := mockadmin.NewPrivateEndpointServicesApi(t)
131+
mockPrivateEndpointAPI.EXPECT().
132+
ListPrivateEndpointServices(mock.Anything, mock.Anything, mock.Anything).
133+
Return(admin.ListPrivateEndpointServicesApiRequest{ApiService: mockPrivateEndpointAPI})
134+
mockPrivateEndpointAPI.EXPECT().
135+
ListPrivateEndpointServicesExecute(admin.ListPrivateEndpointServicesApiRequest{ApiService: mockPrivateEndpointAPI}).
136+
Return([]admin.EndpointService{}, nil, nil)
137+
138+
mockPeeringEndpointAPI := mockadmin.NewNetworkPeeringApi(t)
139+
mockPeeringEndpointAPI.EXPECT().ListPeeringConnectionsWithParams(mock.Anything, mock.Anything).
140+
Return(admin.ListPeeringConnectionsApiRequest{ApiService: mockPeeringEndpointAPI})
141+
mockPeeringEndpointAPI.EXPECT().
142+
ListPeeringConnectionsExecute(admin.ListPeeringConnectionsApiRequest{ApiService: mockPeeringEndpointAPI}).
143+
Return(&admin.PaginatedContainerPeer{}, nil, nil)
144+
145+
workflowCtx := &workflow.Context{
146+
Client: &atlasClient,
147+
Context: context.Background(),
148+
SdkClient: &admin.APIClient{
149+
PrivateEndpointServicesApi: mockPrivateEndpointAPI,
150+
NetworkPeeringApi: mockPeeringEndpointAPI,
151+
},
152+
Log: zap.S(),
153+
}
154+
workflowResult := reconciler.handleDeletion(workflowCtx, workflowCtx.Client, testProject)
155+
assert.True(t, workflowResult.IsOk())
156+
assert.Len(t, teamsMock.RemoveTeamFromOrganizationRequests, 1)
157+
})
158+
}

‎pkg/controller/atlasproject/teams.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type TeamDataContainer struct {
3232

3333
func (r *AtlasProjectReconciler) ensureAssignedTeams(workflowCtx *workflow.Context, project *akov2.AtlasProject, protected bool) workflow.Result {
3434
resourcesToWatch := make([]watch.WatchedObject, 0, len(project.Spec.Teams))
35+
3536
defer func() {
3637
workflowCtx.AddResourcesToWatch(resourcesToWatch...)
3738
r.Log.Debugf("watching team resources: %v\r\n", r.WatchedResources)
@@ -43,7 +44,6 @@ func (r *AtlasProjectReconciler) ensureAssignedTeams(workflowCtx *workflow.Conte
4344

4445
if assignedTeam.TeamRef.Name == "" {
4546
workflowCtx.Log.Warnf("missing team name. skipping assignment for entry %v", assignedTeam)
46-
4747
continue
4848
}
4949

‎pkg/controller/atlasproject/teams_test.go‎

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

8+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
9+
810
"go.uber.org/zap"
911
"sigs.k8s.io/controller-runtime/pkg/client"
1012

@@ -16,7 +18,6 @@ import (
1618
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1719
"k8s.io/apimachinery/pkg/runtime"
1820
"k8s.io/apimachinery/pkg/types"
19-
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2021

2122
"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas"
2223
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
@@ -269,13 +270,21 @@ func TestEnsureAssignedTeams(t *testing.T) {
269270
akoProject := &akov2.AtlasProject{}
270271
akoProject.WithAnnotations(map[string]string{customresource.AnnotationLastAppliedConfiguration: "{}"})
271272
logger := zaptest.NewLogger(t).Sugar()
273+
274+
testScheme := runtime.NewScheme()
275+
akov2.AddToScheme(testScheme)
276+
k8sClient := fake.NewClientBuilder().
277+
WithScheme(testScheme).
278+
Build()
279+
272280
workflowCtx := &workflow.Context{
273281
Client: &atlasClient,
274282
Log: logger,
275283
Context: context.Background(),
276284
}
277285
reconciler := &AtlasProjectReconciler{
278-
Log: logger,
286+
Log: logger,
287+
Client: k8sClient,
279288
}
280289
result := reconciler.ensureAssignedTeams(workflowCtx, akoProject, true)
281290

@@ -303,8 +312,8 @@ func TestEnsureAssignedTeams(t *testing.T) {
303312
}
304313

305314
testScheme := runtime.NewScheme()
306-
testScheme.AddKnownTypes(akov2.GroupVersion, &akov2.AtlasProject{})
307-
testScheme.AddKnownTypes(akov2.GroupVersion, &akov2.AtlasTeam{})
315+
akov2.AddToScheme(testScheme)
316+
corev1.AddToScheme(testScheme)
308317
k8sClient := fake.NewClientBuilder().
309318
WithScheme(testScheme).
310319
WithObjects(team1, team2).
@@ -370,9 +379,8 @@ func TestUpdateTeamState(t *testing.T) {
370379
Log: logger,
371380
}
372381
testScheme := runtime.NewScheme()
373-
testScheme.AddKnownTypes(akov2.GroupVersion, &akov2.AtlasProject{})
374-
testScheme.AddKnownTypes(akov2.GroupVersion, &akov2.AtlasTeam{})
375-
testScheme.AddKnownTypes(akov2.GroupVersion, &corev1.Secret{})
382+
akov2.AddToScheme(testScheme)
383+
corev1.AddToScheme(testScheme)
376384
secret := &corev1.Secret{
377385
ObjectMeta: metav1.ObjectMeta{
378386
Name: "my-secret",
@@ -437,4 +445,78 @@ func TestUpdateTeamState(t *testing.T) {
437445
k8sClient.Get(context.Background(), types.NamespacedName{Name: team.ObjectMeta.Name, Namespace: team.ObjectMeta.Namespace}, team)
438446
assert.Equal(t, 1, len(team.Status.Projects))
439447
})
448+
449+
t.Run("must remove a team from Atlas is a team is unassigned", func(t *testing.T) {
450+
logger := zaptest.NewLogger(t).Sugar()
451+
workflowCtx := &workflow.Context{
452+
Context: context.Background(),
453+
Log: logger,
454+
}
455+
testScheme := runtime.NewScheme()
456+
akov2.AddToScheme(testScheme)
457+
corev1.AddToScheme(testScheme)
458+
secret := &corev1.Secret{
459+
ObjectMeta: metav1.ObjectMeta{
460+
Name: "my-secret",
461+
},
462+
Data: map[string][]byte{
463+
"orgId": []byte("0987654321"),
464+
"publicApiKey": []byte("api-pub-key"),
465+
"privateApiKey": []byte("api-priv-key"),
466+
},
467+
Type: "Opaque",
468+
}
469+
project := &akov2.AtlasProject{
470+
Spec: akov2.AtlasProjectSpec{
471+
Name: "projectName",
472+
ConnectionSecret: &common.ResourceRefNamespaced{
473+
Name: "my-secret",
474+
},
475+
},
476+
Status: status.AtlasProjectStatus{
477+
ID: "projectID",
478+
},
479+
}
480+
team := &akov2.AtlasTeam{
481+
ObjectMeta: metav1.ObjectMeta{
482+
Name: "testTeam",
483+
Namespace: "testNS",
484+
},
485+
Status: status.TeamStatus{
486+
ID: "testTeamStatus",
487+
Projects: []status.TeamProject{},
488+
},
489+
}
490+
teamsMock := &atlas.TeamsClientMock{
491+
RemoveTeamFromOrganizationFunc: func(orgID string, teamID string) (*mongodbatlas.Response, error) {
492+
return nil, nil
493+
},
494+
RemoveTeamFromOrganizationRequests: map[string]struct{}{},
495+
}
496+
atlasProvider := &atlas.TestProvider{
497+
ClientFunc: func(secretRef *client.ObjectKey, log *zap.SugaredLogger) (*mongodbatlas.Client, string, error) {
498+
return &mongodbatlas.Client{
499+
Teams: teamsMock,
500+
}, "0987654321", nil
501+
},
502+
}
503+
k8sClient := fake.NewClientBuilder().
504+
WithScheme(testScheme).
505+
WithObjects(secret, project, team).
506+
Build()
507+
reconciler := &AtlasProjectReconciler{
508+
Client: k8sClient,
509+
Log: logger,
510+
AtlasProvider: atlasProvider,
511+
}
512+
teamRef := &common.ResourceRefNamespaced{
513+
Name: team.Name,
514+
Namespace: "testNS",
515+
}
516+
517+
err := reconciler.updateTeamState(workflowCtx, project, teamRef, true)
518+
assert.NoError(t, err)
519+
k8sClient.Get(context.Background(), types.NamespacedName{Name: team.ObjectMeta.Name, Namespace: team.ObjectMeta.Namespace}, team)
520+
assert.Len(t, teamsMock.RemoveTeamFromOrganizationRequests, 1)
521+
})
440522
}

‎pkg/controller/workflow/reason.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ const (
8888
TeamInvalidSpec ConditionReason = "TeamInvalidSpec"
8989
TeamUsersNotReady ConditionReason = "TeamUsersNotReady"
9090
TeamDoesNotExist ConditionReason = "TeamDoesNotExist"
91+
TeamNotCleaned ConditionReason = "TeamCleanupFailed"
9192
)
9293

9394
// Atlas Federated Auth reasons

‎test/e2e/teams_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ func projectTeamsFlow(userData *model.TestDataProvider, teams []akov2.Team) {
128128
func ensureTeamsStatus(g Gomega, testData model.TestDataProvider, teams []akov2.Team, check func(res *akov2.AtlasTeam) bool) bool {
129129
for _, team := range teams {
130130
resource := &akov2.AtlasTeam{}
131-
g.Expect(testData.K8SClient.Get(testData.Context, types.NamespacedName{Name: team.TeamRef.Name, Namespace: testData.Resources.Namespace}, resource)).Should(Succeed())
131+
g.Expect(testData.K8SClient.Get(testData.Context,
132+
types.NamespacedName{Name: team.TeamRef.Name, Namespace: testData.Resources.Namespace}, resource)).Should(Succeed())
132133

133134
if !check(resource) {
134135
return false

0 commit comments

Comments
 (0)