feat: simplify flag parsing in network plugin

Commands should be written such that they take arguments as is.

plugins/network/network.go

@@ -25,17 +25,19 @@ var (
 )
 
 // BuildConfig builds network config files
-func BuildConfig(appName string) {
+func BuildConfig(appName string) error {
 	if err := common.VerifyAppName(appName); err != nil {
-		common.LogFail(err.Error())
+		return err
 	}
+
 	if !common.IsDeployed(appName) {
-		return
+		return nil
 	}
+
 	appRoot := common.AppRoot(appName)
 	scaleFile := strings.Join([]string{appRoot, "DOKKU_SCALE"}, "/")
 	if !common.FileExists(scaleFile) {
-		return
+		return nil
 	}
 
 	image := common.GetAppImageName(appName, "", "")
@@ -43,8 +45,9 @@ func BuildConfig(appName string) {
 	common.LogInfo1(fmt.Sprintf("Ensuring network configuration is in sync for %s", appName))
 	lines, err := common.FileToSlice(scaleFile)
 	if err != nil {
-		return
+		return err
 	}
+
 	for _, line := range lines {
 		if line == "" || strings.HasPrefix(line, "#") {
 			continue
@@ -91,6 +94,8 @@ func BuildConfig(appName string) {
 			}
 		}
 	}
+
+	return nil
 }
 
 // GetContainerIpaddress returns the ipaddr for a given app container

plugins/network/src/subcommands/subcommands.go

@@ -14,56 +14,60 @@ import (
 func main() {
 	parts := strings.Split(os.Args[0], "/")
 	subcommand := parts[len(parts)-1]
-	flag.Parse()
 
+	var err error
 	switch subcommand {
 	case "create":
-		networkName := flag.Arg(1)
+		args := flag.NewFlagSet("network:create", flag.ExitOnError)
-		err := network.CommandCreate(networkName)
+		args.Parse(os.Args[2:])
-		if err != nil {
+		networkName := args.Arg(0)
-			common.LogFail(err.Error())
+		err = network.CommandCreate(networkName)
-		}
 	case "destroy":
-		networkName := flag.Arg(1)
+		args := flag.NewFlagSet("network:destroy", flag.ExitOnError)
-		forceDestroy := false
+		force := args.Bool("force", false, "--force: force destroy without confirmation")
-		if flag.Arg(2) == "force" {
+		args.Parse(os.Args[2:])
-			forceDestroy = true
+		networkName := args.Arg(0)
-		}
+		err = network.CommandDestroy(networkName, *force)
-		err := network.CommandDestroy(networkName, forceDestroy)
-		if err != nil {
-			common.LogFail(err.Error())
-		}
 	case "exists":
-		networkName := flag.Arg(1)
+		args := flag.NewFlagSet("network:exists", flag.ExitOnError)
-		err := network.CommandExists(networkName)
+		args.Parse(os.Args[2:])
-		if err != nil {
+		networkName := args.Arg(0)
-			common.LogFail(err.Error())
+		err = network.CommandExists(networkName)
-		}
 	case "info":
-		err := network.CommandInfo()
+		args := flag.NewFlagSet("network:info", flag.ExitOnError)
-		if err != nil {
+		args.Parse(os.Args[2:])
-			common.LogFail(err.Error())
+		err = network.CommandInfo()
-		}
 	case "list":
-		err := network.CommandList()
+		args := flag.NewFlagSet("network:list", flag.ExitOnError)
-		if err != nil {
+		args.Parse(os.Args[2:])
-			common.LogFail(err.Error())
+		err = network.CommandList()
-		}
 	case "rebuild":
-		appName := flag.Arg(1)
+		args := flag.NewFlagSet("network:rebuild", flag.ExitOnError)
-		network.BuildConfig(appName)
+		args.Parse(os.Args[2:])
+		appName := args.Arg(0)
+		err = network.BuildConfig(appName)
 	case "rebuildall":
-		network.CommandRebuildall()
+		args := flag.NewFlagSet("network:rebuildall", flag.ExitOnError)
+		args.Parse(os.Args[2:])
+		err = network.CommandRebuildall()
 	case "report":
-		appName := flag.Arg(1)
+		args := flag.NewFlagSet("network:report", flag.ExitOnError)
-		infoFlag := flag.Arg(2)
+		args.Parse(os.Args[2:])
-		network.CommandReport(appName, infoFlag)
+		appName := args.Arg(0)
+		infoFlag := args.Arg(1)
+		err = network.CommandReport(appName, infoFlag)
 	case "set":
-		appName := flag.Arg(1)
+		args := flag.NewFlagSet("network:set", flag.ExitOnError)
-		property := flag.Arg(2)
+		args.Parse(os.Args[2:])
-		value := flag.Arg(3)
+		appName := args.Arg(0)
-		network.CommandSet(appName, property, value)
+		property := args.Arg(1)
+		value := args.Arg(2)
+		err = network.CommandSet(appName, property, value)
 	default:
 		common.LogFail(fmt.Sprintf("Invalid plugin subcommand call: %s", subcommand))
 	}
+
+	if err != nil {
+		common.LogFail(err.Error())
+	}
 }

plugins/network/subcommands.go

@@ -106,14 +106,20 @@ func CommandList() error {
 }
 
 // CommandRebuildall rebuilds network settings for all apps
-func CommandRebuildall() {
+func CommandRebuildall() error {
 	apps, err := common.DokkuApps()
 	if err != nil {
-		common.LogFail(err.Error())
+		return err
 	}
+
 	for _, appName := range apps {
-		BuildConfig(appName)
+		err = BuildConfig(appName)
+		if err != nil {
+			return err
+		}
 	}
+
+	return nil
 }
 
 // CommandReport displays a network report for one or more apps
@@ -140,7 +146,7 @@ func CommandReport(appName string, infoFlag string) error {
 }
 
 // CommandSet set or clear a network property for an app
-func CommandSet(appName string, property string, value string) {
+func CommandSet(appName string, property string, value string) error {
 	if property == "bind-all-interfaces" && value == "" {
 		value = "false"
 	}
@@ -156,13 +162,14 @@ func CommandSet(appName string, property string, value string) {
 	}
 	if attachProperites[property] {
 		if invalidNetworks[value] {
-			common.LogFail("Invalid network name specified for attach")
+			return errors.New("Invalid network name specified for attach")
 		}
 
 		if isConflictingPropertyValue(appName, property, value) {
-			common.LogFail("Network name already associated with this app")
+			return errors.New("Network name already associated with this app")
 		}
 	}
 
 	common.CommandPropertySet("network", appName, property, value, DefaultProperties)
+	return nil
 }