solo-io / wasm

Web Assembly tools and SDKs for extending cloud-native infrastructure
Apache License 2.0
305 stars 39 forks source link

`wasme deploy istio` usage message is wrong for multiple arguments #260

Open jameshbarton opened 3 years ago

jameshbarton commented 3 years ago

Describe the bug The usage message for wasme deploy istio is wrong for both the --namespaces and --name options.

To Reproduce

  1. Run this command and observe that --namespaces is identified as an unknown flag even though it is listed as an optional argument [--namespaces <comma separated namespaced] in the usage message.
    
    % wasme deploy istio webassemblyhub.io/jameshbarton/remove-headers:v1.0 --id=remove-headers --config "x-envoy" --namespaces istio-system --name istio-ingressgateway
    Error: unknown flag: --namespaces
    Usage:
    wasme deploy istio <image> --id=<unique name> [--config=<inline string>] [--root-id=<root id>] [--namespaces <comma separated namespaces>] [--name deployment-name] [--patch-context={any|inbound|outbound|gateway}] [flags]

Flags: --cache-custom-command strings custom command to provide to the cache server image --cache-image-pull-policy string image pull policy for the cache server daemonset. see https://kubernetes.io/docs/concepts/containers/images/ (default "IfNotPresent") --cache-name string name of resources for the wasm image cache server (default "wasme-cache") --cache-namespace string namespace of resources for the wasm image cache server (default "wasme") --cache-repo string name of the image repository to use for the cache server daemonset (default "quay.io/solo-io/wasme") --cache-tag string image tag to use for the cache server daemonset (default "0.0.33") --cache-timeout duration the length of time to wait for the server-side filter cache to pull the filter image before giving up with an error. set to 0 to skip the check entirely (note, this may produce a known race condition). (default 1m0s) -h, --help help for istio --ignore-version-check set to disable abi version compatability check. --istio-namespace string the namespace where the Istio control plane is installed (default "istio-system") --istiod-name string deployment name of the istiod (default "istiod") -l, --labels stringToString labels of the deployment or daemonset into which to inject the filter. if not set, will apply to all workloads in the target namespace (default []) -n, --namespace string namespace of the workload(s) to inject the filter. (default "default") --patch-context string patch context of the filter. possible values are any, inbound, outbound, gateway (default "inbound") -t, --workload-type string type of workload into which the filter should be injected. possible values are daemonset, deployment, statefulset (default "deployment")

Global Flags: --config string optional config that will be passed to the filter. accepts an inline string. --id string unique id for naming the deployed filter. this is used for logging as well as removing the filter. when running wasme deploy istio, this name must be a valid Kubernetes resource name. --root-id string optional root ID used to bind the filter at the Envoy level. this value is normally read from the filter image directly. -v, --verbose verbose output


2. Change `--namespaces` to `--namespace` and rerun.  Note that `--name` is identified as an unknown flag even though it is listed as an optional argument `[--name deployment-name]` in the usage message.

% wasme deploy istio webassemblyhub.io/jameshbarton/remove-headers:v1.0 --id=remove-headers --config "x-envoy" --namespace istio-system --name istio-ingressgateway Error: unknown flag: --name Usage: wasme deploy istio --id= [--config=] [--root-id=] [--namespaces ] [--name deployment-name] [--patch-context={any|inbound|outbound|gateway}] [flags] ...


**Expected behavior**
Usage message should accurately reflect CLI arguments.

**Additional context**

% wasme --version wasme version 0.0.33

swartz-k commented 3 years ago

Hey, I cannot reproduce this problem. --name is not supported,only use --namespace didnt get error image

jameshbarton commented 3 years ago

@swartz-k - I was able to reproduce just now. This issue isn't about whether we can get wasme deploy istio to work; it's about correcting multiple confusing errors in the usage message.

In the first example above, I used the option --namespaces. The usage message says that that is an unknown flag, but the following line says that it's an optional argument [--namespaces <comma separated namespaces>]: wasme deploy istio <image> --id=<unique name> [--config=<inline string>] [--root-id=<root id>] [--namespaces <comma separated namespaces>] [--name deployment-name] [--patch-context={any|inbound|outbound|gateway}] [flags]

In the second example, I used the option --name. Your comment says that --name is not supported, but the usage message clearly says that it is supported as an optional argument: wasme deploy istio <image> --id=<unique name> [--config=<inline string>] [--root-id=<root id>] [--namespaces <comma separated namespaces>] [--name deployment-name] [--patch-context={any|inbound|outbound|gateway}] [flags]

The usage message should accurately reflect the acceptable CLI arguments.

swartz-k commented 3 years ago

@jameshbarton Thanks. I got that and found code about this logic. I will make pr to fix problems like this.