From d2ebb60438446532466eca003b3ff0870e77d9c3 Mon Sep 17 00:00:00 2001 From: FengyunPan Date: Fri, 7 Jul 2017 17:05:21 +0800 Subject: [PATCH] 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. --- .../providers/openstack/openstack.go | 47 +++++++++- .../providers/openstack/openstack_test.go | 94 +++++++++++++++++++ 2 files changed, 137 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 31f913426e..7f9ebc7804 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -75,10 +75,10 @@ type LoadBalancer struct { } type LoadBalancerOpts struct { - LBVersion string `gcfg:"lb-version"` // overrides autodetection. v1 or v2 - SubnetId string `gcfg:"subnet-id"` // required - FloatingNetworkId string `gcfg:"floating-network-id"` - LBMethod string `gcfg:"lb-method"` + LBVersion string `gcfg:"lb-version"` // overrides autodetection. v1 or v2 + SubnetId string `gcfg:"subnet-id"` // required + FloatingNetworkId string `gcfg:"floating-network-id"` // If specified, will create floating ip for loadbalancer, or do not create floating ip. + LBMethod string `gcfg:"lb-method"` // default to ROUND_ROBIN. CreateMonitor bool `gcfg:"create-monitor"` MonitorDelay MyDuration `gcfg:"monitor-delay"` MonitorTimeout MyDuration `gcfg:"monitor-timeout"` @@ -216,6 +216,40 @@ func readInstanceID() (string, error) { return md.Uuid, nil } +// check opts for OpenStack +func checkOpenStackOpts(openstackOpts *OpenStack) error { + lbOpts := openstackOpts.lbOpts + + // subnet-id is required + if len(lbOpts.SubnetId) == 0 { + return fmt.Errorf("subnet-id not set in cloud provider config") + } + + // if need to create health monitor for Neutron LB, + // monitor-delay, monitor-timeout and monitor-max-retries should be set. + emptyDuration := MyDuration{} + if lbOpts.CreateMonitor { + if lbOpts.MonitorDelay == emptyDuration { + return fmt.Errorf("monitor-delay not set in cloud provider config") + } + if lbOpts.MonitorTimeout == emptyDuration { + return fmt.Errorf("monitor-timeout not set in cloud provider config") + } + if lbOpts.MonitorMaxRetries == uint(0) { + return fmt.Errorf("monitor-max-retries not set in cloud provider config") + } + } + + // if enable ManageSecurityGroups, node-security-group should be set. + if lbOpts.ManageSecurityGroups { + if len(lbOpts.NodeSecurityGroupID) == 0 { + return fmt.Errorf("node-security-group not set in cloud provider config") + } + } + + return nil +} + func newOpenStack(cfg Config) (*OpenStack, error) { provider, err := openstack.NewClient(cfg.Global.AuthUrl) if err != nil { @@ -260,6 +294,11 @@ func newOpenStack(cfg Config) (*OpenStack, error) { localInstanceID: id, } + err = checkOpenStackOpts(&os) + if err != nil { + return nil, err + } + return &os, nil } diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index 6ac40884e6..d69d53e919 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -144,6 +144,100 @@ func TestToAuthOptions(t *testing.T) { } } +func TestCheckOpenStackOpts(t *testing.T) { + delay := MyDuration{60 * time.Second} + timeout := MyDuration{30 * time.Second} + tests := []struct { + name string + openstackOpts *OpenStack + expectedError error + }{ + { + name: "test1", + openstackOpts: &OpenStack{ + provider: nil, + lbOpts: LoadBalancerOpts{ + LBVersion: "v2", + SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef", + FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786", + LBMethod: "ROUND_ROBIN", + CreateMonitor: true, + MonitorDelay: delay, + MonitorTimeout: timeout, + MonitorMaxRetries: uint(3), + ManageSecurityGroups: true, + NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144", + }, + }, + expectedError: nil, + }, + { + name: "test2", + openstackOpts: &OpenStack{ + provider: nil, + lbOpts: LoadBalancerOpts{ + LBVersion: "v2", + FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786", + LBMethod: "ROUND_ROBIN", + CreateMonitor: true, + MonitorDelay: delay, + MonitorTimeout: timeout, + MonitorMaxRetries: uint(3), + ManageSecurityGroups: true, + NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144", + }, + }, + expectedError: fmt.Errorf("subnet-id not set in cloud provider config"), + }, + { + name: "test3", + openstackOpts: &OpenStack{ + provider: nil, + lbOpts: LoadBalancerOpts{ + LBVersion: "v2", + SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef", + FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786", + LBMethod: "ROUND_ROBIN", + CreateMonitor: true, + ManageSecurityGroups: true, + NodeSecurityGroupID: "b41d28c2-d02f-4e1e-8ffb-23b8e4f5c144", + }, + }, + expectedError: fmt.Errorf("monitor-delay not set in cloud provider config"), + }, + { + name: "test4", + openstackOpts: &OpenStack{ + provider: nil, + lbOpts: LoadBalancerOpts{ + LBVersion: "v2", + SubnetId: "6261548e-ffde-4bc7-bd22-59c83578c5ef", + FloatingNetworkId: "38b8b5f9-64dc-4424-bf86-679595714786", + LBMethod: "ROUND_ROBIN", + CreateMonitor: true, + MonitorDelay: delay, + MonitorTimeout: timeout, + MonitorMaxRetries: uint(3), + ManageSecurityGroups: true, + }, + }, + expectedError: fmt.Errorf("node-security-group not set in cloud provider config"), + }, + } + + for _, testcase := range tests { + err := checkOpenStackOpts(testcase.openstackOpts) + + if err == nil && testcase.expectedError == nil { + continue + } + if (err != nil && testcase.expectedError == nil) || (err == nil && testcase.expectedError != nil) || err.Error() != testcase.expectedError.Error() { + t.Errorf("%s failed: expected err=%q, got %q", + testcase.name, testcase.expectedError, err) + } + } +} + func TestCaller(t *testing.T) { called := false myFunc := func() { called = true }