Skip to content

Commit 4e9b516

Browse files
LiorLiebermanLiorLieberman
authored
fix flakly ingressnginx test (#139)
* fix flakly ingressnginx test * address feedback
1 parent 6dc297a commit 4e9b516

File tree

4 files changed

+113
-82
lines changed

4 files changed

+113
-82
lines changed

‎pkg/i2gw/providers/ingressnginx/converter.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package ingressnginx
1919
import (
2020
"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw"
2121
"github.com/kubernetes-sigs/ingress2gateway/pkg/i2gw/providers/common"
22-
networkingv1 "k8s.io/api/networking/v1"
2322
"k8s.io/apimachinery/pkg/util/validation/field"
2423
)
2524

@@ -40,10 +39,7 @@ func newConverter() *converter {
4039
func (c *converter) convert(storage *storage) (i2gw.GatewayResources, field.ErrorList) {
4140

4241
// TODO(liorliberman) temporary until we decide to change ToGateway and featureParsers to get a map of [types.NamespacedName]*networkingv1.Ingress instead of a list
43-
ingressList := []networkingv1.Ingress{}
44-
for _, ing := range storage.Ingresses {
45-
ingressList = append(ingressList, *ing)
46-
}
42+
ingressList := storage.Ingresses.List()
4743

4844
// Convert plain ingress resources to gateway resources, ignoring all
4945
// provider-specific features.

‎pkg/i2gw/providers/ingressnginx/converter_test.go

+79-73
Original file line numberDiff line numberDiff line change
@@ -42,66 +42,69 @@ func Test_ToGateway(t *testing.T) {
4242

4343
testCases := []struct {
4444
name string
45-
ingresses map[types.NamespacedName]*networkingv1.Ingress
45+
ingresses OrderedIngressMap
4646
expectedGatewayResources i2gw.GatewayResources
4747
expectedErrors field.ErrorList
4848
}{
4949
{
5050
name: "canary deployment",
51-
ingresses: map[types.NamespacedName]*networkingv1.Ingress{
52-
{Namespace: "default", Name: "production"}: {
53-
ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "default"},
54-
Spec: networkingv1.IngressSpec{
55-
IngressClassName: ptrTo("ingress-nginx"),
56-
Rules: []networkingv1.IngressRule{{
57-
Host: "echo.prod.mydomain.com",
58-
IngressRuleValue: networkingv1.IngressRuleValue{
59-
HTTP: &networkingv1.HTTPIngressRuleValue{
60-
Paths: []networkingv1.HTTPIngressPath{{
61-
Path: "/",
62-
PathType: &iPrefix,
63-
Backend: networkingv1.IngressBackend{
64-
Resource: &corev1.TypedLocalObjectReference{
65-
Name: "production",
66-
Kind: "StorageBucket",
67-
APIGroup: ptrTo("vendor.example.com"),
51+
ingresses: OrderedIngressMap{
52+
ingressNames: []types.NamespacedName{{Namespace: "default", Name: "production"}, {Namespace: "default", Name: "canary"}},
53+
ingressObjects: map[types.NamespacedName]*networkingv1.Ingress{
54+
{Namespace: "default", Name: "production"}: {
55+
ObjectMeta: metav1.ObjectMeta{Name: "production", Namespace: "default"},
56+
Spec: networkingv1.IngressSpec{
57+
IngressClassName: ptrTo("ingress-nginx"),
58+
Rules: []networkingv1.IngressRule{{
59+
Host: "echo.prod.mydomain.com",
60+
IngressRuleValue: networkingv1.IngressRuleValue{
61+
HTTP: &networkingv1.HTTPIngressRuleValue{
62+
Paths: []networkingv1.HTTPIngressPath{{
63+
Path: "/",
64+
PathType: &iPrefix,
65+
Backend: networkingv1.IngressBackend{
66+
Resource: &corev1.TypedLocalObjectReference{
67+
Name: "production",
68+
Kind: "StorageBucket",
69+
APIGroup: ptrTo("vendor.example.com"),
70+
},
6871
},
69-
},
70-
}},
72+
}},
73+
},
7174
},
72-
},
73-
}},
74-
},
75-
},
76-
{Namespace: "default", Name: "canary"}: {
77-
ObjectMeta: metav1.ObjectMeta{
78-
Name: "canary",
79-
Namespace: "default",
80-
Annotations: map[string]string{
81-
"nginx.ingress.kubernetes.io/canary": "true",
82-
"nginx.ingress.kubernetes.io/canary-weight": "20",
75+
}},
8376
},
8477
},
85-
Spec: networkingv1.IngressSpec{
86-
IngressClassName: ptrTo("ingress-nginx"),
87-
Rules: []networkingv1.IngressRule{{
88-
Host: "echo.prod.mydomain.com",
89-
IngressRuleValue: networkingv1.IngressRuleValue{
90-
HTTP: &networkingv1.HTTPIngressRuleValue{
91-
Paths: []networkingv1.HTTPIngressPath{{
92-
Path: "/",
93-
PathType: &iPrefix,
94-
Backend: networkingv1.IngressBackend{
95-
Resource: &corev1.TypedLocalObjectReference{
96-
Name: "canary",
97-
Kind: "StorageBucket",
98-
APIGroup: ptrTo("vendor.example.com"),
78+
{Namespace: "default", Name: "canary"}: {
79+
ObjectMeta: metav1.ObjectMeta{
80+
Name: "canary",
81+
Namespace: "default",
82+
Annotations: map[string]string{
83+
"nginx.ingress.kubernetes.io/canary": "true",
84+
"nginx.ingress.kubernetes.io/canary-weight": "20",
85+
},
86+
},
87+
Spec: networkingv1.IngressSpec{
88+
IngressClassName: ptrTo("ingress-nginx"),
89+
Rules: []networkingv1.IngressRule{{
90+
Host: "echo.prod.mydomain.com",
91+
IngressRuleValue: networkingv1.IngressRuleValue{
92+
HTTP: &networkingv1.HTTPIngressRuleValue{
93+
Paths: []networkingv1.HTTPIngressPath{{
94+
Path: "/",
95+
PathType: &iPrefix,
96+
Backend: networkingv1.IngressBackend{
97+
Resource: &corev1.TypedLocalObjectReference{
98+
Name: "canary",
99+
Kind: "StorageBucket",
100+
APIGroup: ptrTo("vendor.example.com"),
101+
},
99102
},
100-
},
101-
}},
103+
}},
104+
},
102105
},
103-
},
104-
}},
106+
}},
107+
},
105108
},
106109
},
107110
},
@@ -168,33 +171,36 @@ func Test_ToGateway(t *testing.T) {
168171
},
169172
{
170173
name: "ImplementationSpecific HTTPRouteMatching",
171-
ingresses: map[types.NamespacedName]*networkingv1.Ingress{
172-
{Namespace: "default", Name: "implementation-specific-regex"}: {
173-
ObjectMeta: metav1.ObjectMeta{
174-
Name: "implementation-specific-regex",
175-
Namespace: "default",
176-
},
177-
Spec: networkingv1.IngressSpec{
178-
IngressClassName: ptrTo("ingress-nginx"),
179-
Rules: []networkingv1.IngressRule{{
180-
Host: "test.mydomain.com",
181-
IngressRuleValue: networkingv1.IngressRuleValue{
182-
HTTP: &networkingv1.HTTPIngressRuleValue{
183-
Paths: []networkingv1.HTTPIngressPath{{
184-
Path: "/~/echo/**/test",
185-
PathType: &isPathType,
186-
Backend: networkingv1.IngressBackend{
187-
Service: &networkingv1.IngressServiceBackend{
188-
Name: "test",
189-
Port: networkingv1.ServiceBackendPort{
190-
Number: 80,
174+
ingresses: OrderedIngressMap{
175+
ingressNames: []types.NamespacedName{{Namespace: "default", Name: "implementation-specific-regex"}},
176+
ingressObjects: map[types.NamespacedName]*networkingv1.Ingress{
177+
{Namespace: "default", Name: "implementation-specific-regex"}: {
178+
ObjectMeta: metav1.ObjectMeta{
179+
Name: "implementation-specific-regex",
180+
Namespace: "default",
181+
},
182+
Spec: networkingv1.IngressSpec{
183+
IngressClassName: ptrTo("ingress-nginx"),
184+
Rules: []networkingv1.IngressRule{{
185+
Host: "test.mydomain.com",
186+
IngressRuleValue: networkingv1.IngressRuleValue{
187+
HTTP: &networkingv1.HTTPIngressRuleValue{
188+
Paths: []networkingv1.HTTPIngressPath{{
189+
Path: "/~/echo/**/test",
190+
PathType: &isPathType,
191+
Backend: networkingv1.IngressBackend{
192+
Service: &networkingv1.IngressServiceBackend{
193+
Name: "test",
194+
Port: networkingv1.ServiceBackendPort{
195+
Number: 80,
196+
},
191197
},
192198
},
193-
},
194-
}},
199+
}},
200+
},
195201
},
196-
},
197-
}},
202+
}},
203+
},
198204
},
199205
},
200206
},

‎pkg/i2gw/providers/ingressnginx/resource_reader.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (r *resourceReader) readResourcesFromCluster(ctx context.Context) (*storage
4242
if err != nil {
4343
return nil, err
4444
}
45-
storage.Ingresses = ingresses
45+
storage.Ingresses.FromMap(ingresses)
4646
return storage, nil
4747
}
4848

@@ -53,6 +53,6 @@ func (r *resourceReader) readResourcesFromFile(filename string) (*storage, error
5353
if err != nil {
5454
return nil, err
5555
}
56-
storage.Ingresses = ingresses
56+
storage.Ingresses.FromMap(ingresses)
5757
return storage, nil
5858
}

‎pkg/i2gw/providers/ingressnginx/storage.go

+31-2
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,45 @@ limitations under the License.
1717
package ingressnginx
1818

1919
import (
20+
"sort"
21+
2022
networkingv1 "k8s.io/api/networking/v1"
2123
"k8s.io/apimachinery/pkg/types"
2224
)
2325

26+
type OrderedIngressMap struct {
27+
ingressNames []types.NamespacedName
28+
ingressObjects map[types.NamespacedName]*networkingv1.Ingress
29+
}
2430
type storage struct {
25-
Ingresses map[types.NamespacedName]*networkingv1.Ingress
31+
Ingresses OrderedIngressMap
2632
}
2733

2834
func newResourcesStorage() *storage {
2935
return &storage{
30-
Ingresses: map[types.NamespacedName]*networkingv1.Ingress{},
36+
Ingresses: OrderedIngressMap{
37+
ingressNames: []types.NamespacedName{},
38+
ingressObjects: map[types.NamespacedName]*networkingv1.Ingress{},
39+
},
40+
}
41+
}
42+
43+
func (oim *OrderedIngressMap) List() []networkingv1.Ingress {
44+
ingressList := []networkingv1.Ingress{}
45+
for _, ing := range oim.ingressNames {
46+
ingressList = append(ingressList, *oim.ingressObjects[ing])
47+
}
48+
return ingressList
49+
}
50+
51+
func (oim *OrderedIngressMap) FromMap(ingresses map[types.NamespacedName]*networkingv1.Ingress) {
52+
ingNames := []types.NamespacedName{}
53+
for ing := range ingresses {
54+
ingNames = append(ingNames, ing)
3155
}
56+
sort.Slice(ingNames, func(i, j int) bool {
57+
return ingNames[i].Name < ingNames[j].Name
58+
})
59+
oim.ingressNames = ingNames
60+
oim.ingressObjects = ingresses
3261
}

0 commit comments

Comments
 (0)