Skip to content

Commit 3d533ef

Browse files
prydieowainlewis
prydie
authored and
owainlewis
committed
Fix panic when no backends (#157)
* Refactor LBSpec to derive state up-front * Fix panic when no backends * Refactor security list manager to use portSpec * Remove old node port rule in Update() * Remove old node port rule in securityListManger.Update() rather than
1 parent 686c192 commit 3d533ef

File tree

8 files changed

+584
-340
lines changed

8 files changed

+584
-340
lines changed

‎cmd/oci-cloud-controller-manager/main.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ func main() {
4747
pflag.CommandLine.SetNormalizeFunc(utilflag.WordSepNormalizeFunc)
4848
pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
4949
goflag.CommandLine.Parse([]string{})
50-
// utilflag.InitFlags()
5150
logs.InitLogs()
5251
defer logs.FlushLogs()
5352

‎pkg/oci/load_balancer.go‎

Lines changed: 90 additions & 149 deletions
Large diffs are not rendered by default.

‎pkg/oci/load_balancer_security_lists.go‎

Lines changed: 89 additions & 40 deletions
Large diffs are not rendered by default.

‎pkg/oci/load_balancer_security_lists_test.go‎

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,16 @@ import (
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
v1listers "k8s.io/client-go/listers/core/v1"
2727
"k8s.io/client-go/tools/cache"
28+
k8sports "k8s.io/kubernetes/pkg/master/ports"
2829
)
2930

3031
func TestGetNodeIngressRules(t *testing.T) {
3132
testCases := []struct {
3233
name string
3334
securityList *core.SecurityList
3435
lbSubnets []*core.Subnet
35-
port int
36+
actualPorts *portSpec
37+
desiredPorts portSpec
3638
services []*v1.Service
3739
expected []core.IngressSecurityRule
3840
}{
@@ -47,81 +49,108 @@ func TestGetNodeIngressRules(t *testing.T) {
4749
{CidrBlock: common.String("1")},
4850
{CidrBlock: common.String("2")},
4951
},
50-
port: 80,
52+
desiredPorts: portSpec{
53+
BackendPort: 80,
54+
HealthCheckerPort: k8sports.ProxyHealthzPort,
55+
},
5156
services: []*v1.Service{},
5257
expected: []core.IngressSecurityRule{
5358
makeIngressSecurityRule("existing", 9000),
5459
makeIngressSecurityRule("1", 80),
5560
makeIngressSecurityRule("2", 80),
61+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
62+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
5663
},
5764
}, {
5865
name: "no change",
5966
securityList: &core.SecurityList{
6067
IngressSecurityRules: []core.IngressSecurityRule{
6168
makeIngressSecurityRule("existing", 9000),
6269
makeIngressSecurityRule("1", 80),
70+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
6371
makeIngressSecurityRule("2", 80),
72+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
6473
},
6574
},
6675
lbSubnets: []*core.Subnet{
6776
{CidrBlock: common.String("1")},
6877
{CidrBlock: common.String("2")},
6978
},
70-
port: 80,
79+
desiredPorts: portSpec{
80+
BackendPort: 80,
81+
HealthCheckerPort: k8sports.ProxyHealthzPort,
82+
},
7183
services: []*v1.Service{},
7284
expected: []core.IngressSecurityRule{
7385
makeIngressSecurityRule("existing", 9000),
7486
makeIngressSecurityRule("1", 80),
87+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
7588
makeIngressSecurityRule("2", 80),
89+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
7690
},
7791
}, {
7892
name: "change lb subnet",
7993
securityList: &core.SecurityList{
8094
IngressSecurityRules: []core.IngressSecurityRule{
8195
makeIngressSecurityRule("existing", 9000),
8296
makeIngressSecurityRule("1", 80),
97+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
8398
makeIngressSecurityRule("2", 80),
99+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
84100
makeIngressSecurityRule("existing", 9001),
85101
},
86102
},
87103
lbSubnets: []*core.Subnet{
88104
{CidrBlock: common.String("1")},
89105
{CidrBlock: common.String("3")},
90106
},
91-
port: 80,
107+
desiredPorts: portSpec{
108+
BackendPort: 80,
109+
HealthCheckerPort: k8sports.ProxyHealthzPort,
110+
},
92111
services: []*v1.Service{},
93112
expected: []core.IngressSecurityRule{
94113
makeIngressSecurityRule("existing", 9000),
95114
makeIngressSecurityRule("1", 80),
115+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
96116
makeIngressSecurityRule("existing", 9001),
97117
makeIngressSecurityRule("3", 80),
118+
makeIngressSecurityRule("3", k8sports.ProxyHealthzPort),
98119
},
99120
}, {
100121
name: "remove lb subnets",
101122
securityList: &core.SecurityList{
102123
IngressSecurityRules: []core.IngressSecurityRule{
103124
makeIngressSecurityRule("existing", 9000),
104125
makeIngressSecurityRule("1", 80),
126+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
105127
makeIngressSecurityRule("2", 80),
128+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
106129
makeIngressSecurityRule("existing", 9001),
107130
},
108131
},
109132
lbSubnets: []*core.Subnet{},
110-
port: 80,
111-
services: []*v1.Service{},
133+
desiredPorts: portSpec{
134+
BackendPort: 80,
135+
HealthCheckerPort: k8sports.ProxyHealthzPort,
136+
},
137+
services: []*v1.Service{},
112138
expected: []core.IngressSecurityRule{
113139
makeIngressSecurityRule("existing", 9000),
114140
makeIngressSecurityRule("existing", 9001),
115141
},
116142
}, {
117-
name: "do not delete a port rule which is used by another services (default) health check",
143+
name: "do not delete a rule which is used by another services (default) health check",
118144
securityList: &core.SecurityList{
119145
IngressSecurityRules: []core.IngressSecurityRule{
120146
makeIngressSecurityRule("0.0.0.0/0", lbNodesHealthCheckPort),
121147
},
122148
},
123149
lbSubnets: []*core.Subnet{},
124-
port: lbNodesHealthCheckPort,
150+
desiredPorts: portSpec{
151+
BackendPort: 80,
152+
HealthCheckerPort: k8sports.ProxyHealthzPort,
153+
},
125154
services: []*v1.Service{
126155
{
127156
ObjectMeta: metav1.ObjectMeta{Namespace: "namespace", Name: "using-default-health-check-port"},
@@ -134,6 +163,37 @@ func TestGetNodeIngressRules(t *testing.T) {
134163
expected: []core.IngressSecurityRule{
135164
makeIngressSecurityRule("0.0.0.0/0", lbNodesHealthCheckPort),
136165
},
166+
}, {
167+
name: "update node port",
168+
securityList: &core.SecurityList{
169+
IngressSecurityRules: []core.IngressSecurityRule{
170+
makeIngressSecurityRule("existing", 9000),
171+
makeIngressSecurityRule("1", 8081),
172+
makeIngressSecurityRule("2", 8081),
173+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
174+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
175+
},
176+
},
177+
lbSubnets: []*core.Subnet{
178+
{CidrBlock: common.String("1")},
179+
{CidrBlock: common.String("2")},
180+
},
181+
actualPorts: &portSpec{
182+
BackendPort: 8081,
183+
HealthCheckerPort: k8sports.ProxyHealthzPort,
184+
},
185+
desiredPorts: portSpec{
186+
BackendPort: 80,
187+
HealthCheckerPort: k8sports.ProxyHealthzPort,
188+
},
189+
services: []*v1.Service{},
190+
expected: []core.IngressSecurityRule{
191+
makeIngressSecurityRule("existing", 9000),
192+
makeIngressSecurityRule("1", k8sports.ProxyHealthzPort),
193+
makeIngressSecurityRule("2", k8sports.ProxyHealthzPort),
194+
makeIngressSecurityRule("1", 80),
195+
makeIngressSecurityRule("2", 80),
196+
},
137197
},
138198
}
139199

@@ -146,7 +206,7 @@ func TestGetNodeIngressRules(t *testing.T) {
146206
}
147207
}
148208
t.Run(tc.name, func(t *testing.T) {
149-
rules := getNodeIngressRules(tc.securityList.IngressSecurityRules, tc.lbSubnets, tc.port, serviceLister)
209+
rules := getNodeIngressRules(tc.securityList.IngressSecurityRules, tc.lbSubnets, tc.actualPorts, tc.desiredPorts, serviceLister)
150210
if !reflect.DeepEqual(rules, tc.expected) {
151211
t.Errorf("expected rules\n%+v\nbut got\n%+v", tc.expected, rules)
152212
}

0 commit comments

Comments
 (0)