Merge pull request #17773 from sosiouxme/20171213-diagnostic-parameters

Automatic merge from submit-queue.

diagnostics: individual parameters

Updated version of #16589 based on feedback.

This addresses #14640 by making individual diagnostics into subcommands that can have their own flags. Existing top-level flags for `NetworkCheck` are removed and the config envvar for `EtcdWriteVolume` is deprecated in favor of a flag. All individual flags are available underneath the `all` subcommand.

This required rather more refactoring as the flags had to be known in order to define the command, not just at runtime. Usages are given below:

```
$ oc adm diagnostics --help
This utility helps troubleshoot and diagnose known problems for an OpenShift cluster and/or local host. The base command
runs a standard set of diagnostics: 

  oc adm diagnostics
  
[...]

An individual diagnostic may be run as a subcommand which may have flags for specifying options specific to that
diagnostic. 

Finally, the "all" subcommand runs all available diagnostics (including heavyweight ones skipped in the standard set)
and provides all individual diagnostic flags.

Usage:
  oc adm diagnostics [options]

Available Commands:
  aggregatedlogging          Check aggregated logging integration for proper configuration
  all                        Diagnose common cluster problems
[...]
  unitstatus                 Check status for related systemd units

Options:
      --cluster-context='': Client context to use for cluster administrator
      --config='': Path to the config file to use for CLI requests.
      --context='': The name of the kubeconfig context to use
  -l, --diaglevel=1: Level of diagnostic output: 4: Error, 3: Warn, 2: Notice, 1: Info, 0: Debug
      --host=false: If true, look for systemd and journald units even without master/node config
      --loglevel=0: Set the level of log output (0-10)
      --logspec='': Set per module logging with file|pattern=LEVEL,...
      --master-config='': Path to master config file (implies --host)
      --node-config='': Path to node config file (implies --host)
      --prevent-modification=false: If true, may be set to prevent diagnostics making any changes via the API
```

(Note `all` is now intermingled with the individual subcommands.)

```
$ oc adm diagnostics all --help
This utility helps troubleshoot and diagnose known problems for an OpenShift cluster and/or local host. This subcommand
exists to run all available diagnostics: 

  oc adm diagnostics all
  
Available diagnostics vary based on client config and local OpenShift host config. All flags from the base command work
similarly here, but all possible flags for individual diagnostics are also available.

Usage:
  oc adm diagnostics all [options]

Options:
      --cluster-context='': Client context to use for cluster administrator
      --config='': Path to the config file to use for CLI requests.
      --context='': The name of the kubeconfig context to use
  -l, --diaglevel=1: Level of diagnostic output: 4: Error, 3: Warn, 2: Notice, 1: Info, 0: Debug
      --diagnosticpod-images='openshift/origin-${component}:${version}': Image template to use in creating a pod
      --diagnosticpod-latest-images=false: If true, when expanding the image template, use latest version, not release
version
      --etcdwritevolume-duration='1m': How long to perform the write test
      --host=false: If true, look for systemd and journald units even without master/node config
      --loglevel=0: Set the level of log output (0-10)
      --logspec='': Set per module logging with file|pattern=LEVEL,...
      --master-config='': Path to master config file (implies --host)
      --networkcheck-logdir='/tmp/openshift/': Path to store diagnostic results in case of errors
      --networkcheck-pod-image='openshift/origin:v3.9.0-alpha.0': Image to use for diagnostic pod
      --networkcheck-test-pod-image='openshift/origin-deployer:v3.9.0-alpha.0': Image to use for diagnostic test pod
      --networkcheck-test-pod-port=8080: Serving port on the diagnostic test pod
      --networkcheck-test-pod-protocol='TCP': Protocol used to connect to diagnostic test pod
      --node-config='': Path to node config file (implies --host)
      --prevent-modification=false: If true, may be set to prevent diagnostics making any changes via the API
```

```
$ oc adm diagnostics EtcdWriteVolume --help
Runs the EtcdWriteVolume diagnostic. 

Check the volume of writes against etcd over a time period and classify them by operation and key

Aliases:
etcdwritevolume, EtcdWriteVolume

Usage:
  oc adm diagnostics etcdwritevolume [options]

Options:
  -l, --diaglevel=1: Level of diagnostic output: 4: Error, 3: Warn, 2: Notice, 1: Info, 0: Debug
      --duration='1m': How long to perform the write test
      --host=false: If true, look for systemd and journald units even without master/node config
      --loglevel=0: Set the level of log output (0-10)
      --logspec='': Set per module logging with file|pattern=LEVEL,...
      --master-config='': Path to master config file (implies --host)
      --node-config='': Path to node config file (implies --host)
```

```
$ oc adm diagnostics NetworkCheck --help
Runs the NetworkCheck diagnostic. 

Create a pod on all schedulable nodes and run network diagnostics from the application standpoint

Aliases:
networkcheck, NetworkCheck

Usage:
  oc adm diagnostics networkcheck [options]

Options:
      --cluster-context='': Client context to use for cluster administrator
      --config='': Path to the config file to use for CLI requests.
      --context='': The name of the kubeconfig context to use
  -l, --diaglevel=1: Level of diagnostic output: 4: Error, 3: Warn, 2: Notice, 1: Info, 0: Debug
      --logdir='/tmp/openshift/': Path to store diagnostic results in case of errors
      --loglevel=0: Set the level of log output (0-10)
      --logspec='': Set per module logging with file|pattern=LEVEL,...
      --pod-image='openshift/origin:v3.9.0-alpha.0': Image to use for diagnostic pod
      --prevent-modification=false: If true, may be set to prevent diagnostics making any changes via the API
      --test-pod-image='openshift/origin-deployer:v3.9.0-alpha.0': Image to use for diagnostic test pod
      --test-pod-port=8080: Serving port on the diagnostic test pod
      --test-pod-protocol='TCP': Protocol used to connect to diagnostic test pod
```

pkg/oc/admin/diagnostics/client.go

@@ -11,16 +11,16 @@ import (
 	"github.com/openshift/origin/pkg/oc/admin/diagnostics/diagnostics/types"
 )
 
-var (
-	// availableClientDiagnostics contains the names of client diagnostics that can be executed
-	// during a single run of diagnostics. Add more diagnostics to the list as they are defined.
-	availableClientDiagnostics = sets.NewString(clientdiags.ConfigContextsName, clientdiags.DiagnosticPodName, networkdiags.NetworkDiagnosticName)
-)
+// availableClientDiagnostics returns definitions of client diagnostics that can be executed
+// during a single run of diagnostics. Add more diagnostics to the list as they are defined.
+func availableClientDiagnostics() types.DiagnosticList {
+	return types.DiagnosticList{clientdiags.ConfigContext{}, &clientdiags.DiagnosticPod{}, &networkdiags.NetworkDiagnostic{}}
+}
 
 // buildClientDiagnostics builds client Diagnostic objects based on the rawConfig passed in.
 // Returns the Diagnostics built, "ok" bool for whether to proceed or abort, and an error if any was encountered during the building of diagnostics.) {
 func (o DiagnosticsOptions) buildClientDiagnostics(rawConfig *clientcmdapi.Config) ([]types.Diagnostic, bool, error) {
-	available := availableClientDiagnostics
+	available := availableClientDiagnostics().Names()
 
 	networkClient, err := o.Factory.OpenshiftInternalNetworkClient()
 	kubeClient, clientErr := o.Factory.ClientSet()
@@ -30,7 +30,7 @@ func (o DiagnosticsOptions) buildClientDiagnostics(rawConfig *clientcmdapi.Confi
 	}
 
 	diagnostics := []types.Diagnostic{}
-	requestedDiagnostics := available.Intersection(sets.NewString(o.RequestedDiagnostics...)).List()
+	requestedDiagnostics := available.Intersection(sets.NewString(o.RequestedDiagnostics.List()...)).List()
 	for _, diagnosticName := range requestedDiagnostics {
 		switch diagnosticName {
 		case clientdiags.ConfigContextsName:
@@ -46,29 +46,23 @@ func (o DiagnosticsOptions) buildClientDiagnostics(rawConfig *clientcmdapi.Confi
 				}
 			}
 		case clientdiags.DiagnosticPodName:
-			diagnostics = append(diagnostics, &clientdiags.DiagnosticPod{
-				KubeClient:          kubeClient,
-				Namespace:           rawConfig.Contexts[rawConfig.CurrentContext].Namespace,
-				Level:               o.LogOptions.Level,
-				Factory:             o.Factory,
-				PreventModification: o.PreventModification,
-				ImageTemplate:       o.ImageTemplate,
-			})
+			dp := o.ParameterizedDiagnostics[diagnosticName].(*clientdiags.DiagnosticPod)
+			dp.KubeClient = kubeClient
+			dp.Namespace = rawConfig.Contexts[rawConfig.CurrentContext].Namespace
+			dp.Level = o.LogOptions.Level
+			dp.Factory = o.Factory
+			dp.PreventModification = dp.PreventModification || o.PreventModification
+			diagnostics = append(diagnostics, dp)
 		case networkdiags.NetworkDiagnosticName:
-			diagnostics = append(diagnostics, &networkdiags.NetworkDiagnostic{
-				KubeClient:           kubeClient,
-				NetNamespacesClient:  networkClient.Network(),
-				ClusterNetworkClient: networkClient.Network(),
-				ClientFlags:          o.ClientFlags,
-				Level:                o.LogOptions.Level,
-				Factory:              o.Factory,
-				PreventModification:  o.PreventModification,
-				LogDir:               o.NetworkOptions.LogDir,
-				PodImage:             o.NetworkOptions.PodImage,
-				TestPodImage:         o.NetworkOptions.TestPodImage,
-				TestPodProtocol:      o.NetworkOptions.TestPodProtocol,
-				TestPodPort:          o.NetworkOptions.TestPodPort,
-			})
+			nd := o.ParameterizedDiagnostics[diagnosticName].(*networkdiags.NetworkDiagnostic)
+			nd.KubeClient = kubeClient
+			nd.NetNamespacesClient = networkClient.Network()
+			nd.ClusterNetworkClient = networkClient.Network()
+			nd.ClientFlags = o.ClientFlags
+			nd.Level = o.LogOptions.Level
+			nd.Factory = o.Factory
+			nd.PreventModification = o.PreventModification
+			diagnostics = append(diagnostics, nd)
 		default:
 			return nil, false, fmt.Errorf("unknown diagnostic: %v", diagnosticName)
 		}

pkg/oc/admin/diagnostics/cluster.go

@@ -25,27 +25,27 @@ import (
 	"k8s.io/kubernetes/pkg/apis/authorization"
 )
 
-var (
-	// availableClusterDiagnostics contains the names of cluster diagnostics that can be executed
-	// during a single run of diagnostics. Add more diagnostics to the list as they are defined.
-	availableClusterDiagnostics = sets.NewString(
-		agldiags.AggregatedLoggingName,
-		clustdiags.ClusterRegistryName,
-		clustdiags.ClusterRouterName,
-		clustdiags.ClusterRolesName,
-		clustdiags.ClusterRoleBindingsName,
-		clustdiags.MasterNodeName,
-		clustdiags.MetricsApiProxyName,
-		clustdiags.NodeDefinitionsName,
-		clustdiags.RouteCertificateValidationName,
-		clustdiags.ServiceExternalIPsName,
-	)
-)
+// availableClusterDiagnostics contains the names of cluster diagnostics that can be executed
+// during a single run of diagnostics. Add more diagnostics to the list as they are defined.
+func availableClusterDiagnostics() types.DiagnosticList {
+	return types.DiagnosticList{
+		&agldiags.AggregatedLogging{},
+		&clustdiags.ClusterRegistry{},
+		&clustdiags.ClusterRouter{},
+		&clustdiags.ClusterRoles{},
+		&clustdiags.ClusterRoleBindings{},
+		&clustdiags.MasterNode{},
+		&clustdiags.MetricsApiProxy{},
+		&clustdiags.NodeDefinitions{},
+		&clustdiags.RouteCertificateValidation{},
+		&clustdiags.ServiceExternalIPs{},
+	}
+}
 
 // buildClusterDiagnostics builds cluster Diagnostic objects if a cluster-admin client can be extracted from the rawConfig passed in.
 // Returns the Diagnostics built, "ok" bool for whether to proceed or abort, and an error if any was encountered during the building of diagnostics.) {
 func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Config) ([]types.Diagnostic, bool, error) {
-	requestedDiagnostics := availableClusterDiagnostics.Intersection(sets.NewString(o.RequestedDiagnostics...)).List()
+	requestedDiagnostics := availableClusterDiagnostics().Names().Intersection(sets.NewString(o.RequestedDiagnostics.List()...)).List()
 	if len(requestedDiagnostics) == 0 { // no diagnostics to run here
 		return nil, true, nil // don't waste time on discovery
 	}

pkg/oc/admin/diagnostics/config.go

@@ -11,14 +11,17 @@ import (
 )
 
 // determine if we even have a client config
-func (o DiagnosticsOptions) detectClientConfig() (bool, []types.DiagnosticError, []types.DiagnosticError) {
+func (o DiagnosticsOptions) detectClientConfig() (bool, bool, []types.DiagnosticError, []types.DiagnosticError) {
+	if o.ClientFlags == nil {
+		return false, false, []types.DiagnosticError{}, []types.DiagnosticError{}
+	}
 	diagnostic := &clientdiagnostics.ConfigLoading{ConfFlagName: config.OpenShiftConfigFlagName, ClientFlags: o.ClientFlags}
 	o.Logger.Notice("CED2011", "Determining if client configuration exists for client/cluster diagnostics")
 	result := diagnostic.Check()
 	for _, entry := range result.Logs() {
 		o.Logger.LogEntry(entry)
 	}
-	return diagnostic.SuccessfulLoad(), result.Warnings(), result.Errors()
+	return true, diagnostic.SuccessfulLoad(), result.Warnings(), result.Errors()
 }
 
 // use the base factory to return a raw config (not specific to a context)