Make Svcat print help text if command is incorrect

[jichenjc]

Make Svcat print help text if command is incorrect

[duglin]

Can you please use a more descriptive title. "Issue/2166" doesn't mean anything w/o a lot more clicks/work.

[jichenjc]

Thanks for the reminder, I know gerrit process but I am brand new to github process :(
I will refer to other submit patches ...

[MHBauer]

@carolynvs @jberkhahn close or hold?
@jichenjc this needs to be gofmt'd according to the build result.

[jichenjc]

ok, I ran make format locally and this should be ok now

[carolynvs]

@jichenjc There is one more change that needs to be made to make the build happy

https://travis-ci.org/kubernetes-incubator/service-catalog/jobs/403481082#L582

[carolynvs]

I just took a look at what you had to do in order to make this work. Let me check and see if we have a way to do this universally for all commands such that when validate fails, we print the help text.

[carolynvs]

Found it!

https://github.com/kubernetes-incubator/service-catalog/blob/7b9f8bd470031d9506cddb5928a665b9d53f9687/cmd/svcat/command/command.go#L45-L75

This is a function used by ALL commands and you should be able to test for an error (instead of returning it directly) and then displaying the command help. You have access to the command directly in that function, so no other changes are necessary to the individual commands themselves.

[carolynvs]

Let's switch this to work for all commands (see my comment on the PR with a link to the shared function that will let you do that).

[jichenjc]

ok, I need check this code and learn how it works ... will submit a patch after I get it :), thanks ~

[jichenjc]

my local test seems better now with a few lines change only

root@k8s:~/go/src/github.com/kubernetes-incubator/service-catalog# bin/svcat/svcat provision
an instance name is required
Usage:
svcat provision NAME --plan PLAN --class CLASS [flags]

Examples:
svcat provision wordpress-mysql-instance --class mysqldb --plan free -p location=eastus -p sslEnforcement=disabled
svcat provision wordpress-mysql-instance --external-id a7c00676-4398-11e8-842f-0ed5f89f718b --class mysqldb --plan free
svcat provision wordpress-mysql-instance --class mysqldb --plan free -s mysecret[dbparams]
svcat provision secure-instance --class mysqldb --plan secureDB --params-json '{
"encrypt" : true,
"firewallRules" : [
{
"name": "AllowSome",
"startIPAddress": "75.70.113.50",
"endIPAddress" : "75.70.113.131"
}
]
}'

Flags:
--class string The class name (Required)
--external-id string The ID of the instance for use with the OSB SB API (Optional)
-h, --help help for provision
--interval string Poll interval for --wait, specified in human readable format: 30s, 1m, 1h (default "1s")
-n, --namespace string If present, the namespace scope for this request
-p, --param strings Additional parameter to use when provisioning the service, format: NAME=VALUE. Cannot be combined with --params-json, Sensitive information should be placed in a secret and specified with --secret
--params-json string Additional parameters to use when provisioning the service, provided as a JSON object. Cannot be combined with --param
--plan string The plan name (Required)
-s, --secret strings Additional parameter, whose value is stored in a secret, to use when provisioning the service, format: SECRET[KEY]
--timeout string Timeout for --wait, specified in human readable format: 30s, 1m, 1h. Specify -1 to wait indefinitely. (default "5m")
--wait Wait until the operation completes.

Global Flags:
--context string name of the kubeconfig context to use.
--kubeconfig string path to kubeconfig file. Overrides $KUBECONFIG
--logtostderr log to standard error instead of files (default false)
-v, --v Level log level for V logs

Error: required flag(s) "class", "plan" not set

[jichenjc]

saw this in previous comments just now..
https://travis-ci.org/kubernetes-incubator/service-catalog/jobs/403481082#L582
maybe golden files need to be changed as well? need watch new CI result and see whether need take further action...

[carolynvs]

Oh that's much easier! Let me look at the failure and figure out why the build isn't green...

[carolynvs]

Here's the failing tests TestCommandValidation

Looks like the way that we are checking for the error message isn't working anymore in that test. Can you take a look and see if we can change the test to figure out why?

Let me know if you get stuck on that and I can help figure it out tomorrow.

--- FAIL: TestCommandValidation (0.14s)
    --- FAIL: TestCommandValidation/describe_broker_requires_name (0.01s)
    	svcat_test.go:502: unexpected error 
    		
    		WANT:
    		"a broker name is required"
    		
    		GOT:
    		""

[jichenjc]

sorry still be familiar with how github works,
just pushed to issue/2166 and comments there, copied from there:

made a minor change, and UT now succeed , hopefully better now ...

ok github.com/kubernetes-incubator/service-catalog/cmd/svcat 7.238s

[jberkhahn]

/lgtm

This seems pretty reasonable now.

[jichenjc]

/retest

[jichenjc]

thanks for info , I just triggered the retest

[k8s-ci-robot]

@jichenjc: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jichenjc]

/test pull-service-catalog-xbuild

[k8s-ci-robot]

@jichenjc: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-service-catalog-xbuild

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[MHBauer]

/ok-to-test
/test pull-service-catalog-xbuild

[carolynvs]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carolynvs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cmd/svcat/OWNERS [carolynvs]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment