Skip to content

Commit e107958

Browse files
Sugar-packSugar-pack
authored
CLOUDP-151638: Fix network peer container handling (#820)
* Fix network peer container handling * Make custom roles e2e test more stable
1 parent bddeefb commit e107958

File tree

11 files changed

+104
-50
lines changed

11 files changed

+104
-50
lines changed

‎.github/workflows/test-e2e.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,11 @@ jobs:
7676
- name: Change path for the test
7777
run: |
7878
awk '{gsub(/cloud.mongodb.com/, "cloud-qa.mongodb.com", $0); print}' bundle/manifests/mongodb-atlas-kubernetes.clusterserviceversion.yaml > tmp && mv tmp bundle/manifests/mongodb-atlas-kubernetes.clusterserviceversion.yaml
79-
- name: Cache all files
79+
- name: Cache repo files
8080
uses: actions/cache@v3
8181
with:
82-
path: ./*
82+
path: |
83+
./*
8384
key: ${{ github.sha }}
8485
- name: Prepare docker tag
8586
id: prepare-docker-bundle-tag
@@ -137,7 +138,7 @@ jobs:
137138
"teams",
138139
]
139140
steps:
140-
- name: Cache all files
141+
- name: Get repo files from cache
141142
uses: actions/cache@v3
142143
with:
143144
path: ./*

‎pkg/controller/atlasproject/network_peering.go‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/provider"
1414
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
1515
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/workflow"
16+
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util"
1617
)
1718

1819
const (
@@ -39,7 +40,7 @@ func ensureNetworkPeers(ctx *workflow.Context, groupID string, project *mdbv1.At
3940
return result
4041
}
4142
ctx.SetConditionTrue(status.NetworkPeerReadyType)
42-
if len(networkPeerStatus) == 0 && len(networkPeerSpec) == 0 {
43+
if len(networkPeerSpec) == 0 {
4344
ctx.UnsetCondition(status.NetworkPeerReadyType)
4445
}
4546

@@ -69,7 +70,7 @@ func SyncNetworkPeer(context context.Context, ctx *workflow.Context, groupID str
6970
logger := ctx.Log
7071
mongoClient := ctx.Client
7172
logger.Debugf("syncing network peers for project %v", groupID)
72-
list, err := getAllExistedNetworkPeer(context, logger, mongoClient.Peers, groupID)
73+
list, err := GetAllExistedNetworkPeer(context, mongoClient.Peers, groupID)
7374
if err != nil {
7475
logger.Errorf("failed to get all network peers: %v", err)
7576
return workflow.Terminate(workflow.ProjectNetworkPeerIsNotReadyInAtlas, "failed to get all network peers"),
@@ -97,10 +98,16 @@ func SyncNetworkPeer(context context.Context, ctx *workflow.Context, groupID str
9798
peerStatuses = createNetworkPeers(context, mongoClient, groupID, diff.PeersToCreate, logger)
9899
peerStatuses, err = UpdateStatuses(context, mongoClient.Containers, peerStatuses, diff.PeersToUpdate, groupID, logger)
99100
if err != nil {
101+
logger.Errorf("failed to update network peer statuses: %v", err)
100102
return workflow.Terminate(workflow.ProjectNetworkPeerIsNotReadyInAtlas,
101103
"failed to update network peer statuses"), status.NetworkPeerReadyType
102104
}
103-
105+
err = deleteUnusedContainers(context, mongoClient.Containers, groupID, getPeerIDs(peerStatuses))
106+
if err != nil {
107+
logger.Errorf("failed to delete unused containers: %v", err)
108+
return workflow.Terminate(workflow.ProjectNetworkPeerIsNotReadyInAtlas,
109+
fmt.Sprintf("failed to delete unused containers: %s", err)), status.NetworkPeerReadyType
110+
}
104111
return ensurePeerStatus(peerStatuses, len(peerSpecs), logger)
105112
}
106113

@@ -124,6 +131,30 @@ func UpdateStatuses(context context.Context, containerService mongodbatlas.Conta
124131
return peerStatuses, nil
125132
}
126133

134+
func getPeerIDs(statuses []status.AtlasNetworkPeer) []string {
135+
ids := make([]string, 0, len(statuses))
136+
for _, networkPeer := range statuses {
137+
ids = append(ids, networkPeer.ContainerID)
138+
}
139+
return ids
140+
}
141+
142+
func deleteUnusedContainers(context context.Context, containerService mongodbatlas.ContainersService, groupID string, doNotDelete []string) error {
143+
containers, _, err := containerService.List(context, groupID, nil)
144+
if err != nil {
145+
return err
146+
}
147+
for _, container := range containers {
148+
if !util.Contains(doNotDelete, container.ID) {
149+
response, errDelete := containerService.Delete(context, groupID, container.ID)
150+
if errDelete != nil && response.StatusCode != http.StatusConflict { // AWS peer does not contain container id
151+
return errDelete
152+
}
153+
}
154+
}
155+
return nil
156+
}
157+
127158
func getContainer(context context.Context, containerService mongodbatlas.ContainersService,
128159
peerToUpdate mongodbatlas.Peer, groupID string, logger *zap.SugaredLogger) (mongodbatlas.Container, error) {
129160
var container mongodbatlas.Container
@@ -260,34 +291,28 @@ func createNetworkPeers(context context.Context, mongoClient mongodbatlas.Client
260291
return newPeerStatuses
261292
}
262293

263-
func getAllExistedNetworkPeer(ctx context.Context, logger *zap.SugaredLogger, peerService mongodbatlas.PeersService, groupID string) ([]mongodbatlas.Peer, error) {
294+
func GetAllExistedNetworkPeer(ctx context.Context, peerService mongodbatlas.PeersService, groupID string) ([]mongodbatlas.Peer, error) {
264295
var peersList []mongodbatlas.Peer
265296
listAWS, _, err := peerService.List(ctx, groupID, &mongodbatlas.ContainersListOptions{})
266297
if err != nil {
267-
logger.Errorf("failed to list network peers: %v", err)
268-
return nil, err
298+
return nil, fmt.Errorf("failed to list network peers for AWS: %w", err)
269299
}
270-
logger.Debugf("got %d aws peers", len(listAWS))
271300
peersList = append(peersList, listAWS...)
272301

273302
listGCP, _, err := peerService.List(ctx, groupID, &mongodbatlas.ContainersListOptions{
274303
ProviderName: string(provider.ProviderGCP),
275304
})
276305
if err != nil {
277-
logger.Errorf("failed to list network peers: %v", err)
278-
return nil, err
306+
return nil, fmt.Errorf("failed to list network peers for GCP: %w", err)
279307
}
280-
logger.Debugf("got %d gcp peers", len(listGCP))
281308
peersList = append(peersList, listGCP...)
282309

283310
listAzure, _, err := peerService.List(ctx, groupID, &mongodbatlas.ContainersListOptions{
284311
ProviderName: string(provider.ProviderAzure),
285312
})
286313
if err != nil {
287-
logger.Errorf("failed to list network peers: %v", err)
288-
return nil, err
314+
return nil, fmt.Errorf("failed to list network peers for Azure: %w", err)
289315
}
290-
logger.Debugf("got %d azure peers", len(listAzure))
291316
peersList = append(peersList, listAzure...)
292317
return peersList, nil
293318
}
@@ -548,7 +573,7 @@ func DeleteAllNetworkPeers(ctx context.Context, groupID string, service mongodba
548573
}
549574

550575
func deleteAllNetworkPeers(ctx context.Context, groupID string, service mongodbatlas.PeersService, logger *zap.SugaredLogger) error {
551-
peers, err := getAllExistedNetworkPeer(ctx, logger, service, groupID)
576+
peers, err := GetAllExistedNetworkPeer(ctx, service, groupID)
552577
if err != nil {
553578
logger.Errorf("failed to list network peers for project %s: %v", groupID, err)
554579
return err

‎pkg/util/comparation.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,12 @@ func PtrValuesEqual[T comparable](a, b *T) bool {
2525
}
2626
return *a == *b
2727
}
28+
29+
func Contains[T comparable](a []T, b T) bool {
30+
for _, item := range a {
31+
if item == b {
32+
return true
33+
}
34+
}
35+
return false
36+
}

‎test/e2e/actions/conditions.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,35 @@ import (
44
"fmt"
55
"time"
66

7-
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
8-
9-
v1 "k8s.io/api/core/v1"
10-
117
. "github.com/onsi/gomega"
12-
138
"github.com/onsi/gomega/types"
9+
v1 "k8s.io/api/core/v1"
1410

15-
kube "github.com/mongodb/mongodb-atlas-kubernetes/test/e2e/actions/kube"
11+
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
12+
"github.com/mongodb/mongodb-atlas-kubernetes/test/e2e/actions/kube"
1613
"github.com/mongodb/mongodb-atlas-kubernetes/test/e2e/model"
1714
)
1815

19-
func WaitForConditionsToBecomeTrue(userData *model.TestDataProvider, conditonTypes ...status.ConditionType) {
20-
Eventually(allConditionsAreTrueFunc(userData, conditonTypes...)).
16+
func WaitForConditionsToBecomeTrue(userData *model.TestDataProvider, conditionTypes ...status.ConditionType) {
17+
Eventually(allConditionsAreTrueFunc(userData, conditionTypes...)).
2118
WithTimeout(15*time.Minute).WithPolling(20*time.Second).
22-
Should(BeTrue(), fmt.Sprintf("Status conditions %v are not all 'True'", conditonTypes))
19+
Should(BeTrue(), fmt.Sprintf("Status conditions %v are not all 'True'", conditionTypes))
2320
}
2421

25-
// CheckConditionsNotSet wait for Ready condition to become true and checks that input conditions are unset
26-
func CheckConditionsNotSet(userData *model.TestDataProvider, conditonTypes ...status.ConditionType) {
27-
Eventually(conditionsAreUnset(userData, conditonTypes...)).
22+
// CheckProjectConditionsNotSet wait for Ready condition to become true and checks that input conditions are unset
23+
func CheckProjectConditionsNotSet(userData *model.TestDataProvider, conditionTypes ...status.ConditionType) {
24+
Eventually(conditionsAreUnset(userData, conditionTypes...)).
2825
WithTimeout(15*time.Minute).WithPolling(20*time.Second).
29-
Should(BeTrue(), fmt.Sprintf("Status conditions %v should be unset", conditonTypes))
26+
Should(BeTrue(), fmt.Sprintf("Status conditions %v should be unset. project status: %v",
27+
conditionTypes, userData.Project.Status.Conditions))
3028
}
3129

32-
func allConditionsAreTrueFunc(userData *model.TestDataProvider, conditonTypes ...status.ConditionType) func(g types.Gomega) bool {
30+
func allConditionsAreTrueFunc(userData *model.TestDataProvider, conditionTypes ...status.ConditionType) func(g types.Gomega) bool {
3331
return func(g Gomega) bool {
3432
conditions, err := kube.GetAllProjectConditions(userData)
3533
g.Expect(err).ShouldNot(HaveOccurred())
3634

37-
for _, conditionType := range conditonTypes {
35+
for _, conditionType := range conditionTypes {
3836
foundTrue := false
3937
for _, condition := range conditions {
4038
if condition.Type == conditionType && condition.Status == v1.ConditionTrue {
@@ -52,7 +50,7 @@ func allConditionsAreTrueFunc(userData *model.TestDataProvider, conditonTypes ..
5250
}
5351
}
5452

55-
func conditionsAreUnset(userData *model.TestDataProvider, unsetConditonTypes ...status.ConditionType) func(g types.Gomega) bool {
53+
func conditionsAreUnset(userData *model.TestDataProvider, unsetConditionTypes ...status.ConditionType) func(g types.Gomega) bool {
5654
return func(g Gomega) bool {
5755
conditions, err := kube.GetAllProjectConditions(userData)
5856
g.Expect(err).ShouldNot(HaveOccurred())
@@ -70,7 +68,7 @@ func conditionsAreUnset(userData *model.TestDataProvider, unsetConditonTypes ...
7068
}
7169

7270
for _, condition := range conditions {
73-
for _, unsetConditionType := range unsetConditonTypes {
71+
for _, unsetConditionType := range unsetConditionTypes {
7472
if condition.Type == unsetConditionType {
7573
return false
7674
}

‎test/e2e/auditing_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func auditingFlow(userData *model.TestDataProvider, auditing *v1.Auditing) {
6868
By("Remove Auditing from the project", func() {
6969
userData.Project.Spec.Auditing = nil
7070
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
71-
actions.CheckConditionsNotSet(userData, status.AuditingReadyType)
71+
actions.CheckProjectConditionsNotSet(userData, status.AuditingReadyType)
7272
})
7373
}
7474

‎test/e2e/custom_roles_test.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,23 @@ func projectCustomRolesFlow(userData *model.TestDataProvider, customRoles []v1.C
125125
})
126126

127127
By("Remove one Custom Roles from the project", func() {
128-
Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name,
129-
Namespace: userData.Project.Namespace}, userData.Project)).Should(Succeed())
130-
131-
cr := userData.Project.Spec.CustomRoles
132-
userData.Project.Spec.CustomRoles = cr[:1]
133-
134-
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
128+
Eventually(func(g Gomega) {
129+
g.Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name,
130+
Namespace: userData.Project.Namespace}, userData.Project)).Should(Succeed())
131+
cr := userData.Project.Spec.CustomRoles
132+
userData.Project.Spec.CustomRoles = cr[:1]
133+
g.Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
134+
}).Should(Succeed())
135135
actions.WaitForConditionsToBecomeTrue(userData, status.ProjectCustomRolesReadyType, status.ReadyType)
136136
})
137137

138138
By("Remove all Custom Roles from the project", func() {
139-
userData.Project.Spec.CustomRoles = nil
140-
141-
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
142-
actions.CheckConditionsNotSet(userData, status.ProjectCustomRolesReadyType)
139+
Eventually(func(g Gomega) {
140+
g.Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name,
141+
Namespace: userData.Project.Namespace}, userData.Project)).Should(Succeed())
142+
userData.Project.Spec.CustomRoles = nil
143+
g.Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
144+
}).Should(Succeed())
145+
actions.CheckProjectConditionsNotSet(userData, status.ProjectCustomRolesReadyType)
143146
})
144147
}

‎test/e2e/encryption_at_rest_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func encryptionAtRestFlow(userData *model.TestDataProvider, encAtRest v1.Encrypt
132132
})
133133

134134
By("Check if project returned back to the initial state", func() {
135-
actions.CheckConditionsNotSet(userData, status.EncryptionAtRestReadyType)
135+
actions.CheckProjectConditionsNotSet(userData, status.EncryptionAtRestReadyType)
136136

137137
Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name,
138138
Namespace: userData.Resources.Namespace}, userData.Project)).Should(Succeed())

‎test/e2e/integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func integrationCycle(data *model.TestDataProvider, key string) {
113113
Namespace: data.Resources.Namespace}, data.Project)).Should(Succeed())
114114
data.Project.Spec.Integrations = []project.Integration{}
115115
Expect(data.K8SClient.Update(data.Context, data.Project)).Should(Succeed())
116-
actions.CheckConditionsNotSet(data, status.IntegrationReadyType)
116+
actions.CheckProjectConditionsNotSet(data, status.IntegrationReadyType)
117117
})
118118

119119
By("Delete integration check", func() {

‎test/e2e/network_peering_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"testing"
77
"time"
88

9+
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/atlasproject"
10+
911
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
1012

1113
"github.com/mongodb/mongodb-atlas-kubernetes/test/e2e/data"
@@ -229,6 +231,22 @@ func networkPeerFlow(userData *model.TestDataProvider, peers []v1.NetworkPeer) {
229231
Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name, Namespace: userData.Project.Namespace}, userData.Project)).Should(Succeed())
230232
Expect(userData.Project.Status.NetworkPeers).Should(HaveLen(len(peers)))
231233
})
234+
235+
By("Delete network peers", func() {
236+
Expect(userData.K8SClient.Get(userData.Context, types.NamespacedName{Name: userData.Project.Name,
237+
Namespace: userData.Project.Namespace}, userData.Project)).Should(Succeed())
238+
userData.Project.Spec.NetworkPeers = nil
239+
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
240+
actions.CheckProjectConditionsNotSet(userData, status.NetworkPeerReadyType)
241+
Eventually(func(g Gomega) {
242+
atlasPeers, err := atlasproject.GetAllExistedNetworkPeer(userData.Context, atlasClient.Client.Peers, userData.Project.ID())
243+
g.Expect(err).ToNot(HaveOccurred())
244+
g.Expect(atlasPeers).To(BeEmpty(), "All network peers should be deleted")
245+
containers, _, err := atlasClient.Client.Containers.ListAll(userData.Context, userData.Project.Status.ID, nil)
246+
g.Expect(err).ToNot(HaveOccurred())
247+
g.Expect(containers).To(BeEmpty(), "All containers should be deleted")
248+
})
249+
})
232250
}
233251

234252
func EnsurePeersReadyToConnect(g Gomega, userData *model.TestDataProvider, lenOfSpec int) bool {

‎test/e2e/project_settings_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,6 @@ func projectSettingsFlow(userData *model.TestDataProvider, settings *v1.ProjectS
7070
By("Remove Project Settings from the project", func() {
7171
userData.Project.Spec.Settings = nil
7272
Expect(userData.K8SClient.Update(userData.Context, userData.Project)).Should(Succeed())
73-
actions.CheckConditionsNotSet(userData, status.ProjectSettingsReadyType)
73+
actions.CheckProjectConditionsNotSet(userData, status.ProjectSettingsReadyType)
7474
})
7575
}

0 commit comments

Comments
 (0)