Merge pull request #111904 from pandaamanda/controller_codeclean

refactor: move attachdetach controller param validation ahead

cmd/kube-controller-manager/app/core.go

@@ -105,22 +105,11 @@ func startNodeIpamController(ctx context.Context, controllerContext ControllerCo
 		return nil, false, nil
 	}
 
-	// failure: bad cidrs in config
+	clusterCIDRs, err := validateCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
-	clusterCIDRs, dualStack, err := processCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
 	if err != nil {
 		return nil, false, err
 	}
 
-	// failure: more than one cidr but they are not configured as dual stack
-	if len(clusterCIDRs) > 1 && !dualStack {
-		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily)", len(clusterCIDRs))
-	}
-
-	// failure: more than cidrs is not allowed even with dual stack
-	if len(clusterCIDRs) > 2 {
-		return nil, false, fmt.Errorf("len of clusters is:%v > more than max allowed of 2", len(clusterCIDRs))
-	}
-
 	// service cidr processing
 	if len(strings.TrimSpace(controllerContext.ComponentConfig.NodeIPAMController.ServiceCIDR)) != 0 {
 		_, serviceCIDR, err = netutils.ParseCIDRSloppy(controllerContext.ComponentConfig.NodeIPAMController.ServiceCIDR)
@@ -238,22 +227,11 @@ func startRouteController(ctx context.Context, controllerContext ControllerConte
 		return nil, false, nil
 	}
 
-	// failure: bad cidrs in config
+	clusterCIDRs, err := validateCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
-	clusterCIDRs, dualStack, err := processCIDRs(controllerContext.ComponentConfig.KubeCloudShared.ClusterCIDR)
 	if err != nil {
 		return nil, false, err
 	}
 
-	// failure: more than one cidr but they are not configured as dual stack
-	if len(clusterCIDRs) > 1 && !dualStack {
-		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
-	}
-
-	// failure: more than cidrs is not allowed even with dual stack
-	if len(clusterCIDRs) > 2 {
-		return nil, false, fmt.Errorf("length of clusterCIDRs is:%v more than max allowed of 2", len(clusterCIDRs))
-	}
-
 	routeController := routecontroller.New(routes,
 		controllerContext.ClientBuilder.ClientOrDie("route-controller"),
 		controllerContext.InformerFactory.Core().V1().Nodes(),
@@ -297,10 +275,6 @@ func startPersistentVolumeBinderController(ctx context.Context, controllerContex
 }
 
 func startAttachDetachController(ctx context.Context, controllerContext ControllerContext) (controller.Interface, bool, error) {
-	if controllerContext.ComponentConfig.AttachDetachController.ReconcilerSyncLoopPeriod.Duration < time.Second {
-		return nil, true, fmt.Errorf("duration time must be greater than one second as set via command line option reconcile-sync-loop-period")
-	}
-
 	csiNodeInformer := controllerContext.InformerFactory.Storage().V1().CSINodes()
 	csiDriverInformer := controllerContext.InformerFactory.Storage().V1().CSIDrivers()
 
@@ -581,6 +555,29 @@ func startTTLAfterFinishedController(ctx context.Context, controllerContext Cont
 	return nil, true, nil
 }
 
+// processCIDRs is a helper function that works on a comma separated cidrs and returns
+// a list of typed cidrs
+// error if failed to parse any of the cidrs or invalid length of cidrs
+func validateCIDRs(cidrsList string) ([]*net.IPNet, error) {
+	// failure: bad cidrs in config
+	clusterCIDRs, dualStack, err := processCIDRs(cidrsList)
+	if err != nil {
+		return nil, err
+	}
+
+	// failure: more than one cidr but they are not configured as dual stack
+	if len(clusterCIDRs) > 1 && !dualStack {
+		return nil, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
+	}
+
+	// failure: more than cidrs is not allowed even with dual stack
+	if len(clusterCIDRs) > 2 {
+		return nil, fmt.Errorf("length of clusterCIDRs is:%v more than max allowed of 2", len(clusterCIDRs))
+	}
+
+	return clusterCIDRs, nil
+}
+
 // processCIDRs is a helper function that works on a comma separated cidrs and returns
 // a list of typed cidrs
 // a flag if cidrs represents a dual stack

cmd/kube-controller-manager/app/options/attachdetachcontroller.go

@@ -17,7 +17,9 @@ limitations under the License.
 package options
 
 import (
+	"fmt"
 	"github.com/spf13/pflag"
+	"time"
 
 	attachdetachconfig "k8s.io/kubernetes/pkg/controller/volume/attachdetach/config"
 )
@@ -56,5 +58,10 @@ func (o *AttachDetachControllerOptions) Validate() []error {
 	}
 
 	errs := []error{}
+
+	if o.ReconcilerSyncLoopPeriod.Duration < time.Second {
+		errs = append(errs, fmt.Errorf("duration time must be greater than one second as set via command line option reconcile-sync-loop-period"))
+	}
+
 	return errs
 }