From 0b9d917f73ed9de61f5f3bb69aecb432ac2bd2fd Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 12 Jan 2016 12:45:08 -0500 Subject: [PATCH 1/3] mark validation commands as deprecated --- contrib/completions/bash/openshift | 204 ----------------------------- pkg/cmd/admin/validate/master.go | 11 +- pkg/cmd/admin/validate/node.go | 11 +- pkg/cmd/admin/validate/validate.go | 18 ++- 4 files changed, 26 insertions(+), 218 deletions(-) diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index b1c40906956e..f9b1b49db7e1 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -13435,209 +13435,6 @@ _openshift_kube() must_have_one_noun=() } -_openshift_ex_validate_master-config() -{ - last_command="openshift_ex_validate_master-config" - commands=() - - flags=() - two_word_flags=() - flags_with_completion=() - flags_completion=() - - flags+=("--alsologtostderr") - flags+=("--api-version=") - flags+=("--boot-id-file=") - flags+=("--certificate-authority=") - flags_with_completion+=("--certificate-authority") - flags_completion+=("_filedir") - flags+=("--client-certificate=") - flags_with_completion+=("--client-certificate") - flags_completion+=("_filedir") - flags+=("--client-key=") - flags_with_completion+=("--client-key") - flags_completion+=("_filedir") - flags+=("--cluster=") - flags+=("--config=") - flags_with_completion+=("--config") - flags_completion+=("_filedir") - flags+=("--container-hints=") - flags+=("--context=") - flags+=("--docker=") - flags+=("--docker-only") - flags+=("--docker-root=") - flags+=("--docker-run=") - flags+=("--enable-load-reader") - flags+=("--event-storage-age-limit=") - flags+=("--event-storage-event-limit=") - flags+=("--global-housekeeping-interval=") - flags+=("--google-json-key=") - flags+=("--housekeeping-interval=") - flags+=("--httptest.serve=") - flags+=("--insecure-skip-tls-verify") - flags+=("--ir-data-source=") - flags+=("--ir-dbname=") - flags+=("--ir-influxdb-host=") - flags+=("--ir-namespace-only") - flags+=("--ir-password=") - flags+=("--ir-percentile=") - flags+=("--ir-user=") - flags+=("--log-backtrace-at=") - flags+=("--log-cadvisor-usage") - flags+=("--log-dir=") - flags+=("--log-flush-frequency=") - flags+=("--logtostderr") - flags+=("--machine-id-file=") - flags+=("--match-server-version") - flags+=("--namespace=") - two_word_flags+=("-n") - flags+=("--server=") - flags+=("--stderrthreshold=") - flags+=("--token=") - flags+=("--user=") - flags+=("--v=") - flags+=("--vmodule=") - - must_have_one_flag=() - must_have_one_noun=() -} - -_openshift_ex_validate_node-config() -{ - last_command="openshift_ex_validate_node-config" - commands=() - - flags=() - two_word_flags=() - flags_with_completion=() - flags_completion=() - - flags+=("--alsologtostderr") - flags+=("--api-version=") - flags+=("--boot-id-file=") - flags+=("--certificate-authority=") - flags_with_completion+=("--certificate-authority") - flags_completion+=("_filedir") - flags+=("--client-certificate=") - flags_with_completion+=("--client-certificate") - flags_completion+=("_filedir") - flags+=("--client-key=") - flags_with_completion+=("--client-key") - flags_completion+=("_filedir") - flags+=("--cluster=") - flags+=("--config=") - flags_with_completion+=("--config") - flags_completion+=("_filedir") - flags+=("--container-hints=") - flags+=("--context=") - flags+=("--docker=") - flags+=("--docker-only") - flags+=("--docker-root=") - flags+=("--docker-run=") - flags+=("--enable-load-reader") - flags+=("--event-storage-age-limit=") - flags+=("--event-storage-event-limit=") - flags+=("--global-housekeeping-interval=") - flags+=("--google-json-key=") - flags+=("--housekeeping-interval=") - flags+=("--httptest.serve=") - flags+=("--insecure-skip-tls-verify") - flags+=("--ir-data-source=") - flags+=("--ir-dbname=") - flags+=("--ir-influxdb-host=") - flags+=("--ir-namespace-only") - flags+=("--ir-password=") - flags+=("--ir-percentile=") - flags+=("--ir-user=") - flags+=("--log-backtrace-at=") - flags+=("--log-cadvisor-usage") - flags+=("--log-dir=") - flags+=("--log-flush-frequency=") - flags+=("--logtostderr") - flags+=("--machine-id-file=") - flags+=("--match-server-version") - flags+=("--namespace=") - two_word_flags+=("-n") - flags+=("--server=") - flags+=("--stderrthreshold=") - flags+=("--token=") - flags+=("--user=") - flags+=("--v=") - flags+=("--vmodule=") - - must_have_one_flag=() - must_have_one_noun=() -} - -_openshift_ex_validate() -{ - last_command="openshift_ex_validate" - commands=() - commands+=("master-config") - commands+=("node-config") - - flags=() - two_word_flags=() - flags_with_completion=() - flags_completion=() - - flags+=("--alsologtostderr") - flags+=("--api-version=") - flags+=("--boot-id-file=") - flags+=("--certificate-authority=") - flags_with_completion+=("--certificate-authority") - flags_completion+=("_filedir") - flags+=("--client-certificate=") - flags_with_completion+=("--client-certificate") - flags_completion+=("_filedir") - flags+=("--client-key=") - flags_with_completion+=("--client-key") - flags_completion+=("_filedir") - flags+=("--cluster=") - flags+=("--config=") - flags_with_completion+=("--config") - flags_completion+=("_filedir") - flags+=("--container-hints=") - flags+=("--context=") - flags+=("--docker=") - flags+=("--docker-only") - flags+=("--docker-root=") - flags+=("--docker-run=") - flags+=("--enable-load-reader") - flags+=("--event-storage-age-limit=") - flags+=("--event-storage-event-limit=") - flags+=("--global-housekeeping-interval=") - flags+=("--google-json-key=") - flags+=("--housekeeping-interval=") - flags+=("--httptest.serve=") - flags+=("--insecure-skip-tls-verify") - flags+=("--ir-data-source=") - flags+=("--ir-dbname=") - flags+=("--ir-influxdb-host=") - flags+=("--ir-namespace-only") - flags+=("--ir-password=") - flags+=("--ir-percentile=") - flags+=("--ir-user=") - flags+=("--log-backtrace-at=") - flags+=("--log-cadvisor-usage") - flags+=("--log-dir=") - flags+=("--log-flush-frequency=") - flags+=("--logtostderr") - flags+=("--machine-id-file=") - flags+=("--match-server-version") - flags+=("--namespace=") - two_word_flags+=("-n") - flags+=("--server=") - flags+=("--stderrthreshold=") - flags+=("--token=") - flags+=("--user=") - flags+=("--v=") - flags+=("--vmodule=") - - must_have_one_flag=() - must_have_one_noun=() -} - _openshift_ex_tokens_validate-token() { last_command="openshift_ex_tokens_validate-token" @@ -14319,7 +14116,6 @@ _openshift_ex() { last_command="openshift_ex" commands=() - commands+=("validate") commands+=("tokens") commands+=("ipfailover") commands+=("build-chain") diff --git a/pkg/cmd/admin/validate/master.go b/pkg/cmd/admin/validate/master.go index 0db3a36d6371..fb87b8bfe389 100644 --- a/pkg/cmd/admin/validate/master.go +++ b/pkg/cmd/admin/validate/master.go @@ -27,6 +27,8 @@ This command validates that a configuration file intended to be used for a maste validateMasterConfigExample = ` // Validate master server configuration file $ %s openshift.local.config/master/master-config.yaml` + + validateMasterConfigDeprecationMessage = `This command is deprecated and will be removed. Use 'openshift ex diagnostics MasterConfigCheck --master-config=path/to/config.yaml' instead.` ) type ValidateMasterConfigOptions struct { @@ -44,10 +46,11 @@ func NewCommandValidateMasterConfig(name, fullName string, out io.Writer) *cobra } cmd := &cobra.Command{ - Use: fmt.Sprintf("%s SOURCE", name), - Short: "Validate the configuration file for a master server", - Long: validateMasterConfigLong, - Example: fmt.Sprintf(validateMasterConfigExample, fullName), + Use: fmt.Sprintf("%s SOURCE", name), + Short: "Validate the configuration file for a master server", + Long: validateMasterConfigLong, + Example: fmt.Sprintf(validateMasterConfigExample, fullName), + Deprecated: validateMasterConfigDeprecationMessage, Run: func(c *cobra.Command, args []string) { if err := options.Complete(args); err != nil { cmdutil.CheckErr(cmdutil.UsageError(c, err.Error())) diff --git a/pkg/cmd/admin/validate/node.go b/pkg/cmd/admin/validate/node.go index 215f292c9a81..b86cb7f0dd6d 100644 --- a/pkg/cmd/admin/validate/node.go +++ b/pkg/cmd/admin/validate/node.go @@ -26,6 +26,8 @@ This command validates that a configuration file intended to be used for a node valiateNodeConfigExample = ` // Validate node configuration file $ %s openshift.local.config/master/node-config.yaml` + + validateNodeConfigDeprecationMessage = `This command is deprecated and will be removed. Use 'openshift ex diagnostics NodeConfigCheck --node-config= Date: Wed, 13 Jan 2016 08:51:39 -0500 Subject: [PATCH 2/3] accept diagnostics as args --- contrib/completions/bash/openshift | 2 - pkg/cmd/admin/validate/node.go | 2 +- pkg/cmd/experimental/diagnostics/client.go | 2 +- pkg/cmd/experimental/diagnostics/cluster.go | 2 +- .../experimental/diagnostics/diagnostics.go | 100 +++++++++--------- pkg/cmd/experimental/diagnostics/host.go | 2 +- .../diagnostics/options/diagnostics.go | 10 -- .../diagnostics/options/flaginfo.go | 1 - pkg/cmd/experimental/diagnostics/pod.go | 12 ++- test/cmd/admin.sh | 7 +- 10 files changed, 70 insertions(+), 70 deletions(-) diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index f9b1b49db7e1..0fb0af57e106 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -13819,8 +13819,6 @@ _openshift_ex_diagnostics() flags+=("--context=") flags+=("--diaglevel=") two_word_flags+=("-l") - flags+=("--diagnostics=") - two_word_flags+=("-d") flags+=("--host") flags+=("--images=") flags+=("--latest-images") diff --git a/pkg/cmd/admin/validate/node.go b/pkg/cmd/admin/validate/node.go index b86cb7f0dd6d..aa4c325ba920 100644 --- a/pkg/cmd/admin/validate/node.go +++ b/pkg/cmd/admin/validate/node.go @@ -27,7 +27,7 @@ This command validates that a configuration file intended to be used for a node valiateNodeConfigExample = ` // Validate node configuration file $ %s openshift.local.config/master/node-config.yaml` - validateNodeConfigDeprecationMessage = `This command is deprecated and will be removed. Use 'openshift ex diagnostics NodeConfigCheck --node-config= 0, nil, warnCount, errorCount } - -// TODO move upstream -func intersection(s1 sets.String, s2 sets.String) sets.String { - result := sets.NewString() - for key := range s1 { - if s2.Has(key) { - result.Insert(key) - } - } - return result -} diff --git a/pkg/cmd/experimental/diagnostics/host.go b/pkg/cmd/experimental/diagnostics/host.go index 4b4b2535ac6e..7da46dd5a765 100644 --- a/pkg/cmd/experimental/diagnostics/host.go +++ b/pkg/cmd/experimental/diagnostics/host.go @@ -19,7 +19,7 @@ var ( // buildHostDiagnostics builds host Diagnostic objects based on the host environment. // 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) buildHostDiagnostics() ([]types.Diagnostic, bool, error) { - requestedDiagnostics := intersection(sets.NewString(o.RequestedDiagnostics...), availableHostDiagnostics).List() + requestedDiagnostics := availableHostDiagnostics.Intersection(sets.NewString(o.RequestedDiagnostics...)).List() if len(requestedDiagnostics) == 0 { // no diagnostics to run here return nil, true, nil // don't waste time on discovery } diff --git a/pkg/cmd/experimental/diagnostics/options/diagnostics.go b/pkg/cmd/experimental/diagnostics/options/diagnostics.go index 3687a8771291..bf38f7e3a0bf 100644 --- a/pkg/cmd/experimental/diagnostics/options/diagnostics.go +++ b/pkg/cmd/experimental/diagnostics/options/diagnostics.go @@ -22,13 +22,3 @@ func RecommendedLoggerOptionFlags() LoggerOptionFlags { func BindLoggerOptionFlags(cmdFlags *pflag.FlagSet, loggerOptions *log.LoggerOptions, flags LoggerOptionFlags) { flags.Level.BindIntFlag(cmdFlags, &loggerOptions.Level) } - -// NewRecommendedDiagnosticFlag provides default overrideable Diagnostic flag specifications to be bound to options. -func NewRecommendedDiagnosticFlag() FlagInfo { - return FlagInfo{FlagDiagnosticsName, "d", "", `Comma-separated list of diagnostic names to run, e.g. "AnalyzeLogs"`} -} - -// BindLoggerOptionFlags binds a flag on a diagnostics command per the flagInfo. -func BindDiagnosticFlag(cmdFlags *pflag.FlagSet, diagnostics *[]string, flagInfo FlagInfo) { - flagInfo.BindListFlag(cmdFlags, diagnostics) -} diff --git a/pkg/cmd/experimental/diagnostics/options/flaginfo.go b/pkg/cmd/experimental/diagnostics/options/flaginfo.go index 72e2b777f8ed..b2cc144fefcb 100644 --- a/pkg/cmd/experimental/diagnostics/options/flaginfo.go +++ b/pkg/cmd/experimental/diagnostics/options/flaginfo.go @@ -48,7 +48,6 @@ const ( FlagMasterConfigName = "master-config" FlagNodeConfigName = "node-config" FlagClusterContextName = "cluster-context" - FlagDiagnosticsName = "diagnostics" FlagLevelName = "diaglevel" FlagIsHostName = "host" FlagImageTemplateName = "images" diff --git a/pkg/cmd/experimental/diagnostics/pod.go b/pkg/cmd/experimental/diagnostics/pod.go index 9a61aa69fee5..723e7ddb5667 100644 --- a/pkg/cmd/experimental/diagnostics/pod.go +++ b/pkg/cmd/experimental/diagnostics/pod.go @@ -53,7 +53,7 @@ func NewCommandPodDiagnostics(name string, out io.Writer) *cobra.Command { Short: "Within a pod, run pod diagnostics", Long: fmt.Sprintf(longPodDiagDescription), Run: func(c *cobra.Command, args []string) { - kcmdutil.CheckErr(o.Complete()) + kcmdutil.CheckErr(o.Complete(args)) failed, err, warnCount, errorCount := o.BuildAndRunDiagnostics() o.Logger.Summary(warnCount, errorCount) @@ -68,19 +68,23 @@ func NewCommandPodDiagnostics(name string, out io.Writer) *cobra.Command { cmd.SetOutput(out) // for output re: usage / help options.BindLoggerOptionFlags(cmd.Flags(), o.LogOptions, options.RecommendedLoggerOptionFlags()) - options.BindDiagnosticFlag(cmd.Flags(), &o.RequestedDiagnostics, options.NewRecommendedDiagnosticFlag()) return cmd } // Complete fills in PodDiagnosticsOptions needed if the command is actually invoked. -func (o *PodDiagnosticsOptions) Complete() error { +func (o *PodDiagnosticsOptions) Complete(args []string) error { var err error o.Logger, err = o.LogOptions.NewLogger() if err != nil { return err } + o.RequestedDiagnostics = append(o.RequestedDiagnostics, args...) + if len(o.RequestedDiagnostics) == 0 { + o.RequestedDiagnostics = availablePodDiagnostics.List() + } + return nil } @@ -196,7 +200,7 @@ func determineRequestedDiagnostics(available []string, requested []string, logge diagnostics := []string{} if len(requested) == 0 { // not specified, use the available list diagnostics = available - } else if diagnostics = intersection(sets.NewString(requested...), sets.NewString(available...)).List(); len(diagnostics) == 0 { + } else if diagnostics = sets.NewString(requested...).Intersection(sets.NewString(available...)).List(); len(diagnostics) == 0 { logger.Error("CED6001", log.EvalTemplate("CED6001", "None of the requested diagnostics are available:\n {{.requested}}\nPlease try from the following:\n {{.available}}", log.Hash{"requested": requested, "available": available})) return fmt.Errorf("No requested diagnostics available"), diagnostics diff --git a/test/cmd/admin.sh b/test/cmd/admin.sh index ebf90f0ca8c6..9bbf2eb20004 100755 --- a/test/cmd/admin.sh +++ b/test/cmd/admin.sh @@ -9,7 +9,7 @@ source "${OS_ROOT}/hack/util.sh" source "${OS_ROOT}/hack/cmd_util.sh" os::log::install_errexit -# Cleanup cluster resources created by this test +#Cleanup cluster resources created by this test ( set +e oc delete project/example project/ui-test-project project/recreated-project @@ -300,3 +300,8 @@ os::cmd::expect_success_and_not_text "oc get clusterrolebindings/cluster-admins os::cmd::expect_success_and_not_text "oc get rolebindings/cluster-admin --output-version=v1 --template='{{.subjects}}' -n default" 'cascaded-group' os::cmd::expect_success_and_not_text "oc get scc/restricted --output-version=v1 --template='{{.groups}}'" 'cascaded-group' echo "user-group-cascade: ok" + + +os::cmd::expect_failure_and_text 'openshift ex diagnostics FakeDiagnostic AlsoMissing' 'No requested diagnostics are available: requested=FakeDiagnostic AlsoMissing' +os::cmd::expect_failure_and_text 'openshift ex diagnostics AnalyzeLogs AlsoMissing' 'Not all requested diagnostics are available: missing=AlsoMissing requested=AnalyzeLogs AlsoMissing available=' +echo "diagnostics: ok" From fac65bef5ec42b0f8ea648bfebb4152d06cb760a Mon Sep 17 00:00:00 2001 From: Luke Meyer Date: Fri, 22 Jan 2016 12:34:46 -0500 Subject: [PATCH 3/3] cmd tests: fix diagnostics tests --- contrib/completions/bash/openshift | 2 -- test/cmd/admin.sh | 4 ---- test/cmd/diagnostics.sh | 15 +++++++++------ test/end-to-end/core.sh | 2 +- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index 0fb0af57e106..31a2355b16d7 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -14474,8 +14474,6 @@ _openshift_infra_diagnostic-pod() flags+=("--diaglevel=") two_word_flags+=("-l") - flags+=("--diagnostics=") - two_word_flags+=("-d") flags+=("--google-json-key=") flags+=("--log-flush-frequency=") diff --git a/test/cmd/admin.sh b/test/cmd/admin.sh index 9bbf2eb20004..9e22dd6ba375 100755 --- a/test/cmd/admin.sh +++ b/test/cmd/admin.sh @@ -301,7 +301,3 @@ os::cmd::expect_success_and_not_text "oc get rolebindings/cluster-admin os::cmd::expect_success_and_not_text "oc get scc/restricted --output-version=v1 --template='{{.groups}}'" 'cascaded-group' echo "user-group-cascade: ok" - -os::cmd::expect_failure_and_text 'openshift ex diagnostics FakeDiagnostic AlsoMissing' 'No requested diagnostics are available: requested=FakeDiagnostic AlsoMissing' -os::cmd::expect_failure_and_text 'openshift ex diagnostics AnalyzeLogs AlsoMissing' 'Not all requested diagnostics are available: missing=AlsoMissing requested=AnalyzeLogs AlsoMissing available=' -echo "diagnostics: ok" diff --git a/test/cmd/diagnostics.sh b/test/cmd/diagnostics.sh index c011b78fa09c..38f92648d790 100755 --- a/test/cmd/diagnostics.sh +++ b/test/cmd/diagnostics.sh @@ -16,12 +16,15 @@ os::log::install_errexit # Without things feeding into systemd, AnalyzeLogs and UnitStatus are irrelevant. # The rest should be included in some fashion. -os::cmd::expect_success 'openshift ex diagnostics -d ClusterRoleBindings,ClusterRoles,ConfigContexts ' +os::cmd::expect_success 'openshift ex diagnostics ClusterRoleBindings ClusterRoles ConfigContexts ' # DiagnosticPod can't run without Docker, would just time out. Exercise flags instead. -os::cmd::expect_success "openshift ex diagnostics -d DiagnosticPod --prevent-modification --images=foo" -os::cmd::expect_success "openshift ex diagnostics -d MasterConfigCheck,NodeConfigCheck --master-config=${MASTER_CONFIG_DIR}/master-config.yaml --node-config=${NODE_CONFIG_DIR}/node-config.yaml" -os::cmd::expect_success_and_text 'openshift ex diagnostics -d ClusterRegistry' "DClu1002 from diagnostic ClusterRegistry" +os::cmd::expect_success "openshift ex diagnostics DiagnosticPod --prevent-modification --images=foo" +os::cmd::expect_success "openshift ex diagnostics MasterConfigCheck NodeConfigCheck --master-config=${MASTER_CONFIG_DIR}/master-config.yaml --node-config=${NODE_CONFIG_DIR}/node-config.yaml" +os::cmd::expect_success_and_text 'openshift ex diagnostics ClusterRegistry' "DClu1002 from diagnostic ClusterRegistry" # ClusterRouter fails differently depending on whether other tests have run first, so don't test for specific error -os::cmd::expect_failure 'openshift ex diagnostics -d ClusterRouter' # "DClu2001 from diagnostic ClusterRouter" -os::cmd::expect_failure 'openshift ex diagnostics -d NodeDefinitions' +# no ordering allowed +#os::cmd::expect_failure 'openshift ex diagnostics ClusterRouter' # "DClu2001 from diagnostic ClusterRouter" +os::cmd::expect_failure 'openshift ex diagnostics NodeDefinitions' +os::cmd::expect_failure_and_text 'openshift ex diagnostics FakeDiagnostic AlsoMissing' 'No requested diagnostics are available: requested=FakeDiagnostic AlsoMissing' +os::cmd::expect_failure_and_text 'openshift ex diagnostics AnalyzeLogs AlsoMissing' 'Not all requested diagnostics are available: missing=AlsoMissing requested=AnalyzeLogs AlsoMissing available=' echo "diagnostics: ok" diff --git a/test/end-to-end/core.sh b/test/end-to-end/core.sh index d2ede779ded5..a090bdd54415 100755 --- a/test/end-to-end/core.sh +++ b/test/end-to-end/core.sh @@ -183,7 +183,7 @@ oc logs --version=1 dc/failing-dc echo "[INFO] Run pod diagnostics" # Requires a node to run the pod; uses origin-deployer pod, expects registry deployed -openshift ex diagnostics -d DiagnosticPod --images="${USE_IMAGES}" +openshift ex diagnostics DiagnosticPod --images="${USE_IMAGES}" echo "[INFO] Applying STI application config" oc create -f "${STI_CONFIG_FILE}"