Make wait, no-wait and async flags per bool var CLI convention (#802)

* Make wait, no-wait and async flags per bool var CLI convention

 Fixes #800

 - Deprecated bool vars can be supported for CLI convention
 - Bind --async flag value to --no-wait
 - Only one flag among [wait, no-wait, async] can be provided, else raise an error

* Simplify conditionals

* Add unit tests for deprecated flag async

* Fix a typo
This commit is contained in:
Navid Shaikh 2020-04-15 01:06:16 +05:30 committed by GitHub
parent 2225b4c8dd
commit 46ecdcc32f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 127 additions and 51 deletions

View File

@ -21,10 +21,11 @@ kn revision delete NAME [flags]
### Options ### Options
``` ```
--async DEPRECATED: please use --no-wait instead. Delete revision and don't wait for it to be deleted. --async DEPRECATED: please use --no-wait instead. Do not wait for 'revision delete' operation to be completed. (default true)
-h, --help help for delete -h, --help help for delete
-n, --namespace string Specify the namespace to operate in. -n, --namespace string Specify the namespace to operate in.
--no-wait Delete revision and don't wait for it to be deleted. (default true) --no-wait Do not wait for 'revision delete' operation to be completed. (default true)
--wait Wait for 'revision delete' operation to be completed.
--wait-timeout int Seconds to wait before giving up on waiting for revision to be deleted. (default 600) --wait-timeout int Seconds to wait before giving up on waiting for revision to be deleted. (default 600)
``` ```

View File

@ -47,7 +47,7 @@ kn service create NAME --image IMAGE [flags]
``` ```
-a, --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). -a, --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
--arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
--async DEPRECATED: please use --no-wait instead. Create service and don't wait for it to be ready. --async DEPRECATED: please use --no-wait instead. Do not wait for 'service create' operation to be completed.
--autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
--cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available) --cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available)
--cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.
@ -71,7 +71,7 @@ kn service create NAME --image IMAGE [flags]
-n, --namespace string Specify the namespace to operate in. -n, --namespace string Specify the namespace to operate in.
--no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true) --no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true)
--no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
--no-wait Create service and don't wait for it to be ready. --no-wait Do not wait for 'service create' operation to be completed.
-p, --port int32 The port where application listens on. -p, --port int32 The port where application listens on.
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
--requests-cpu string The requested CPU (e.g., 250m). --requests-cpu string The requested CPU (e.g., 250m).
@ -80,6 +80,7 @@ kn service create NAME --image IMAGE [flags]
--service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
--user int The user ID to run the container (e.g., 1001). --user int The user ID to run the container (e.g., 1001).
--volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
--wait Wait for 'service create' operation to be completed. (default true)
--wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600)
``` ```

View File

@ -24,10 +24,11 @@ kn service delete NAME [flags]
### Options ### Options
``` ```
--async DEPRECATED: please use --no-wait instead. Delete service and don't wait for it to be deleted. --async DEPRECATED: please use --no-wait instead. Do not wait for 'service delete' operation to be completed. (default true)
-h, --help help for delete -h, --help help for delete
-n, --namespace string Specify the namespace to operate in. -n, --namespace string Specify the namespace to operate in.
--no-wait Delete service and don't wait for it to be deleted. (default true) --no-wait Do not wait for 'service delete' operation to be completed. (default true)
--wait Wait for 'service delete' operation to be completed.
--wait-timeout int Seconds to wait before giving up on waiting for service to be deleted. (default 600) --wait-timeout int Seconds to wait before giving up on waiting for service to be deleted. (default 600)
``` ```

View File

@ -40,7 +40,7 @@ kn service update NAME [flags]
``` ```
-a, --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). -a, --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
--arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
--async DEPRECATED: please use --no-wait instead. Update service and don't wait for it to be ready. --async DEPRECATED: please use --no-wait instead. Do not wait for 'service update' operation to be completed.
--autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
--cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available) --cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available)
--cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.
@ -63,7 +63,7 @@ kn service update NAME [flags]
-n, --namespace string Specify the namespace to operate in. -n, --namespace string Specify the namespace to operate in.
--no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true) --no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available) (default true)
--no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
--no-wait Update service and don't wait for it to be ready. --no-wait Do not wait for 'service update' operation to be completed.
-p, --port int32 The port where application listens on. -p, --port int32 The port where application listens on.
--pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
--requests-cpu string The requested CPU (e.g., 250m). --requests-cpu string The requested CPU (e.g., 250m).
@ -75,6 +75,7 @@ kn service update NAME [flags]
--untag strings Untag revision (format: --untag tagName). This flag can be specified multiple times. --untag strings Untag revision (format: --untag tagName). This flag can be specified multiple times.
--user int The user ID to run the container (e.g., 1001). --user int The user ID to run the container (e.g., 1001).
--volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
--wait Wait for 'service update' operation to be completed. (default true)
--wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600)
``` ```

View File

@ -49,7 +49,7 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
for _, name := range args { for _, name := range args {
timeout := time.Duration(0) timeout := time.Duration(0)
if !waitFlags.NoWait { if waitFlags.Wait {
timeout = time.Duration(waitFlags.TimeoutInSeconds) * time.Second timeout = time.Duration(waitFlags.TimeoutInSeconds) * time.Second
} }
err = client.DeleteRevision(name, timeout) err = client.DeleteRevision(name, timeout)
@ -63,6 +63,6 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
}, },
} }
commands.AddNamespaceFlags(RevisionDeleteCommand.Flags(), false) commands.AddNamespaceFlags(RevisionDeleteCommand.Flags(), false)
waitFlags.AddConditionWaitFlags(RevisionDeleteCommand, commands.WaitDefaultTimeout, "Delete", "revision", "deleted") waitFlags.AddConditionWaitFlags(RevisionDeleteCommand, commands.WaitDefaultTimeout, "delete", "revision", "deleted")
return RevisionDeleteCommand return RevisionDeleteCommand
} }

View File

@ -117,7 +117,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
} }
commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false) commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false)
editFlags.AddCreateFlags(serviceCreateCommand) editFlags.AddCreateFlags(serviceCreateCommand)
waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "Create", "service", "ready") waitFlags.AddConditionWaitFlags(serviceCreateCommand, commands.WaitDefaultTimeout, "create", "service", "ready")
return serviceCreateCommand return serviceCreateCommand
} }
@ -145,7 +145,7 @@ func waitIfRequested(client clientservingv1.KnServingClient, service *servingv1.
fmt.Fprintf(out, "Service '%s' %s in namespace '%s'.\n", service.Name, verbDone, client.Namespace()) fmt.Fprintf(out, "Service '%s' %s in namespace '%s'.\n", service.Name, verbDone, client.Namespace())
return nil return nil
} }
if waitFlags.NoWait { if !waitFlags.Wait {
fmt.Fprintf(out, "Service '%s' %s in namespace '%s'.\n", service.Name, verbDone, client.Namespace()) fmt.Fprintf(out, "Service '%s' %s in namespace '%s'.\n", service.Name, verbDone, client.Namespace())
return nil return nil
} }

View File

@ -53,7 +53,7 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
} }
for _, name := range args { for _, name := range args {
timeout := time.Duration(0) timeout := time.Duration(0)
if !waitFlags.NoWait { if waitFlags.Wait {
timeout = time.Duration(waitFlags.TimeoutInSeconds) * time.Second timeout = time.Duration(waitFlags.TimeoutInSeconds) * time.Second
} }
err = client.DeleteService(name, timeout) err = client.DeleteService(name, timeout)
@ -67,6 +67,6 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
}, },
} }
commands.AddNamespaceFlags(serviceDeleteCommand.Flags(), false) commands.AddNamespaceFlags(serviceDeleteCommand.Flags(), false)
waitFlags.AddConditionWaitFlags(serviceDeleteCommand, commands.WaitDefaultTimeout, "Delete", "service", "deleted") waitFlags.AddConditionWaitFlags(serviceDeleteCommand, commands.WaitDefaultTimeout, "delete", "service", "deleted")
return serviceDeleteCommand return serviceDeleteCommand
} }

View File

@ -17,9 +17,11 @@ package service
import ( import (
"bytes" "bytes"
"github.com/spf13/cobra"
"k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd"
"knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands"
knflags "knative.dev/client/pkg/kn/flags"
clientservingv1 "knative.dev/client/pkg/serving/v1" clientservingv1 "knative.dev/client/pkg/serving/v1"
) )
@ -60,6 +62,10 @@ func executeServiceCommand(client clientservingv1.KnServingClient, args ...strin
cmd := NewServiceCommand(knParams) cmd := NewServiceCommand(knParams)
cmd.SetArgs(args) cmd.SetArgs(args)
cmd.SetOutput(output) cmd.SetOutput(output)
cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
return knflags.ReconcileBoolFlags(cmd.Flags())
}
err := cmd.Execute() err := cmd.Execute()
return output.String(), err return output.String(), err
} }

View File

@ -110,7 +110,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
out := cmd.OutOrStdout() out := cmd.OutOrStdout()
//TODO: deprecated condition should be once --async is gone //TODO: deprecated condition should be once --async is gone
if !waitFlags.Async && !waitFlags.NoWait { if !waitFlags.Async && waitFlags.Wait {
fmt.Fprintf(out, "Updating Service '%s' in namespace '%s':\n", args[0], namespace) fmt.Fprintf(out, "Updating Service '%s' in namespace '%s':\n", args[0], namespace)
fmt.Fprintln(out, "") fmt.Fprintln(out, "")
err := waitForService(client, name, out, waitFlags.TimeoutInSeconds) err := waitForService(client, name, out, waitFlags.TimeoutInSeconds)
@ -136,7 +136,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
commands.AddNamespaceFlags(serviceUpdateCommand.Flags(), false) commands.AddNamespaceFlags(serviceUpdateCommand.Flags(), false)
editFlags.AddUpdateFlags(serviceUpdateCommand) editFlags.AddUpdateFlags(serviceUpdateCommand)
waitFlags.AddConditionWaitFlags(serviceUpdateCommand, commands.WaitDefaultTimeout, "Update", "service", "ready") waitFlags.AddConditionWaitFlags(serviceUpdateCommand, commands.WaitDefaultTimeout, "update", "service", "ready")
trafficFlags.Add(serviceUpdateCommand) trafficFlags.Add(serviceUpdateCommand)
return serviceUpdateCommand return serviceUpdateCommand
} }

View File

@ -18,6 +18,8 @@ import (
"fmt" "fmt"
"github.com/spf13/cobra" "github.com/spf13/cobra"
knflags "knative.dev/client/pkg/kn/flags"
) )
// Default time out to use when waiting for reconciliation. It is deliberately very long as it is expected that // Default time out to use when waiting for reconciliation. It is deliberately very long as it is expected that
@ -29,9 +31,8 @@ const WaitDefaultTimeout = 600
type WaitFlags struct { type WaitFlags struct {
// Timeout in seconds for how long to wait for a command to return // Timeout in seconds for how long to wait for a command to return
TimeoutInSeconds int TimeoutInSeconds int
// If set then apply resources and wait for completion
// If set then just apply resources and don't wait Wait bool
NoWait bool
//TODO: deprecated variable should be removed with --async flag //TODO: deprecated variable should be removed with --async flag
Async bool Async bool
} }
@ -40,18 +41,16 @@ type WaitFlags struct {
// resources. Set `waitDefault` argument if the default behaviour is synchronous. // resources. Set `waitDefault` argument if the default behaviour is synchronous.
// Use `what` for describing what is waited for. // Use `what` for describing what is waited for.
func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action, what, until string) { func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action, what, until string) {
waitUsage := fmt.Sprintf("%s %s and don't wait for it to be %s.", action, what, until) waitUsage := fmt.Sprintf("Wait for '%s %s' operation to be completed.", what, action)
//TODO: deprecated flag should be removed in next release waitDefault := true
// Special-case 'delete' command so it comes back to the user immediately // Special-case 'delete' command so it comes back to the user immediately
noWaitDefault := false if action == "delete" {
if action == "Delete" { waitDefault = false
noWaitDefault = true
} }
command.Flags().BoolVar(&p.Async, "async", false, "DEPRECATED: please use --no-wait instead. "+waitUsage) //TODO: deprecated flag should be removed in next release
command.Flags().BoolVar(&p.NoWait, "no-wait", noWaitDefault, waitUsage) command.Flags().BoolVar(&p.Async, "async", !waitDefault, "DEPRECATED: please use --no-wait instead. "+knflags.InvertUsage(waitUsage))
knflags.AddBothBoolFlagsUnhidden(command.Flags(), &p.Wait, "wait", "", waitDefault, waitUsage)
timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be %s.", what, until) timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be %s.", what, until)
command.Flags().IntVar(&p.TimeoutInSeconds, "wait-timeout", waitTimeoutDefault, timeoutUsage) command.Flags().IntVar(&p.TimeoutInSeconds, "wait-timeout", waitTimeoutDefault, timeoutUsage)
} }

View File

@ -15,34 +15,37 @@
package commands package commands
import ( import (
"fmt"
"strings" "strings"
"testing" "testing"
knflags "knative.dev/client/pkg/kn/flags"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"gotest.tools/assert"
) )
type waitTestCase struct { type waitTestCase struct {
args []string args []string
timeoutExpected int timeoutExpected int
isNoWaitExpected bool isWaitExpected bool
isParseErrorExpected bool isParseErrorExpected bool
} }
//TODO: deprecated test should be removed with --async flag //TODO: deprecated test should be removed with --async flag
func TestAddWaitForReadyDeprecatedFlags(t *testing.T) { func TestAddWaitForReadyDeprecatedFlags(t *testing.T) {
for i, tc := range []waitTestCase{ for i, tc := range []waitTestCase{
{[]string{"--async"}, 60, true, false}, {[]string{"--async"}, 60, false, false},
{[]string{}, 60, false, false}, {[]string{}, 60, true, false},
{[]string{"--wait-timeout=120"}, 120, false, false}, {[]string{"--wait-timeout=120"}, 120, true, false},
// Can't be easily prevented, the timeout is just ignored in this case: // Can't be easily prevented, the timeout is just ignored in this case:
{[]string{"--async", "--wait-timeout=120"}, 120, true, false}, {[]string{"--async", "--wait-timeout=120"}, 120, false, false},
{[]string{"--wait-timeout=bla"}, 0, true, true}, {[]string{"--wait-timeout=bla"}, 0, true, true},
} { } {
flags := &WaitFlags{} flags := &WaitFlags{}
cmd := cobra.Command{} cmd := cobra.Command{}
flags.AddConditionWaitFlags(&cmd, 60, "Create", "service", "ready") flags.AddConditionWaitFlags(&cmd, 60, "create", "service", "ready")
err := cmd.ParseFlags(tc.args) err := cmd.ParseFlags(tc.args)
if err != nil && !tc.isParseErrorExpected { if err != nil && !tc.isParseErrorExpected {
@ -54,8 +57,13 @@ func TestAddWaitForReadyDeprecatedFlags(t *testing.T) {
if tc.isParseErrorExpected { if tc.isParseErrorExpected {
continue continue
} }
if flags.Async != tc.isNoWaitExpected {
t.Errorf("%d: wrong async mode detected: %t (expected) != %t (actual)", i, tc.isNoWaitExpected, flags.Async) // reconcile to ensure wait, no-wait and async behaves as expected
err = knflags.ReconcileBoolFlags(cmd.Flags())
assert.NilError(t, err)
if flags.Async == tc.isWaitExpected {
t.Errorf("%d: wrong async mode detected: %t (expected) != %t (actual)", i, tc.isWaitExpected, flags.Async)
} }
if flags.TimeoutInSeconds != tc.timeoutExpected { if flags.TimeoutInSeconds != tc.timeoutExpected {
t.Errorf("%d: Invalid timeout set. %d (expected) != %d (actual)", i, tc.timeoutExpected, flags.TimeoutInSeconds) t.Errorf("%d: Invalid timeout set. %d (expected) != %d (actual)", i, tc.timeoutExpected, flags.TimeoutInSeconds)
@ -64,19 +72,17 @@ func TestAddWaitForReadyDeprecatedFlags(t *testing.T) {
} }
func TestAddWaitForReadyFlags(t *testing.T) { func TestAddWaitForReadyFlags(t *testing.T) {
for i, tc := range []waitTestCase{ for i, tc := range []waitTestCase{
{[]string{"--no-wait"}, 60, true, false}, {[]string{}, 60, true, false},
{[]string{}, 60, false, false}, {[]string{"--wait-timeout=120"}, 120, true, false},
{[]string{"--wait-timeout=120"}, 120, false, false},
// Can't be easily prevented, the timeout is just ignored in this case: // Can't be easily prevented, the timeout is just ignored in this case:
{[]string{"--no-wait", "--wait-timeout=120"}, 120, true, false}, {[]string{"--no-wait", "--wait-timeout=120"}, 120, false, false},
{[]string{"--wait-timeout=bla"}, 0, true, true}, {[]string{"--wait-timeout=bla"}, 0, true, true},
} { } {
flags := &WaitFlags{} flags := &WaitFlags{}
cmd := cobra.Command{} cmd := cobra.Command{}
flags.AddConditionWaitFlags(&cmd, 60, "Create", "service", "ready") flags.AddConditionWaitFlags(&cmd, 60, "create", "service", "ready")
err := cmd.ParseFlags(tc.args) err := cmd.ParseFlags(tc.args)
if err != nil && !tc.isParseErrorExpected { if err != nil && !tc.isParseErrorExpected {
@ -88,8 +94,14 @@ func TestAddWaitForReadyFlags(t *testing.T) {
if tc.isParseErrorExpected { if tc.isParseErrorExpected {
continue continue
} }
if flags.NoWait != tc.isNoWaitExpected {
t.Errorf("%d: wrong wait mode detected: %t (expected) != %t (actual)", i, tc.isNoWaitExpected, flags.NoWait) // reconcile to ensure wait, no-wait and async behaves as expected
err = knflags.ReconcileBoolFlags(cmd.Flags())
assert.NilError(t, err)
fmt.Println("wait value")
fmt.Println(flags.Wait)
if flags.Wait != tc.isWaitExpected {
t.Errorf("%d: wrong wait mode detected: %t (expected) != %t (actual)", i, tc.isWaitExpected, flags.Wait)
} }
if flags.TimeoutInSeconds != tc.timeoutExpected { if flags.TimeoutInSeconds != tc.timeoutExpected {
t.Errorf("%d: Invalid timeout set. %d (expected) != %d (actual)", i, tc.timeoutExpected, flags.TimeoutInSeconds) t.Errorf("%d: Invalid timeout set. %d (expected) != %d (actual)", i, tc.timeoutExpected, flags.TimeoutInSeconds)
@ -105,7 +117,7 @@ func TestAddWaitUsageMessage(t *testing.T) {
if !strings.Contains(cmd.UsageString(), "blub") { if !strings.Contains(cmd.UsageString(), "blub") {
t.Error("no type returned in usage") t.Error("no type returned in usage")
} }
if !strings.Contains(cmd.UsageString(), "don't wait") { if !strings.Contains(cmd.UsageString(), "Do not wait") {
t.Error("wrong usage message") t.Error("wrong usage message")
} }
if !strings.Contains(cmd.UsageString(), "60") { if !strings.Contains(cmd.UsageString(), "60") {
@ -119,8 +131,8 @@ func TestAddWaitUsageMessage(t *testing.T) {
func TestAddWaitUsageDelete(t *testing.T) { func TestAddWaitUsageDelete(t *testing.T) {
flags := &WaitFlags{} flags := &WaitFlags{}
cmd := cobra.Command{} cmd := cobra.Command{}
flags.AddConditionWaitFlags(&cmd, 60, "Delete", "blub", "deleted") flags.AddConditionWaitFlags(&cmd, 60, "delete", "blub", "deleted")
if !strings.Contains(cmd.UsageString(), "deleted. (default true)") { if !strings.Contains(cmd.UsageString(), "completed. (default true)") {
t.Error("Delete has wrong default value for --no-wait") t.Error("Delete has wrong default value for --no-wait")
} }
} }

View File

@ -24,7 +24,10 @@ import (
"github.com/spf13/pflag" "github.com/spf13/pflag"
) )
var negPrefix = "no-" var (
negPrefix = "no-"
deprecatedPrefix = "DEPRECATED:"
)
// AddBothBoolFlagsUnhidden is just like AddBothBoolFlags but shows both flags. // AddBothBoolFlagsUnhidden is just like AddBothBoolFlags but shows both flags.
func AddBothBoolFlagsUnhidden(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) { func AddBothBoolFlagsUnhidden(f *pflag.FlagSet, p *bool, name, short string, value bool, usage string) {
@ -32,7 +35,7 @@ func AddBothBoolFlagsUnhidden(f *pflag.FlagSet, p *bool, name, short string, val
negativeName := negPrefix + name negativeName := negPrefix + name
f.BoolVarP(p, name, short, value, usage) f.BoolVarP(p, name, short, value, usage)
f.Bool(negativeName, !value, "Do not "+firstCharToLower(usage)) f.Bool(negativeName, !value, InvertUsage(usage))
} }
// AddBothBoolFlags adds the given flag in both `--foo` and `--no-foo` variants. // AddBothBoolFlags adds the given flag in both `--foo` and `--no-foo` variants.
@ -65,6 +68,20 @@ func ReconcileBoolFlags(f *pflag.FlagSet) error {
if err != nil { if err != nil {
return return
} }
// handle async flag
if flag.Name == "async" && flag.Changed {
if f.Lookup("wait").Changed || f.Lookup("no-wait").Changed {
err = fmt.Errorf("only one of (DEPRECATED) --async, --wait and --no-wait may be specified")
return
}
err = checkExplicitFalse(flag, "wait")
if err != nil {
return
}
f.Lookup("no-wait").Value.Set("true")
}
// Walk the "no-" versions of the flags. Make sure we didn't set // Walk the "no-" versions of the flags. Make sure we didn't set
// both, and set the positive value to the opposite of the "no-" // both, and set the positive value to the opposite of the "no-"
// value if it exists. // value if it exists.
@ -90,6 +107,7 @@ func ReconcileBoolFlags(f *pflag.FlagSet) error {
err = checkExplicitFalse(positive, flag.Name) err = checkExplicitFalse(positive, flag.Name)
} }
} }
}) })
return err return err
} }
@ -110,7 +128,13 @@ func checkExplicitFalse(f *pflag.Flag, betterFlag string) error {
return nil return nil
} }
func firstCharToLower(s string) string { // FirstCharToLower converts first char in given string to lowercase
func FirstCharToLower(s string) string {
r, n := utf8.DecodeRuneInString(s) r, n := utf8.DecodeRuneInString(s)
return string(unicode.ToLower(r)) + s[n:] return string(unicode.ToLower(r)) + s[n:]
} }
// InvertUsage inverts the usage string with prefix "Do not"
func InvertUsage(usage string) string {
return "Do not " + FirstCharToLower(usage)
}

View File

@ -29,6 +29,13 @@ type boolPairTestCase struct {
expectedErrText string expectedErrText string
} }
type boolPairTestCaseDeprecated struct {
waitDefaultVal bool
flags []string
expectedResult bool
expectedErrText string
}
func TestBooleanPair(t *testing.T) { func TestBooleanPair(t *testing.T) {
cases := []*boolPairTestCase{ cases := []*boolPairTestCase{
{"foo", true, []string{}, true, ""}, {"foo", true, []string{}, true, ""},
@ -69,3 +76,27 @@ func TestBooleanPair(t *testing.T) {
} }
} }
} }
func TestBooleanPairWithDeprecatedSyncFlag(t *testing.T) {
cases := []*boolPairTestCaseDeprecated{
{true, []string{}, false, ""},
{true, []string{"--async"}, true, ""},
{true, []string{"--async", "--no-wait"}, false, "only one of (DEPRECATED) --async, --wait and --no-wait may be specified"},
// delete operation
{false, []string{""}, true, ""},
{false, []string{"--async=false"}, false, "use --wait instead of providing \"false\" to --async"},
}
for _, c := range cases {
var result, wait bool
f := &pflag.FlagSet{}
AddBothBoolFlags(f, &wait, "wait", "", c.waitDefaultVal, "set wait")
f.BoolVar(&result, "async", !c.waitDefaultVal, "DEPRECATED: set async")
f.Parse(c.flags)
err := ReconcileBoolFlags(f)
if c.expectedErrText != "" {
assert.ErrorContains(t, err, c.expectedErrText)
} else {
assert.Equal(t, result, c.expectedResult)
}
}
}