Skip to content

Commit 20637da

Browse files
danielm-codefreshdanielm-codefresh
authored
Merge pull request #35 from codefresh-io/CR-8836-sync-v1.1.0
Support Ingress from Networking API version + e2e fix
2 parents cac8a9f + 06e2c30 commit 20637da

38 files changed

+8089
-3179
lines changed

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ DEV_IMAGE=false
2121
# E2E variables
2222
E2E_INSTANCE_ID ?= argo-rollouts-e2e
2323
E2E_TEST_OPTIONS ?=
24-
E2E_PARALLEL ?= 1
24+
E2E_PARALLEL ?= 4
2525

2626
override LDFLAGS += \
2727
-X ${PACKAGE}/utils/version.version=${VERSION} \
@@ -244,7 +244,7 @@ start-e2e:
244244

245245
.PHONY: test-e2e
246246
test-e2e:
247-
go test -timeout 20m -v -count 1 --tags e2e -p ${E2E_PARALLEL} --short ./test/e2e ${E2E_TEST_OPTIONS}
247+
go test -timeout 30m -v -count 1 --tags e2e -p ${E2E_PARALLEL} --short ./test/e2e ${E2E_TEST_OPTIONS}
248248

249249
.PHONY: coverage
250250
coverage: test

‎cmd/rollouts-controller/main.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/argoproj/argo-rollouts/pkg/signals"
2727
controllerutil "github.com/argoproj/argo-rollouts/utils/controller"
2828
"github.com/argoproj/argo-rollouts/utils/defaults"
29+
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress"
2930
istioutil "github.com/argoproj/argo-rollouts/utils/istio"
3031
logutil "github.com/argoproj/argo-rollouts/utils/log"
3132
"github.com/argoproj/argo-rollouts/utils/tolerantinformer"
@@ -54,6 +55,7 @@ func newCommand() *cobra.Command {
5455
istioVersion string
5556
trafficSplitVersion string
5657
ambassadorVersion string
58+
ingressVersion string
5759
albIngressClasses []string
5860
nginxIngressClasses []string
5961
awsVerifyTargetGroup bool
@@ -143,6 +145,10 @@ func newCommand() *cobra.Command {
143145

144146
k8sRequestProvider := &metrics.K8sRequestsCountProvider{}
145147
kubeclientmetrics.AddMetricsTransportWrapper(config, k8sRequestProvider.IncKubernetesRequest)
148+
mode, err := ingressutil.DetermineIngressMode(ingressVersion, kubeClient.DiscoveryClient)
149+
checkError(err)
150+
ingressWrapper, err := ingressutil.NewIngressWrapper(mode, kubeClient, kubeInformerFactory)
151+
checkError(err)
146152

147153
cm := controller.NewManager(
148154
namespace,
@@ -153,7 +159,7 @@ func newCommand() *cobra.Command {
153159
discoveryClient,
154160
kubeInformerFactory.Apps().V1().ReplicaSets(),
155161
kubeInformerFactory.Core().V1().Services(),
156-
kubeInformerFactory.Extensions().V1beta1().Ingresses(),
162+
ingressWrapper,
157163
jobInformerFactory.Batch().V1().Jobs(),
158164
tolerantinformer.NewTolerantRolloutInformer(dynamicInformerFactory),
159165
tolerantinformer.NewTolerantExperimentInformer(dynamicInformerFactory),
@@ -211,6 +217,7 @@ func newCommand() *cobra.Command {
211217
command.Flags().StringVar(&istioVersion, "istio-api-version", defaults.DefaultIstioVersion, "Set the default Istio apiVersion that controller should look when manipulating VirtualServices.")
212218
command.Flags().StringVar(&ambassadorVersion, "ambassador-api-version", defaults.DefaultAmbassadorVersion, "Set the Ambassador apiVersion that controller should look when manipulating Ambassador Mappings.")
213219
command.Flags().StringVar(&trafficSplitVersion, "traffic-split-api-version", defaults.DefaultSMITrafficSplitVersion, "Set the default TrafficSplit apiVersion that controller uses when creating TrafficSplits.")
220+
command.Flags().StringVar(&ingressVersion, "ingress-api-version", "", "Set the Ingress apiVersion that the controller should use.")
214221
command.Flags().StringArrayVar(&albIngressClasses, "alb-ingress-classes", defaultALBIngressClass, "Defines all the ingress class annotations that the alb ingress controller operates on. Defaults to alb")
215222
command.Flags().StringArrayVar(&nginxIngressClasses, "nginx-ingress-classes", defaultNGINXIngressClass, "Defines all the ingress class annotations that the nginx ingress controller operates on. Defaults to nginx")
216223
command.Flags().BoolVar(&awsVerifyTargetGroup, "alb-verify-weight", false, "Verify ALB target group weights before progressing through steps (requires AWS privileges)")

‎controller/controller.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
appsinformers "k8s.io/client-go/informers/apps/v1"
2121
batchinformers "k8s.io/client-go/informers/batch/v1"
2222
coreinformers "k8s.io/client-go/informers/core/v1"
23-
extensionsinformers "k8s.io/client-go/informers/extensions/v1beta1"
2423
"k8s.io/client-go/kubernetes"
2524
"k8s.io/client-go/kubernetes/scheme"
2625
"k8s.io/client-go/tools/cache"
@@ -37,6 +36,7 @@ import (
3736
"github.com/argoproj/argo-rollouts/rollout"
3837
"github.com/argoproj/argo-rollouts/service"
3938
"github.com/argoproj/argo-rollouts/utils/defaults"
39+
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress"
4040
"github.com/argoproj/argo-rollouts/utils/queue"
4141
"github.com/argoproj/argo-rollouts/utils/record"
4242
)
@@ -107,7 +107,7 @@ func NewManager(
107107
discoveryClient discovery.DiscoveryInterface,
108108
replicaSetInformer appsinformers.ReplicaSetInformer,
109109
servicesInformer coreinformers.ServiceInformer,
110-
ingressesInformer extensionsinformers.IngressInformer,
110+
ingressWrap *ingressutil.IngressWrap,
111111
jobInformer batchinformers.JobInformer,
112112
rolloutsInformer informers.RolloutInformer,
113113
experimentsInformer informers.ExperimentInformer,
@@ -181,7 +181,7 @@ func NewManager(
181181
IstioDestinationRuleInformer: istioDestinationRuleInformer,
182182
ReplicaSetInformer: replicaSetInformer,
183183
ServicesInformer: servicesInformer,
184-
IngressInformer: ingressesInformer,
184+
IngressWrapper: ingressWrap,
185185
RolloutsInformer: rolloutsInformer,
186186
ResyncPeriod: resyncPeriod,
187187
RolloutWorkQueue: rolloutWorkqueue,
@@ -231,7 +231,7 @@ func NewManager(
231231

232232
ingressController := ingress.NewController(ingress.ControllerConfig{
233233
Client: kubeclientset,
234-
IngressInformer: ingressesInformer,
234+
IngressWrap: ingressWrap,
235235
IngressWorkQueue: ingressWorkqueue,
236236

237237
RolloutsInformer: rolloutsInformer,
@@ -247,7 +247,7 @@ func NewManager(
247247
metricsServer: metricsServer,
248248
rolloutSynced: rolloutsInformer.Informer().HasSynced,
249249
serviceSynced: servicesInformer.Informer().HasSynced,
250-
ingressSynced: ingressesInformer.Informer().HasSynced,
250+
ingressSynced: ingressWrap.HasSynced,
251251
jobSynced: jobInformer.Informer().HasSynced,
252252
experimentSynced: experimentsInformer.Informer().HasSynced,
253253
analysisRunSynced: analysisRunInformer.Informer().HasSynced,

‎ingress/alb.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import (
77
"strings"
88

99
log "github.com/sirupsen/logrus"
10-
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
11-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1210
"k8s.io/utils/pointer"
1311

1412
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -17,9 +15,10 @@ import (
1715
logutil "github.com/argoproj/argo-rollouts/utils/log"
1816
)
1917

20-
func (c *Controller) syncALBIngress(ingress *extensionsv1beta1.Ingress, rollouts []*v1alpha1.Rollout) error {
18+
func (c *Controller) syncALBIngress(ingress *ingressutil.Ingress, rollouts []*v1alpha1.Rollout) error {
2119
ctx := context.TODO()
22-
managedActions, err := ingressutil.NewManagedALBActions(ingress.Annotations[ingressutil.ManagedActionsAnnotation])
20+
annotations := ingress.GetAnnotations()
21+
managedActions, err := ingressutil.NewManagedALBActions(annotations[ingressutil.ManagedActionsAnnotation])
2322
if err != nil {
2423
return nil
2524
}
@@ -40,32 +39,40 @@ func (c *Controller) syncALBIngress(ingress *extensionsv1beta1.Ingress, rollouts
4039
delete(managedActions, roName)
4140
resetALBAction, err := getResetALBActionStr(ingress, actionKey)
4241
if err != nil {
43-
log.WithField(logutil.RolloutKey, roName).WithField(logutil.IngressKey, ingress.Name).WithField(logutil.NamespaceKey, ingress.Namespace).Error(err)
42+
log.WithField(logutil.RolloutKey, roName).
43+
WithField(logutil.IngressKey, ingress.GetName()).
44+
WithField(logutil.NamespaceKey, ingress.GetNamespace()).
45+
Error(err)
4446
return nil
4547
}
46-
newIngress.Annotations[actionKey] = resetALBAction
48+
annotations := newIngress.GetAnnotations()
49+
annotations[actionKey] = resetALBAction
50+
newIngress.SetAnnotations(annotations)
4751
}
4852
}
4953
if !modified {
5054
return nil
5155
}
5256
newManagedStr := managedActions.String()
53-
newIngress.Annotations[ingressutil.ManagedActionsAnnotation] = newManagedStr
57+
newAnnotations := newIngress.GetAnnotations()
58+
newAnnotations[ingressutil.ManagedActionsAnnotation] = newManagedStr
59+
newIngress.SetAnnotations(newAnnotations)
5460
if newManagedStr == "" {
55-
delete(newIngress.Annotations, ingressutil.ManagedActionsAnnotation)
61+
delete(newIngress.GetAnnotations(), ingressutil.ManagedActionsAnnotation)
5662
}
57-
_, err = c.client.ExtensionsV1beta1().Ingresses(ingress.Namespace).Update(ctx, newIngress, metav1.UpdateOptions{})
63+
_, err = c.ingressWrapper.Update(ctx, ingress.GetNamespace(), newIngress)
5864
return err
5965
}
6066

61-
func getResetALBActionStr(ingress *extensionsv1beta1.Ingress, action string) (string, error) {
67+
func getResetALBActionStr(ingress *ingressutil.Ingress, action string) (string, error) {
6268
parts := strings.Split(action, ingressutil.ALBActionPrefix)
6369
if len(parts) != 2 {
6470
return "", fmt.Errorf("unable to parse action to get the service %s", action)
6571
}
6672
service := parts[1]
6773

68-
previousActionStr := ingress.Annotations[action]
74+
annotations := ingress.GetAnnotations()
75+
previousActionStr := annotations[action]
6976
var previousAction ingressutil.ALBAction
7077
err := json.Unmarshal([]byte(previousActionStr), &previousAction)
7178
if err != nil {

‎ingress/alb_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func TestInvalidManagedALBActions(t *testing.T) {
101101
ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name)
102102
ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-managed-by"
103103

104-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
104+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
105105

106106
err := ctrl.syncIngress("default/test-ingress")
107107
assert.Nil(t, err)
@@ -113,7 +113,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) {
113113
ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
114114
ing.Annotations[albActionAnnotation("stable-service")] = "{"
115115

116-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
116+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
117117

118118
err := ctrl.syncIngress("default/test-ingress")
119119
assert.Nil(t, err)
@@ -124,7 +124,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) {
124124
func TestInvalidPreviousALBActionAnnotationKey(t *testing.T) {
125125
ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
126126
ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-action-key"
127-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
127+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
128128

129129
err := ctrl.syncIngress("default/test-ingress")
130130
assert.Nil(t, err)
@@ -136,7 +136,7 @@ func TestResetActionFailureFindNoPort(t *testing.T) {
136136
ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
137137
ing.Annotations[albActionAnnotation("stable-service")] = "{}"
138138

139-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
139+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
140140

141141
err := ctrl.syncIngress("default/test-ingress")
142142
assert.Nil(t, err)
@@ -148,7 +148,7 @@ func TestALBIngressNoModifications(t *testing.T) {
148148
rollout := rollout("rollout", "stable-service", "test-ingress")
149149
ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name)
150150

151-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
151+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
152152

153153
err := ctrl.syncIngress("default/test-ingress")
154154
assert.Nil(t, err)
@@ -159,7 +159,7 @@ func TestALBIngressNoModifications(t *testing.T) {
159159
func TestALBIngressResetAction(t *testing.T) {
160160
ing := newALBIngress("test-ingress", 80, "stable-service", "non-existing-rollout")
161161

162-
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
162+
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
163163
err := ctrl.syncIngress("default/test-ingress")
164164
assert.Nil(t, err)
165165
assert.Len(t, enqueuedObjects, 0)

‎ingress/ingress.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
package ingress
22

33
import (
4+
"context"
45
"fmt"
56
"strings"
67
"time"
78

89
log "github.com/sirupsen/logrus"
910
"k8s.io/apimachinery/pkg/api/errors"
1011
"k8s.io/apimachinery/pkg/util/wait"
11-
extensionsinformers "k8s.io/client-go/informers/extensions/v1beta1"
1212
"k8s.io/client-go/kubernetes"
13-
extentionslisters "k8s.io/client-go/listers/extensions/v1beta1"
1413
"k8s.io/client-go/tools/cache"
1514
"k8s.io/client-go/util/workqueue"
1615
"k8s.io/kubernetes/cmd/kubeadm/app/util"
@@ -32,7 +31,7 @@ const (
3231
// ControllerConfig describes the data required to instantiate a new ingress controller
3332
type ControllerConfig struct {
3433
Client kubernetes.Interface
35-
IngressInformer extensionsinformers.IngressInformer
34+
IngressWrap *ingressutil.IngressWrap
3635
IngressWorkQueue workqueue.RateLimitingInterface
3736

3837
RolloutsInformer informers.RolloutInformer
@@ -47,7 +46,7 @@ type ControllerConfig struct {
4746
type Controller struct {
4847
client kubernetes.Interface
4948
rolloutsIndexer cache.Indexer
50-
ingressLister extentionslisters.IngressLister
49+
ingressWrapper IngressWrapper
5150
ingressWorkqueue workqueue.RateLimitingInterface
5251

5352
metricServer *metrics.MetricsServer
@@ -56,13 +55,18 @@ type Controller struct {
5655
nginxClasses []string
5756
}
5857

58+
type IngressWrapper interface {
59+
GetCached(namespace, name string) (*ingressutil.Ingress, error)
60+
Update(ctx context.Context, namespace string, ingress *ingressutil.Ingress) (*ingressutil.Ingress, error)
61+
}
62+
5963
// NewController returns a new ingress controller
6064
func NewController(cfg ControllerConfig) *Controller {
6165

6266
controller := &Controller{
6367
client: cfg.Client,
6468
rolloutsIndexer: cfg.RolloutsInformer.Informer().GetIndexer(),
65-
ingressLister: cfg.IngressInformer.Lister(),
69+
ingressWrapper: cfg.IngressWrap,
6670

6771
ingressWorkqueue: cfg.IngressWorkQueue,
6872
metricServer: cfg.MetricsServer,
@@ -79,7 +83,7 @@ func NewController(cfg ControllerConfig) *Controller {
7983
},
8084
}))
8185

82-
cfg.IngressInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
86+
cfg.IngressWrap.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
8387
AddFunc: func(obj interface{}) {
8488
controllerutil.Enqueue(obj, cfg.IngressWorkQueue)
8589
},
@@ -119,7 +123,7 @@ func (c *Controller) syncIngress(key string) error {
119123
if err != nil {
120124
return err
121125
}
122-
ingress, err := c.ingressLister.Ingresses(namespace).Get(name)
126+
ingress, err := c.ingressWrapper.GetCached(namespace, name)
123127
if err != nil {
124128
if !errors.IsNotFound(err) {
125129
// Unknown error occurred
@@ -132,15 +136,16 @@ func (c *Controller) syncIngress(key string) error {
132136
}
133137
return nil
134138
}
135-
rollouts, err := c.getRolloutsByIngress(ingress.Namespace, ingress.Name)
139+
rollouts, err := c.getRolloutsByIngress(ingress.GetNamespace(), ingress.GetName())
136140
if err != nil {
137141
return nil
138142
}
139143
// An ingress without annotations cannot be a alb or nginx ingress
140-
if ingress.Annotations == nil {
144+
if ingress.GetAnnotations() == nil {
141145
return nil
142146
}
143-
class := ingress.Annotations["kubernetes.io/ingress.class"]
147+
annotations := ingress.GetAnnotations()
148+
class := annotations["kubernetes.io/ingress.class"]
144149
switch {
145150
case hasClass(c.albClasses, class):
146151
return c.syncALBIngress(ingress, rollouts)

0 commit comments

Comments
 (0)