Skip to content

Commit 8093227

Browse files
helderjshelderjs
authored
return errors occurring on teams to reflect dry run events (#2268)
1 parent 366c8f1 commit 8093227

File tree

2 files changed

+156
-38
lines changed

2 files changed

+156
-38
lines changed

‎internal/controller/atlasproject/teams.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
package atlasproject
1616

1717
import (
18+
"errors"
19+
1820
"k8s.io/apimachinery/pkg/types"
1921
controllerruntime "sigs.k8s.io/controller-runtime"
2022

@@ -94,6 +96,7 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, teamsS
9496
}
9597

9698
defer statushandler.Update(ctx, r.Client, r.EventRecorder, project)
99+
var teamErrors error
97100

98101
toDelete := make([]*teams.AssignedTeam, 0, len(atlasAssignedTeams))
99102

@@ -119,16 +122,18 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, teamsS
119122
}
120123

121124
ctx.Log.Debugf("removing team %s from project for later update", atlasAssignedTeam.TeamID)
122-
err := teamsService.Unassign(ctx.Context, projectID, atlasAssignedTeam.TeamID)
125+
err = teamsService.Unassign(ctx.Context, projectID, atlasAssignedTeam.TeamID)
123126
if err != nil {
127+
teamErrors = errors.Join(teamErrors, err)
124128
ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.TeamID, err.Error())
125129
}
126130
}
127131

128132
for _, atlasAssignedTeam := range toDelete {
129133
ctx.Log.Debugf("removing team %s from project", atlasAssignedTeam.TeamID)
130-
err := teamsService.Unassign(ctx.Context, projectID, atlasAssignedTeam.TeamID)
134+
err = teamsService.Unassign(ctx.Context, projectID, atlasAssignedTeam.TeamID)
131135
if err != nil {
136+
teamErrors = errors.Join(teamErrors, err)
132137
ctx.Log.Warnf("failed to remove team %s from project: %s", atlasAssignedTeam.TeamID, err.Error())
133138
}
134139

@@ -137,6 +142,7 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, teamsS
137142
ctx.Log.Warnf("unable to find team %s status in the project", atlasAssignedTeam.TeamID)
138143
} else {
139144
if err = r.updateTeamState(ctx, project, teamRef, true); err != nil {
145+
teamErrors = errors.Join(teamErrors, err)
140146
ctx.Log.Warnf("failed to update team %s status with removed project: %s", atlasAssignedTeam.TeamID, err.Error())
141147
}
142148
}
@@ -157,13 +163,14 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, teamsS
157163
}
158164

159165
if err = r.updateTeamState(ctx, project, &teamsToAssign[teamID].TeamRef, false); err != nil {
166+
teamErrors = errors.Join(teamErrors, err)
160167
ctx.Log.Warnf("failed to update team %s status with added project: %s", teamID, err.Error())
161168
}
162169
}
163170

164171
err = teamsService.Assign(ctx.Context, &projectTeams, projectID)
165172
if err != nil {
166-
return err
173+
return errors.Join(teamErrors, err)
167174
}
168175
}
169176

@@ -173,7 +180,7 @@ func (r *AtlasProjectReconciler) syncAssignedTeams(ctx *workflow.Context, teamsS
173180

174181
ctx.EnsureStatusOption(status.AtlasProjectSetTeamsOption(&projectTeamStatus))
175182

176-
return nil
183+
return teamErrors
177184
}
178185

179186
func (r *AtlasProjectReconciler) updateTeamState(ctx *workflow.Context, project *akov2.AtlasProject, teamRef *common.ResourceRefNamespaced, isRemoval bool) error {

‎internal/controller/atlasproject/teams_test.go‎

Lines changed: 145 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ package atlasproject
1616

1717
import (
1818
"context"
19+
"errors"
1920
"testing"
2021

2122
"github.com/stretchr/testify/assert"
22-
"github.com/stretchr/testify/mock"
2323
"go.mongodb.org/atlas-sdk/v20231115008/admin"
2424
"go.uber.org/zap/zaptest"
2525
corev1 "k8s.io/api/core/v1"
@@ -39,10 +39,116 @@ import (
3939
)
4040

4141
func TestSyncAssignedTeams(t *testing.T) {
42+
ctx := context.Background()
43+
4244
tests := map[string]struct {
43-
teamsToAssign map[string]*akov2.Team
44-
expectedErr error
45+
teamsToAssign map[string]*akov2.Team
46+
teamServiceMock func() teams.TeamsService
47+
expectedErr error
4548
}{
49+
"should error to list teams": {
50+
teamServiceMock: func() teams.TeamsService {
51+
s := translation.NewTeamsServiceMock(t)
52+
s.EXPECT().
53+
ListProjectTeams(ctx, "projectID").
54+
Return(nil, errors.New("error to list teams"))
55+
56+
return s
57+
},
58+
expectedErr: errors.New("error to list teams"),
59+
},
60+
"should error to unassign teams": {
61+
teamServiceMock: func() teams.TeamsService {
62+
s := translation.NewTeamsServiceMock(t)
63+
s.EXPECT().ListProjectTeams(ctx, "projectID").Return([]teams.AssignedTeam{
64+
{
65+
Roles: []string{"GROUP_OWNER"},
66+
TeamID: "teamID_1",
67+
TeamName: "teamName_1",
68+
},
69+
{
70+
Roles: []string{"GROUP_OWNER"},
71+
TeamID: "teamID_2",
72+
TeamName: "teamName_2",
73+
},
74+
{
75+
Roles: []string{"GROUP_READ_ONLY"},
76+
TeamID: "teamID_3",
77+
TeamName: "teamName_3",
78+
},
79+
}, nil)
80+
s.EXPECT().Unassign(ctx, "projectID", "teamID_1").Return(errors.New("error to unassign team 1"))
81+
s.EXPECT().Unassign(ctx, "projectID", "teamID_2").Return(errors.New("error to unassign team 2"))
82+
s.EXPECT().Unassign(ctx, "projectID", "teamID_3").Return(errors.New("error to unassign team 3"))
83+
84+
return s
85+
},
86+
expectedErr: errors.Join(
87+
errors.Join(
88+
errors.Join(
89+
nil,
90+
errors.New("error to unassign team 1"),
91+
),
92+
errors.New("error to unassign team 2"),
93+
),
94+
errors.New("error to unassign team 3"),
95+
),
96+
},
97+
"should error to assign teams": {
98+
teamsToAssign: map[string]*akov2.Team{
99+
"teamID_1": {
100+
TeamRef: common.ResourceRefNamespaced{
101+
Name: "teamName_1",
102+
},
103+
Roles: []akov2.TeamRole{"GROUP_OWNER"},
104+
},
105+
"teamID_2": {
106+
TeamRef: common.ResourceRefNamespaced{
107+
Name: "teamName_2",
108+
},
109+
Roles: []akov2.TeamRole{"GROUP_READ_ONLY"},
110+
},
111+
},
112+
teamServiceMock: func() teams.TeamsService {
113+
s := translation.NewTeamsServiceMock(t)
114+
s.EXPECT().ListProjectTeams(ctx, "projectID").Return([]teams.AssignedTeam{
115+
{
116+
Roles: []string{"GROUP_OWNER"},
117+
TeamID: "teamID_1",
118+
TeamName: "teamName_1",
119+
},
120+
{
121+
Roles: []string{"GROUP_OWNER"},
122+
TeamID: "teamID_2",
123+
TeamName: "teamName_2",
124+
},
125+
{
126+
Roles: []string{"GROUP_READ_ONLY"},
127+
TeamID: "teamID_3",
128+
TeamName: "teamName_3",
129+
},
130+
}, nil)
131+
s.EXPECT().Unassign(ctx, "projectID", "teamID_2").Return(nil)
132+
s.EXPECT().Unassign(ctx, "projectID", "teamID_3").Return(nil)
133+
134+
s.EXPECT().Assign(
135+
ctx,
136+
&[]teams.AssignedTeam{
137+
{
138+
Roles: []string{"GROUP_READ_ONLY"},
139+
TeamID: "teamID_2",
140+
},
141+
},
142+
"projectID",
143+
).Return(errors.New("error to assign team 2"))
144+
145+
return s
146+
},
147+
expectedErr: errors.Join(
148+
nil,
149+
errors.New("error to assign team 2"),
150+
),
151+
},
46152
"should sync teams assigned": {
47153
teamsToAssign: map[string]*akov2.Team{
48154
"teamID_1": {
@@ -58,6 +164,41 @@ func TestSyncAssignedTeams(t *testing.T) {
58164
Roles: []akov2.TeamRole{"GROUP_READ_ONLY"},
59165
},
60166
},
167+
teamServiceMock: func() teams.TeamsService {
168+
s := translation.NewTeamsServiceMock(t)
169+
s.EXPECT().ListProjectTeams(ctx, "projectID").Return([]teams.AssignedTeam{
170+
{
171+
Roles: []string{"GROUP_OWNER"},
172+
TeamID: "teamID_1",
173+
TeamName: "teamName_1",
174+
},
175+
{
176+
Roles: []string{"GROUP_OWNER"},
177+
TeamID: "teamID_2",
178+
TeamName: "teamName_2",
179+
},
180+
{
181+
Roles: []string{"GROUP_READ_ONLY"},
182+
TeamID: "teamID_3",
183+
TeamName: "teamName_3",
184+
},
185+
}, nil)
186+
s.EXPECT().Unassign(ctx, "projectID", "teamID_2").Return(nil)
187+
s.EXPECT().Unassign(ctx, "projectID", "teamID_3").Return(nil)
188+
s.EXPECT().Assign(
189+
ctx,
190+
&[]teams.AssignedTeam{
191+
{
192+
Roles: []string{"GROUP_READ_ONLY"},
193+
TeamID: "teamID_2",
194+
},
195+
},
196+
"projectID",
197+
).
198+
Return(nil)
199+
200+
return s
201+
},
61202
},
62203
}
63204

@@ -174,42 +315,12 @@ func TestSyncAssignedTeams(t *testing.T) {
174315
},
175316
Context: context.Background(),
176317
}
177-
teamService := func() teams.TeamsService {
178-
service := translation.NewTeamsServiceMock(t)
179-
service.EXPECT().ListProjectTeams(mock.Anything, "projectID").Return([]teams.AssignedTeam{
180-
{
181-
Roles: []string{"GROUP_OWNER"},
182-
TeamID: team1.Status.ID,
183-
TeamName: "teamName_1",
184-
},
185-
{
186-
Roles: []string{"GROUP_OWNER"},
187-
TeamID: team2.Status.ID,
188-
TeamName: "teamName_2",
189-
},
190-
{
191-
Roles: []string{"GROUP_READ_ONLY"},
192-
TeamID: team3.Status.ID,
193-
TeamName: "teamName_3",
194-
},
195-
}, nil)
196-
service.EXPECT().Unassign(mock.Anything, "projectID", "teamID_2").Return(nil)
197-
service.EXPECT().Unassign(mock.Anything, "projectID", "teamID_3").Return(nil)
198-
service.EXPECT().Assign(mock.Anything,
199-
&[]teams.AssignedTeam{
200-
{
201-
Roles: []string{"GROUP_READ_ONLY"},
202-
TeamID: "teamID_2",
203-
},
204-
}, "projectID").Return(nil)
205-
return service
206-
}
207318
r := &AtlasProjectReconciler{
208319
Client: k8sClient,
209320
EventRecorder: record.NewFakeRecorder(10),
210321
Log: logger,
211322
}
212-
err := r.syncAssignedTeams(ctx, teamService(), "projectID", project, tt.teamsToAssign)
323+
err := r.syncAssignedTeams(ctx, tt.teamServiceMock(), "projectID", project, tt.teamsToAssign)
213324
assert.Equal(t, tt.expectedErr, err)
214325
})
215326
}

0 commit comments

Comments
 (0)