Skip to content

Commit

Permalink
Fix Service NodePort conflicts when running on large cluster
Browse files Browse the repository at this point in the history
  • Loading branch information
@jgwest
jgwest committed Jul 30, 2024
1 parent 192dd2c commit 1a931a5
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 18 deletions.
30 changes: 21 additions & 9 deletions tests/e2e/cluster-scoped/cluster_scoped_rollouts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {

Context("Testing cluster-scoped RolloutManager behaviour", func() {

// Use slightly different NodePorts from default, to avoid conflicting with any other Services using NodePorts on the cluster. This is not an issue when running E2E tests via GitHub actions, but is more likely to be an issue when running against, e.g. a large cluster like default OpenShift.
const (
testServiceNodePort_31000 = 31130
testServiceNodePort_31001 = 31131
testServiceNodePort_31002 = 31132

testServiceNodePort_32000 = 32230
testServiceNodePort_32001 = 32231
testServiceNodePort_32002 = 32232
testServiceNodePort_32003 = 32233
)

var (
err error
ctx context.Context
Expand Down Expand Up @@ -81,7 +93,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Expect(utils.CreateNamespace(ctx, k8sClient, nsName)).To(Succeed())

By("Create and validate rollouts.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, testServiceNodePort_31000, testServiceNodePort_32000)
})

/*
Expand Down Expand Up @@ -115,13 +127,13 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Expect(utils.CreateNamespace(ctx, k8sClient, nsName2)).To(Succeed())

By("1st Namespace: Create and validate Rollout in 1st namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, testServiceNodePort_31000, testServiceNodePort_32000)

By("2nd Namespace: Create a another namespace for 2nd Rollout.")
Expect(utils.CreateNamespace(ctx, k8sClient, nsName3)).To(Succeed())

By("2nd Namespace: Create and validate Rollout in 2nd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName3, 31001, 32002)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName3, testServiceNodePort_31001, testServiceNodePort_32002)
})

/*
Expand All @@ -147,7 +159,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Eventually(rolloutsManagerCl, "1m", "1s").Should(rmFixture.HaveSuccessCondition())

By("1st RM: Create and validate Rollout in 1st namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, testServiceNodePort_31000, testServiceNodePort_32000)

By("2nd RM: Create 2nd namespace.")
Expect(utils.CreateNamespace(ctx, k8sClient, nsName1)).To(Succeed())
Expand All @@ -169,7 +181,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
}))

By("2nd RM: Create and validate Rollout in 2nd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, 31001, 32002)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, testServiceNodePort_31001, testServiceNodePort_32002)

By("1st RM: Update cluster-scoped RolloutManager, after reconciliation it should still work.")
err = k8s.UpdateWithoutConflict(ctx, &rolloutsManagerCl, k8sClient, func(obj client.Object) {
Expand All @@ -190,7 +202,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Expect(utils.CreateNamespace(ctx, k8sClient, nsName2)).To(Succeed())

By("2nd RM: Create and validate Rollout in 3rd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, 31002, 32003)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, testServiceNodePort_31002, testServiceNodePort_32003)
})

/*
Expand Down Expand Up @@ -218,7 +230,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Eventually(rolloutsManagerCl, "1m", "1s").Should(rmFixture.HaveSuccessCondition())

By("1st RM: Create and validate Rollout in 1st namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, testServiceNodePort_31000, testServiceNodePort_32000)

By("2nd RM: Create 2nd namespace.")
Expect(utils.CreateNamespace(ctx, k8sClient, nsName1)).To(Succeed())
Expand All @@ -240,7 +252,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
}))

By("2nd RM: Create and validate Rollout in 2nd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, 31001, 32001)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, testServiceNodePort_31001, testServiceNodePort_32001)

By("1st RM: Update first RolloutManager, after reconciliation it should also stop working.")
err = k8s.UpdateWithoutConflict(ctx, &rolloutsManagerCl, k8sClient, func(obj client.Object) {
Expand All @@ -267,7 +279,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Expect(utils.CreateNamespace(ctx, k8sClient, nsName2)).To(Succeed())

By("1st RM: Create and validate Rollout in 3rd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, 31002, 32002)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, testServiceNodePort_31002, testServiceNodePort_32002)
})

It("After creating 2 cluster-scoped RolloutManager in a namespace, delete 1st RolloutManager and verify it removes the Failed status of 2nd RolloutManager", func() {
Expand Down
29 changes: 20 additions & 9 deletions tests/e2e/namespace-scoped/namespace_scoped_rollouts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {

Context("Testing namespace-scoped RolloutManager behaviour", func() {

// Use slightly different NodePorts from default, to avoid conflicting with any other Services using NodePorts on the cluster. This is not an issue when running E2E tests via GitHub actions, but is more likely to be an issue when running against, e.g. a large cluster like default OpenShift.
const (
testServiceNodePort_31000 = 31130
testServiceNodePort_31001 = 31131
testServiceNodePort_31002 = 31132

testServiceNodePort_32000 = 32230
testServiceNodePort_32001 = 32231
testServiceNodePort_32002 = 32232
testServiceNodePort_32003 = 32_233
)
var (
err error
ctx context.Context
Expand Down Expand Up @@ -73,7 +84,7 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
By("Verify argo Rollouts controller is able to reconcile CR.")

By("Create and validate rollouts.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, testServiceNodePort_31000, testServiceNodePort_32000)
})

/*
Expand All @@ -100,7 +111,7 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
By("1st RM: Verify argo Rollouts controller of 1st namespace is able to reconcile CR created in 1st namespace.")

By("1st RM: Create and validate rollouts.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, testServiceNodePort_31000, testServiceNodePort_32000)

By("2nd RM: Create 2nd namespace.")
Expect(utils.CreateNamespace(ctx, k8sClient, nsName1)).To(Succeed())
Expand All @@ -118,7 +129,7 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
By("2nd RM: Verify argo Rollouts controller of 2nd namespace is able to reconcile CR created in 2nd namespace.")

By("2nd RM: Create and validate rollouts in 2nd namespace.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, 31001, 32001)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName1, testServiceNodePort_31001, testServiceNodePort_32001)

By("1st RM: Update 1st RolloutManager, after reconciliation it should still work.")
err = k8s.UpdateWithoutConflict(ctx, &rolloutsManagerNs1, k8sClient, func(obj client.Object) {
Expand Down Expand Up @@ -197,11 +208,11 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
Expect(utils.CreateNamespace(ctx, k8sClient, nsName2)).To(Succeed())

By("2nd NS: Create active and preview services required for Rollout CR in 2nd namespace.")
rolloutServiceActive, err := utils.CreateService(ctx, k8sClient, utils.RolloutsActiveServiceName, nsName2, 31000)
rolloutServiceActive, err := utils.CreateService(ctx, k8sClient, utils.RolloutsActiveServiceName, nsName2, testServiceNodePort_31000)
Expect(err).ToNot(HaveOccurred())
Eventually(&rolloutServiceActive, "10s", "1s").Should(k8s.ExistByName(k8sClient))

rolloutServicePreview, err := utils.CreateService(ctx, k8sClient, utils.RolloutsPreviewServiceName, nsName2, 32000)
rolloutServicePreview, err := utils.CreateService(ctx, k8sClient, utils.RolloutsPreviewServiceName, nsName2, testServiceNodePort_32000)
Expect(err).ToNot(HaveOccurred())
Eventually(&rolloutServicePreview, "10s", "1s").Should(k8s.ExistByName(k8sClient))

Expand Down Expand Up @@ -280,16 +291,16 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
Eventually(rolloutsManagerNs, "1m", "1s").Should(rmFixture.HaveSuccessCondition())

By("1st RM: Create Rollout CR in 1st namespace and ensure it is reconciled.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, fixture.TestE2ENamespace, testServiceNodePort_31000, testServiceNodePort_32000)

By("2nd RM: Create Rollout in 2nd namespace and it should not be reconciled as 2nd RolloutManager failed.")

By("2nd RM: Create active and preview services in 2nd namespace.")
rolloutServiceActive, err := utils.CreateService(ctx, k8sClient, utils.RolloutsActiveServiceName, nsName, 31001)
rolloutServiceActive, err := utils.CreateService(ctx, k8sClient, utils.RolloutsActiveServiceName, nsName, testServiceNodePort_31001)
Expect(err).ToNot(HaveOccurred())
Eventually(&rolloutServiceActive, "10s", "1s").Should(k8s.ExistByName(k8sClient))

rolloutServicePreview, err := utils.CreateService(ctx, k8sClient, utils.RolloutsPreviewServiceName, nsName, 32002)
rolloutServicePreview, err := utils.CreateService(ctx, k8sClient, utils.RolloutsPreviewServiceName, nsName, testServiceNodePort_32002)
Expect(err).ToNot(HaveOccurred())
Eventually(&rolloutServicePreview, "10s", "1s").Should(k8s.ExistByName(k8sClient))

Expand Down Expand Up @@ -344,7 +355,7 @@ var _ = Describe("Namespace-scoped RolloutManager tests", func() {
By("Verify argo Rollouts controller is able to reconcile CR.")

By("Create and validate rollouts.")
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, 31000, 32000)
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName, testServiceNodePort_31000, testServiceNodePort_32000)
})
})
})

0 comments on commit 1a931a5

Please sign in to comment.