Merge pull request #48348 from FengyunPan/check-openstack-Opts

Automatic merge from submit-queue (batch tested with PRs 47234, 48410, 48514, 48529, 48348)

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.

**Release note**:
```release-note
NONE
```

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
+	LBVersion            string     `gcfg:"lb-version"`          // overrides autodetection. v1 or v2
-	SubnetId             string     `gcfg:"subnet-id"`  // required
+	SubnetId             string     `gcfg:"subnet-id"`           // required
-	FloatingNetworkId    string     `gcfg:"floating-network-id"`
+	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"`
+	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
 }
 

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 }