Skip to content
This repository was archived by the owner on Mar 22, 2018. It is now read-only.

Commit b9904dd

Browse files
author
FengyunPan
committed
Check opts of cloud config file
Fix #48347 Check opts when register OpenStack CloudProvider rather than returning error when use opts to create/use cloud resource.
1 parent 9009e12 commit b9904dd

2 files changed

Lines changed: 137 additions & 4 deletions

File tree

pkg/cloudprovider/providers/openstack/openstack.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ type LoadBalancer struct {
7575
}
7676

7777
type LoadBalancerOpts struct {
78-
LBVersion string `gcfg:"lb-version"` // overrides autodetection. v1 or v2
79-
SubnetId string `gcfg:"subnet-id"` // required
80-
FloatingNetworkId string `gcfg:"floating-network-id"`
81-
LBMethod string `gcfg:"lb-method"`
78+
LBVersion string `gcfg:"lb-version"` // overrides autodetection. v1 or v2
79+
SubnetId string `gcfg:"subnet-id"` // required
80+
FloatingNetworkId string `gcfg:"floating-network-id"` // If specified, will create floating ip for loadbalancer, or do not create floating ip.
81+
LBMethod string `gcfg:"lb-method"` // default to ROUND_ROBIN.
8282
CreateMonitor bool `gcfg:"create-monitor"`
8383
MonitorDelay MyDuration `gcfg:"monitor-delay"`
8484
MonitorTimeout MyDuration `gcfg:"monitor-timeout"`
@@ -216,6 +216,40 @@ func readInstanceID() (string, error) {
216216
return md.Uuid, nil
217217
}
218218

219+
// check opts for OpenStack
220+
func checkOpenStackOpts(openstackOpts *OpenStack) error {
221+
lbOpts := openstackOpts.lbOpts
222+
223+
// subnet-id is required
224+
if len(lbOpts.SubnetId) == 0 {
225+
return fmt.Errorf("subnet-id not set in cloud provider config")
226+
}
227+
228+
// if need to create health monitor for Neutron LB,
229+
// monitor-delay, monitor-timeout and monitor-max-retries should be set.
230+
emptyDuration := MyDuration{}
231+
if lbOpts.CreateMonitor {
232+
if lbOpts.MonitorDelay == emptyDuration {
233+
return fmt.Errorf("monitor-delay not set in cloud provider config")
234+
}
235+
if lbOpts.MonitorTimeout == emptyDuration {
236+
return fmt.Errorf("monitor-timeout not set in cloud provider config")
237+
}
238+
if lbOpts.MonitorMaxRetries == uint(0) {
239+
return fmt.Errorf("monitor-max-retries not set in cloud provider config")
240+
}
241+
}
242+
243+
// if enable ManageSecurityGroups, node-security-group should be set.
244+
if lbOpts.ManageSecurityGroups {
245+
if len(lbOpts.NodeSecurityGroupID) == 0 {
246+
return fmt.Errorf("node-security-group not set in cloud provider config")
247+
}
248+
}
249+
250+
return nil
251+
}
252+
219253
func newOpenStack(cfg Config) (*OpenStack, error) {
220254
provider, err := openstack.NewClient(cfg.Global.AuthUrl)
221255
if err != nil {
@@ -260,6 +294,11 @@ func newOpenStack(cfg Config) (*OpenStack, error) {
260294
localInstanceID: id,
261295
}
262296

297+
err = checkOpenStackOpts(&os)
298+
if err != nil {
299+
return nil, err
300+
}
301+
263302
return &os, nil
264303
}
265304

pkg/cloudprovider/providers/openstack/openstack_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,100 @@ func TestToAuthOptions(t *testing.T) {
144144
}
145145
}
146146

147+
func TestCheckOpenStackOpts(t *testing.T) {
148+
delay := MyDuration{60 * time.Second}
149+
timeout := MyDuration{30 * time.Second}
150+
tests := []struct {
151+
name string
152+
openstackOpts *OpenStack
153+
expectedError error
154+
}{
155+
{
156+
name: "test1",
157+
openstackOpts: &OpenStack{
158+
provider: nil,
159+
lbOpts: LoadBalancerOpts{
160+
LBVersion: "v2",
161+
SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef",
162+
FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786",
163+
LBMethod: "ROUND_ROBIN",
164+
CreateMonitor: true,
165+
MonitorDelay: delay,
166+
MonitorTimeout: timeout,
167+
MonitorMaxRetries: uint(3),
168+
ManageSecurityGroups: true,
169+
NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
170+
},
171+
},
172+
expectedError: nil,
173+
},
174+
{
175+
name: "test2",
176+
openstackOpts: &OpenStack{
177+
provider: nil,
178+
lbOpts: LoadBalancerOpts{
179+
LBVersion: "v2",
180+
FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786",
181+
LBMethod: "ROUND_ROBIN",
182+
CreateMonitor: true,
183+
MonitorDelay: delay,
184+
MonitorTimeout: timeout,
185+
MonitorMaxRetries: uint(3),
186+
ManageSecurityGroups: true,
187+
NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
188+
},
189+
},
190+
expectedError: fmt.Errorf("subnet-id not set in cloud provider config"),
191+
},
192+
{
193+
name: "test3",
194+
openstackOpts: &OpenStack{
195+
provider: nil,
196+
lbOpts: LoadBalancerOpts{
197+
LBVersion: "v2",
198+
SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef",
199+
FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786",
200+
LBMethod: "ROUND_ROBIN",
201+
CreateMonitor: true,
202+
ManageSecurityGroups: true,
203+
NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144",
204+
},
205+
},
206+
expectedError: fmt.Errorf("monitor-delay not set in cloud provider config"),
207+
},
208+
{
209+
name: "test4",
210+
openstackOpts: &OpenStack{
211+
provider: nil,
212+
lbOpts: LoadBalancerOpts{
213+
LBVersion: "v2",
214+
SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef",
215+
FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786",
216+
LBMethod: "ROUND_ROBIN",
217+
CreateMonitor: true,
218+
MonitorDelay: delay,
219+
MonitorTimeout: timeout,
220+
MonitorMaxRetries: uint(3),
221+
ManageSecurityGroups: true,
222+
},
223+
},
224+
expectedError: fmt.Errorf("node-security-group not set in cloud provider config"),
225+
},
226+
}
227+
228+
for _, testcase := range tests {
229+
err := checkOpenStackOpts(testcase.openstackOpts)
230+
231+
if err == nil && testcase.expectedError == nil {
232+
continue
233+
}
234+
if (err != nil && testcase.expectedError == nil) || (err == nil && testcase.expectedError != nil) || err.Error() != testcase.expectedError.Error() {
235+
t.Errorf("%s failed: expected err=%q, got %q",
236+
testcase.name, testcase.expectedError, err)
237+
}
238+
}
239+
}
240+
147241
func TestCaller(t *testing.T) {
148242
called := false
149243
myFunc := func() { called = true }

0 commit comments

Comments
 (0)