Skip to content

Commit

Permalink
feat: support N nginx ingresses (#2467)
Browse files Browse the repository at this point in the history 
* add additionalStableIngresses datatype

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add validation for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add rollout controller logic for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add nginx ingress logic for setting weight & fetching names for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add nginx testing for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add ingress & controller tests for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>

* remove validation_references_test.go snafu

Signed-off-by: Travis Perdue <[email protected]>

* add ingress util testing for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>

* update test pattern

Signed-off-by: Travis Perdue <[email protected]>

* go fmt

Signed-off-by: Travis Perdue <[email protected]>

* make codegen

Signed-off-by: Travis Perdue <[email protected]>

* improve testing, and fix validation error reporting for when there is more than 1 Nginix Ingress Controller

Signed-off-by: Travis Perdue <[email protected]>

* fix bug

Signed-off-by: Travis Perdue <[email protected]>

* update logging

Signed-off-by: Travis Perdue <[email protected]>

* add validation for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add nginx testing for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>

* add ingress & controller tests for additionalStableIngresses

Signed-off-by: Travis Perdue <[email protected]>

* make codegen

Signed-off-by: Travis Perdue <[email protected]>

* fix rebase issues

Signed-off-by: Travis Perdue <[email protected]>

* deduplicate tests

Signed-off-by: Travis Perdue <[email protected]>

* DRY up nginx_text.go

Signed-off-by: Travis Perdue <[email protected]>

* DRY more code

Signed-off-by: Travis Perdue <[email protected]>

* dry up code

Signed-off-by: Travis Perdue <[email protected]>

* address sonarcloud code smells & duplicate code

Signed-off-by: Travis Perdue <[email protected]>

* change additionalStableIngresses to stableIngresses

Signed-off-by: Travis Perdue <[email protected]>

* update validation

Signed-off-by: Travis Perdue <[email protected]>

* reason only about stableIngress or StableIngresses, not both

Signed-off-by: Travis Perdue <[email protected]>

* update rollout controller tests

Signed-off-by: Travis Perdue <[email protected]>

* update validation_references tests

Signed-off-by: Travis Perdue <[email protected]>

* fix nginx bug & tests

Signed-off-by: Travis Perdue <[email protected]>

* fix ingress_test

Signed-off-by: Travis Perdue <[email protected]>

* update specs

Signed-off-by: Travis Perdue <[email protected]>

* update nginx md

Signed-off-by: Travis Perdue <[email protected]>

* make file/test specific variables private

Signed-off-by: Travis Perdue <[email protected]>

* make codegen

Signed-off-by: Travis Perdue <[email protected]>

---------

Signed-off-by: Travis Perdue <[email protected]>
Signed-off-by: Travis Perdue <[email protected]>
  • Loading branch information
@tperdue321
tperdue321 authored Mar 24, 2023
1 parent 874c188 commit 630212f
Show file tree
Hide file tree
Showing 22 changed files with 2,065 additions and 1,154 deletions.
7 changes: 6 additions & 1 deletion docs/features/specification.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,12 @@ spec:

# NGINX Ingress Controller routing configuration
nginx:
stableIngress: primary-ingress # required
# Either stableIngress or stableIngresses must be configured, but not both.
stableIngress: primary-ingress
stableIngresses:
- primary-ingress
- secondary-ingress
- tertiary-ingress
annotationPrefix: customingress.nginx.ingress.kubernetes.io # optional
additionalIngressAnnotations: # optional
canary-by-header: X-Canary
Expand Down
13 changes: 11 additions & 2 deletions docs/features/traffic-management/nginx.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ spec:
stableService: stable-service # required
trafficRouting:
nginx:
stableIngress: primary-ingress # required
# Either stableIngress or stableIngresses must be configured, but not both.
stableIngress: primary-ingress
stableIngresses:
- primary-ingress
- secondary-ingress
- tertiary-ingress
annotationPrefix: customingress.nginx.ingress.kubernetes.io # optional
additionalIngressAnnotations: # optional
canary-by-header: X-Canary
Expand All @@ -38,7 +43,11 @@ The controller routes traffic to the canary Service by creating a second Ingress
Since the Nginx Ingress controller allows users to configure the annotation prefix used by the Ingress controller, Rollouts can specify the optional `annotationPrefix` field. The canary Ingress uses that prefix instead of the default `nginx.ingress.kubernetes.io` if the field set.


## Using Argo Rollouts with multiple NGINX ingress controllers
## Using Argo Rollouts with multiple NGINX ingress controllers per service
Starting with v1.5, argo rollouts supports multiple Nginx ingress controllers pointing at one service with canary deployments. If only one ingress controller is needed, utilize the existing key `stableIngress`. If multiple ingress controllers are needed (e.g., separating internal vs external traffic), use the key `stableIngresses` instead. It takes an array of string values that are the names of the ingress controllers. Canary steps are applied identically across all ingress controllers.


## Using Argo Rollouts with custom NGINX ingress controller names
As a default, the Argo Rollouts controller only operates on ingresses with the `kubernetes.io/ingress.class` annotation or `spec.ingressClassName` set to `nginx`. A user can configure the controller to operate on Ingresses with different class name by specifying the `--nginx-ingress-classes` flag. A user can list the `--nginx-ingress-classes` flag multiple times if the Argo Rollouts controller should operate on multiple values. This solves the case where a cluster has multiple Ingress controllers operating on different class values.

If the user would like the controller to operate on any Ingress without the `kubernetes.io/ingress.class` annotation or `spec.ingressClassName`, a user should add the following `--nginx-ingress-classes ''`.
243 changes: 189 additions & 54 deletions ingress/ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ingress

import (
"context"
"fmt"
"sync"
"testing"
"time"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/stretchr/testify/assert"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
kubeinformers "k8s.io/client-go/informers"
k8sfake "k8s.io/client-go/kubernetes/fake"
Expand All @@ -24,6 +26,15 @@ import (
"k8s.io/client-go/tools/cache"
)

const stableService string = "test-stable-service"
const additionalIngress string = "test-stable-ingress-additional"
const stableIngress string = "test-stable-ingress"

func testString(val string) string {
return fmt.Sprintf("default/%s", val)

}

func newNginxIngress(name string, port int, serviceName string) *extensionsv1beta1.Ingress {
class := "nginx"
return &extensionsv1beta1.Ingress{
Expand Down Expand Up @@ -87,15 +98,29 @@ func newNginxIngressWithAnnotation(name string, port int, serviceName string) *e
}
}

func newFakeIngressControllerMultiIngress(t *testing.T, ing []*extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
return underlyingControllerBuilder(t, ing, rollout)
}

func newFakeIngressController(t *testing.T, ing *extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
return underlyingControllerBuilder(t, []*extensionsv1beta1.Ingress{ing}, rollout)
}

func underlyingControllerBuilder(t *testing.T, ing []*extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
t.Helper()
client := fake.NewSimpleClientset()
if rollout != nil {
client = fake.NewSimpleClientset(rollout)
}
kubeclient := k8sfake.NewSimpleClientset()
if ing != nil {
kubeclient = k8sfake.NewSimpleClientset(ing)
var x []runtime.Object
for _, i := range ing {
if i != nil {
x = append(x, i)
}
}
kubeclient = k8sfake.NewSimpleClientset(x...)
}
i := informers.NewSharedInformerFactory(client, 0)
k8sI := kubeinformers.NewSharedInformerFactory(kubeclient, 0)
Expand Down Expand Up @@ -140,7 +165,11 @@ func newFakeIngressController(t *testing.T, ing *extensionsv1beta1.Ingress, roll
}

if ing != nil {
k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(ing)
for _, i := range ing {
if i != nil {
k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i)
}
}
}
if rollout != nil {
i.Argoproj().V1alpha1().Rollouts().Informer().GetIndexer().Add(rollout)
Expand All @@ -156,78 +185,184 @@ func TestSyncMissingIngress(t *testing.T) {
}

func TestSyncIngressNotReferencedByRollout(t *testing.T) {
ing := newNginxIngress("test-stable-ingress", 80, "test-stable-service")

ctrl, kubeclient, _ := newFakeIngressController(t, ing, nil)
tests := []struct {
ings []*extensionsv1beta1.Ingress
name string
keys []string
}{
{
[]*extensionsv1beta1.Ingress{
newNginxIngress(stableIngress, 80, stableService),
},
"Single Ingress",
[]string{
testString(stableIngress),
},
},
{
[]*extensionsv1beta1.Ingress{
newNginxIngress(stableIngress, 80, stableService),
newNginxIngress(additionalIngress, 80, stableService),
},
"Multi Ingress",
[]string{
testString(stableIngress),
testString(additionalIngress),
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {

err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
ctrl, kubeclient, _ := newFakeIngressControllerMultiIngress(t, test.ings, nil)
for _, key := range test.keys {
err := ctrl.syncIngress(context.Background(), key)
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
}
})
}
}

func TestSyncIngressReferencedByRollout(t *testing.T) {
ing := newNginxIngress("test-stable-ingress", 80, "stable-service")

rollout := &v1alpha1.Rollout{
ObjectMeta: metav1.ObjectMeta{
Name: "rollout",
Namespace: metav1.NamespaceDefault,
tests := []struct {
ings []*extensionsv1beta1.Ingress
name string
keys []string
additionalIngressNames []string
}{
{
[]*extensionsv1beta1.Ingress{
newNginxIngress(stableIngress, 80, stableService),
},
"Single Ingress",
[]string{
testString(stableIngress),
},
[]string{},
},
Spec: v1alpha1.RolloutSpec{
Strategy: v1alpha1.RolloutStrategy{
Canary: &v1alpha1.CanaryStrategy{
StableService: "stable-service",
CanaryService: "canary-service",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Nginx: &v1alpha1.NginxTrafficRouting{
StableIngress: "test-stable-ingress",
},
},
},
{
[]*extensionsv1beta1.Ingress{
newNginxIngress(stableIngress, 80, stableService),
newNginxIngress(additionalIngress, 80, stableService),
},
"Multi Ingress",
[]string{
testString(stableIngress),
testString(additionalIngress),
testString(additionalIngress),
},
[]string{additionalIngress},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {

ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
rollout := &v1alpha1.Rollout{
ObjectMeta: metav1.ObjectMeta{
Name: "rollout",
Namespace: metav1.NamespaceDefault,
},
Spec: v1alpha1.RolloutSpec{
Strategy: v1alpha1.RolloutStrategy{
Canary: &v1alpha1.CanaryStrategy{
StableService: stableService,
CanaryService: "canary-service",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Nginx: &v1alpha1.NginxTrafficRouting{
StableIngress: stableIngress,
StableIngresses: test.additionalIngressNames,
},
},
},
},
},
}

err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
assert.Equal(t, 1, enqueuedObjects["default/rollout"])
ctrl, kubeclient, enqueuedObjects := newFakeIngressControllerMultiIngress(t, test.ings, rollout)

for i, key := range test.keys {
err := ctrl.syncIngress(context.Background(), key)
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
assert.Equal(t, i+1, enqueuedObjects["default/rollout"])
}
})
}
}

func TestSkipIngressWithNoClass(t *testing.T) {
ing := newNginxIngressWithAnnotation("test-stable-ingress", 80, "stable-service")
ing.Annotations = nil
rollout := &v1alpha1.Rollout{
ObjectMeta: metav1.ObjectMeta{
Name: "rollout",
Namespace: metav1.NamespaceDefault,
tests := []struct {
ings []*extensionsv1beta1.Ingress
name string
keys []string
additionalIngressNames []string
}{
{
[]*extensionsv1beta1.Ingress{
newNginxIngressWithAnnotation(stableIngress, 80, stableService),
},
"Single Ingress",
[]string{
testString(stableIngress),
},
[]string{},
},
Spec: v1alpha1.RolloutSpec{
Strategy: v1alpha1.RolloutStrategy{
Canary: &v1alpha1.CanaryStrategy{
StableService: "stable-service",
CanaryService: "canary-service",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Nginx: &v1alpha1.NginxTrafficRouting{
StableIngress: "test-stable-ingress",
},
},
},
{
[]*extensionsv1beta1.Ingress{
newNginxIngressWithAnnotation(stableIngress, 80, stableService),
newNginxIngressWithAnnotation(additionalIngress, 80, stableService),
},
"Multi Ingress",
[]string{
testString(stableIngress),
testString(additionalIngress),
},
[]string{stableIngress},
},
}

ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
for _, i := range test.ings {
i.Annotations = nil
}

err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
assert.Len(t, enqueuedObjects, 0)
rollout := &v1alpha1.Rollout{
ObjectMeta: metav1.ObjectMeta{
Name: "rollout",
Namespace: metav1.NamespaceDefault,
},
Spec: v1alpha1.RolloutSpec{
Strategy: v1alpha1.RolloutStrategy{
Canary: &v1alpha1.CanaryStrategy{
StableService: stableService,
CanaryService: "canary-service",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Nginx: &v1alpha1.NginxTrafficRouting{
StableIngress: stableIngress,
StableIngresses: test.additionalIngressNames,
},
},
},
},
},
}

ctrl, kubeclient, enqueuedObjects := newFakeIngressControllerMultiIngress(t, test.ings, rollout)

for _, key := range test.keys {
err := ctrl.syncIngress(context.Background(), key)
assert.NoError(t, err)
actions := kubeclient.Actions()
assert.Len(t, actions, 0)
assert.Len(t, enqueuedObjects, 0)
}
})
}
}

func TestRun(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions manifests/crds/rollout-crd.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,8 +864,10 @@ spec:
type: string
stableIngress:
type: string
required:
- stableIngress
stableIngresses:
items:
type: string
type: array
type: object
plugins:
type: object
Expand Down
6 changes: 4 additions & 2 deletions manifests/install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11919,8 +11919,10 @@ spec:
type: string
stableIngress:
type: string
required:
- stableIngress
stableIngresses:
items:
type: string
type: array
type: object
plugins:
type: object
Expand Down
Empty file modified manifests/namespace-install.yaml
100755 → 100644
View file
Empty file.
7 changes: 7 additions & 0 deletions pkg/apiclient/rollout/rollout.swagger.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,13 @@
"type": "string"
},
"title": "+optional"
},
"stableIngresses": {
"type": "array",
"items": {
"type": "string"
},
"title": "StableIngresses refers to the names of `Ingress` resources in the same namespace as the `Rollout` in a multi ingress scenario\n+optional"
}
},
"title": "NginxTrafficRouting configuration for Nginx ingress controller to control traffic routing"
Expand Down
Loading

0 comments on commit 630212f

Please sign in to comment.