NetworkCheck diag: deprecate flags, use parameters

Author: sosiouxme

pkg/diagnostics/network/run_pod.go

@@ -5,11 +5,16 @@ import (
 	"fmt"
 	"os"
 	"os/signal"
+	"path/filepath"
+	"strconv"
+	"strings"
 	"syscall"
 	"time"
 
 	flag "github.com/spf13/pflag"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+	kvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/storage/names"
 	kapi "k8s.io/kubernetes/pkg/api"
@@ -22,7 +27,12 @@ import (
 )
 
 const (
-	NetworkDiagnosticName = "NetworkCheck"
+	NetworkDiagnosticName           = "NetworkCheck"
+	NetworkDiagLogDirParam          = "logDir"
+	NetworkDiagPodImageParam        = "podImage"
+	NetworkDiagTestPodImageParam    = "testPodImage"
+	NetworkDiagTestPodProtocolParam = "podProtocol"
+	NetworkDiagTestPodPortParam     = "podPort"
 )
 
 // NetworkDiagnostic is a diagnostic that runs a network diagnostic pod and relays the results.
@@ -59,6 +69,68 @@ func (d *NetworkDiagnostic) Description() string {
 	return "Create a pod on all schedulable nodes and run network diagnostics from the application standpoint"
 }
 
+func (d *NetworkDiagnostic) AvailableParameters() map[string]string {
+	return map[string]string{
+		NetworkDiagLogDirParam:          fmt.Sprintf("Path to store diagnostic results in case of errors (default %s)", util.NetworkDiagDefaultLogDir),
+		NetworkDiagPodImageParam:        fmt.Sprintf("Image to use for diagnostic pod (default %s)", util.GetNetworkDiagDefaultPodImage()),
+		NetworkDiagTestPodImageParam:    fmt.Sprintf("Image to use for diagnostic test pod (default %s)", util.GetNetworkDiagDefaultTestPodImage()),
+		NetworkDiagTestPodProtocolParam: fmt.Sprintf("Protocol used to connect to diagnostic test pod (default %s)", util.NetworkDiagDefaultTestPodProtocol),
+		NetworkDiagTestPodPortParam:     fmt.Sprintf("Serving port on the diagnostic test pod (default %d)", util.NetworkDiagDefaultTestPodPort),
+	}
+}
+
+func (d *NetworkDiagnostic) SetParameters(params map[string]string) error {
+	// translate parameters (if given) into the members previously filled by flags
+	if p := params[NetworkDiagLogDirParam]; p != "" {
+		d.LogDir = p
+	}
+	if p := params[NetworkDiagPodImageParam]; p != "" {
+		d.PodImage = p
+	}
+	if p := params[NetworkDiagTestPodImageParam]; p != "" {
+		d.TestPodImage = p
+	}
+	if p := params[NetworkDiagTestPodProtocolParam]; p != "" {
+		d.TestPodProtocol = p
+	}
+	if p := params[NetworkDiagTestPodPortParam]; p != "" {
+		if port, err := strconv.ParseInt(p, 0, 0); err == nil {
+			d.TestPodPort = int(port)
+		} else {
+			return fmt.Errorf("%s.%s=%s is not a valid port number: %v", NetworkDiagnosticName, NetworkDiagTestPodPortParam, p, err)
+		}
+	}
+
+	// fill in defaults, validate
+	if len(d.LogDir) == 0 {
+		d.LogDir = util.NetworkDiagDefaultLogDir
+	} else {
+		logdir, err := filepath.Abs(d.LogDir)
+		if err != nil {
+			return err
+		}
+		if path, err := os.Stat(d.LogDir); err == nil && !path.Mode().IsDir() {
+			return fmt.Errorf("Network log path %q exists but is not a directory", d.LogDir)
+		}
+		d.LogDir = logdir
+	}
+	if len(d.PodImage) == 0 {
+		d.PodImage = util.GetNetworkDiagDefaultPodImage()
+	}
+	if len(d.TestPodImage) == 0 {
+		d.TestPodImage = util.GetNetworkDiagDefaultTestPodImage()
+	}
+
+	supportedProtocols := sets.NewString(string(kapi.ProtocolTCP), string(kapi.ProtocolUDP))
+	if !supportedProtocols.Has(d.TestPodProtocol) {
+		return fmt.Errorf("invalid protocol for network diagnostic test pod. Supported protocols: %s", strings.Join(supportedProtocols.List(), ","))
+	}
+	if kvalidation.IsValidPortNum(d.TestPodPort) != nil {
+		return fmt.Errorf("invalid port for network diagnostic test pod. Must be in the range 1 to 65535.")
+	}
+	return nil
+}
+
 // CanRun is part of the Diagnostic interface; it determines if the conditions are right to run this diagnostic.
 func (d *NetworkDiagnostic) CanRun() (bool, error) {
 	if d.PreventModification {

pkg/oc/admin/diagnostics/diagnostics.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path/filepath"
 	"runtime/debug"
 	"strings"
 
@@ -13,15 +12,14 @@ import (
 
 	kutilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
-	kvalidation "k8s.io/apimachinery/pkg/util/validation"
-	kapi "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 
 	"github.com/openshift/origin/pkg/cmd/flagtypes"
 	osclientcmd "github.com/openshift/origin/pkg/cmd/util/clientcmd"
 	"github.com/openshift/origin/pkg/cmd/util/variable"
 	"github.com/openshift/origin/pkg/diagnostics/log"
+	netdiag "github.com/openshift/origin/pkg/diagnostics/network"
 	netutil "github.com/openshift/origin/pkg/diagnostics/networkpod/util"
 	"github.com/openshift/origin/pkg/diagnostics/types"
 	"github.com/openshift/origin/pkg/oc/admin/diagnostics/options"
@@ -58,7 +56,7 @@ type DiagnosticsOptions struct {
 	LogOptions *log.LoggerOptions
 	// The Logger is built with the options and should be used for all diagnostic output.
 	Logger *log.Logger
-	// Options specific to network diagnostics
+	// Options specific to network diagnostics (deprecated)
 	NetworkOptions *NetworkDiagnosticsOptions
 }
 
@@ -161,15 +159,30 @@ func NewCmdDiagnostics(name string, fullName string, out io.Writer) *cobra.Comma
 	cmd.Flags().StringVar(&o.ImageTemplate.Format, options.FlagImageTemplateName, o.ImageTemplate.Format, "Image template for DiagnosticPod to use in creating a pod")
 	cmd.Flags().BoolVar(&o.ImageTemplate.Latest, options.FlagLatestImageName, false, "If true, when expanding the image template, use latest version, not release version")
 	cmd.Flags().BoolVar(&o.PreventModification, options.FlagPreventModificationName, false, "If true, may be set to prevent diagnostics making any changes via the API")
+	o.setNetworkFlags(cmd)
+	flagtypes.GLog(cmd.Flags())
+	options.BindLoggerOptionFlags(cmd.Flags(), o.LogOptions, options.RecommendedLoggerOptionFlags())
+
+	return cmd
+}
+
+// Flags specific to NetworkCheck which are deprecated in favor of equivalent diagnostic parameters
+func (o *DiagnosticsOptions) setNetworkFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.NetworkOptions.LogDir, options.FlagNetworkDiagLogDir, netutil.NetworkDiagDefaultLogDir, "Path to store network diagnostic results in case of errors")
 	cmd.Flags().StringVar(&o.NetworkOptions.PodImage, options.FlagNetworkDiagPodImage, netutil.GetNetworkDiagDefaultPodImage(), "Image to use for network diagnostic pod")
 	cmd.Flags().StringVar(&o.NetworkOptions.TestPodImage, options.FlagNetworkDiagTestPodImage, netutil.GetNetworkDiagDefaultTestPodImage(), "Image to use for network diagnostic test pod")
 	cmd.Flags().StringVar(&o.NetworkOptions.TestPodProtocol, options.FlagNetworkDiagTestPodProtocol, netutil.NetworkDiagDefaultTestPodProtocol, "Protocol used to connect to network diagnostic test pod")
 	cmd.Flags().IntVar(&o.NetworkOptions.TestPodPort, options.FlagNetworkDiagTestPodPort, netutil.NetworkDiagDefaultTestPodPort, "Serving port on the network diagnostic test pod")
-	flagtypes.GLog(cmd.Flags())
-	options.BindLoggerOptionFlags(cmd.Flags(), o.LogOptions, options.RecommendedLoggerOptionFlags())
-
-	return cmd
+	for flag, param := range map[string]string{
+		options.FlagNetworkDiagLogDir:          netdiag.NetworkDiagLogDirParam,
+		options.FlagNetworkDiagPodImage:        netdiag.NetworkDiagPodImageParam,
+		options.FlagNetworkDiagTestPodImage:    netdiag.NetworkDiagTestPodImageParam,
+		options.FlagNetworkDiagTestPodProtocol: netdiag.NetworkDiagTestPodProtocolParam,
+		options.FlagNetworkDiagTestPodPort:     netdiag.NetworkDiagTestPodPortParam,
+	} {
+		cmd.Flags().MarkDeprecated(flag, fmt.Sprintf("please set %s.%s instead.", netdiag.NetworkDiagnosticName, param))
+		cmd.Flags().MarkHidden(flag)
+	}
 }
 
 // Complete fills in DiagnosticsOptions needed if the command is actually invoked.
@@ -193,42 +206,9 @@ func (o *DiagnosticsOptions) Complete(args []string) error {
 		}
 	}
 
-	if err = o.completeNetworkOptions(); err != nil {
-		return err
-	}
 	return o.processArgs(args)
 }
 
-func (o *DiagnosticsOptions) completeNetworkOptions() error {
-	if len(o.NetworkOptions.LogDir) == 0 {
-		o.NetworkOptions.LogDir = netutil.NetworkDiagDefaultLogDir
-	} else {
-		logdir, err := filepath.Abs(o.NetworkOptions.LogDir)
-		if err != nil {
-			return err
-		}
-		if path, err := os.Stat(o.NetworkOptions.LogDir); err == nil && !path.Mode().IsDir() {
-			return fmt.Errorf("Network log path %q exists but is not a directory", o.NetworkOptions.LogDir)
-		}
-		o.NetworkOptions.LogDir = logdir
-	}
-	if len(o.NetworkOptions.PodImage) == 0 {
-		o.NetworkOptions.PodImage = netutil.GetNetworkDiagDefaultPodImage()
-	}
-	if len(o.NetworkOptions.TestPodImage) == 0 {
-		o.NetworkOptions.TestPodImage = netutil.GetNetworkDiagDefaultTestPodImage()
-	}
-
-	supportedProtocols := sets.NewString(string(kapi.ProtocolTCP), string(kapi.ProtocolUDP))
-	if !supportedProtocols.Has(o.NetworkOptions.TestPodProtocol) {
-		return fmt.Errorf("invalid protocol for network diagnostic test pod. Supported protocols: %s", strings.Join(supportedProtocols.List(), ","))
-	}
-	if kvalidation.IsValidPortNum(o.NetworkOptions.TestPodPort) != nil {
-		return fmt.Errorf("invalid port for network diagnostic test pod. Must be in the range 1 to 65535.")
-	}
-	return nil
-}
-
 // Sort out the diagnostic names and any of their parameters.
 // oc adm diagnostics                   # all standard diagnostics
 // oc adm diagnostics Foo               # only Foo