From afcde2d5512010d9083b6378a24eec6b5bbe46a5 Mon Sep 17 00:00:00 2001 From: Luke Kingland <58986931+lkingland@users.noreply.github.com> Date: Sat, 10 Jul 2021 00:15:23 +0900 Subject: [PATCH] src: testable commands (#415) * feat: client progress listener 'stopping' state * src: testable commands Restructures commands to accept a fn.Client constructor on command instantiation. This allows the concrete implementations, or entire client to be mocked for testing. Also some minor refacotring as necessary to shoehorn into the pattern. * fix: increase default timeout to 120s for service creation * chore: bump kind, knative and kubectl versions --- .github/workflows/pull_requests.yaml | 10 +- client.go | 53 +++++++- cmd/build.go | 89 ++++++------ cmd/create.go | 22 ++- cmd/create_test.go | 2 +- cmd/delete.go | 138 ++++++++++--------- cmd/delete_test.go | 143 ++++++++++---------- cmd/deploy.go | 123 +++++++++-------- cmd/describe.go | 72 ++++++---- cmd/emit.go | 193 +++++++++++++++++---------- cmd/list.go | 115 ++++++++++------ cmd/run.go | 63 +++++---- knative/client.go | 2 +- progress/progress.go | 7 + 14 files changed, 622 insertions(+), 410 deletions(-) diff --git a/.github/workflows/pull_requests.yaml b/.github/workflows/pull_requests.yaml index 471b8743e..24de403b5 100644 --- a/.github/workflows/pull_requests.yaml +++ b/.github/workflows/pull_requests.yaml @@ -80,11 +80,11 @@ jobs: - name: Provision Cluster uses: container-tools/kind-action@v1 # use ./hack/allocate.sh locally with: - version: v0.10.0 - kubectl_version: v1.20.0 - knative_serving: v0.22.0 - knative_kourier: v0.22.0 - knative_eventing: v0.22.0 + version: v0.11.1 + kubectl_version: v1.21.2 + knative_serving: v0.23.0 + knative_kourier: v0.23.0 + knative_eventing: v0.23.0 config: testdata/cluster.yaml - name: Configure Cluster run: ./hack/configure.sh diff --git a/client.go b/client.go index 0391cadfd..e6f3a38b2 100644 --- a/client.go +++ b/client.go @@ -112,6 +112,10 @@ type ProgressListener interface { // Complete signals completion, which is expected to be somewhat different than a step increment. Complete(message string) + // Stopping indicates the process is in the state of stopping, such as when a context cancelation + // has been received + Stopping() + // Done signals a cessation of progress updates. Should be called in a defer statement to ensure // the progress listener can stop any outstanding tasks such as synchronous user updates. Done() @@ -278,7 +282,15 @@ func WithEmitter(e Emitter) Option { // Use Create, Build and Deploy independently for lower level control. func (c *Client) New(ctx context.Context, cfg Function) (err error) { c.progressListener.SetTotal(3) - defer c.progressListener.Done() + // Always start a concurrent routine listening for context cancellation. + // On this event, immediately indicate the task is canceling. + // (this is useful, for example, when a progress listener is mutating + // stdout, and a context cancelation needs to free up stdout entirely for + // the status or error from said cancelltion. + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() // Create local template err = c.Create(cfg) @@ -404,6 +416,10 @@ func (c *Client) Create(cfg Function) (err error) { // not contain a populated Image. func (c *Client) Build(ctx context.Context, path string) (err error) { c.progressListener.Increment("Building function image") + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() f, err := NewFunction(path) if err != nil { @@ -436,6 +452,12 @@ func (c *Client) Build(ctx context.Context, path string) (err error) { // Deploy the Function at path. Errors if the Function has not been // initialized with an image tag. func (c *Client) Deploy(ctx context.Context, path string) (err error) { + c.progressListener.Increment("Deployin function") + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() + f, err := NewFunction(path) if err != nil { return @@ -489,6 +511,10 @@ func (c *Client) Route(path string) (err error) { // Run the Function whose code resides at root. func (c *Client) Run(ctx context.Context, root string) error { + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() // Create an instance of a Function representation at the given root. f, err := NewFunction(root) @@ -514,6 +540,10 @@ func (c *Client) List(ctx context.Context) ([]ListItem, error) { // Describe a Function. Name takes precidence. If no name is provided, // the Function defined at root is used. func (c *Client) Describe(ctx context.Context, name, root string) (d Description, err error) { + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() // If name is provided, it takes precidence. // Otherwise load the Function defined at root. if name != "" { @@ -533,6 +563,10 @@ func (c *Client) Describe(ctx context.Context, name, root string) (d Description // Remove a Function. Name takes precidence. If no name is provided, // the Function defined at root is used if it exists. func (c *Client) Remove(ctx context.Context, cfg Function) error { + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() // If name is provided, it takes precidence. // Otherwise load the Function deined at root. if cfg.Name != "" { @@ -551,15 +585,23 @@ func (c *Client) Remove(ctx context.Context, cfg Function) error { // Emit a CloudEvent to a function endpoint func (c *Client) Emit(ctx context.Context, endpoint string) error { + go func() { + <-ctx.Done() + c.progressListener.Stopping() + }() return c.emitter.Emit(ctx, endpoint) } // Manual implementations (noops) of required interfaces. // In practice, the user of this client package (for example the CLI) will -// provide a concrete implementation for all of the interfaces. For testing or -// development, however, it is usefule that they are defaulted to noops and -// provded only when necessary. Unit tests for the concrete implementations -// serve to keep the core logic here separate from the imperitive. +// provide a concrete implementation for only the interfaces necessary to +// complete the given command. Integrators importing the package would +// provide a concrete implementation for all interfaces to be used. To +// enable partial definition (in particular used for testing) they +// are defaulted to noop implementations such that they can be provded +// only when necessary. Unit tests for the concrete implementations +// serve to keep the core logic here separate from the imperitive, and +// with a minimum of external dependencies. // ----------------------------------------------------- type noopBuilder struct{ output io.Writer } @@ -597,6 +639,7 @@ type noopProgressListener struct{} func (p *noopProgressListener) SetTotal(i int) {} func (p *noopProgressListener) Increment(m string) {} func (p *noopProgressListener) Complete(m string) {} +func (p *noopProgressListener) Stopping() {} func (p *noopProgressListener) Done() {} type noopEmitter struct{} diff --git a/cmd/build.go b/cmd/build.go index a295a1046..a229044de 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -14,23 +14,32 @@ import ( ) func init() { - root.AddCommand(buildCmd) - buildCmd.Flags().StringP("builder", "b", "", "Buildpack builder, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml for subsequent builds.") - buildCmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") - buildCmd.Flags().StringP("image", "i", "", "Full image name in the orm [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE") - buildCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") - buildCmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)") - - err := buildCmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList) - if err != nil { - fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) - } + // Add to the root a new "Build" command which obtains an appropriate + // instance of fn.Client from the given client creator function. + root.AddCommand(NewBuildCmd(newBuildClient)) } -var buildCmd = &cobra.Command{ - Use: "build", - Short: "Build a function project as a container image", - Long: `Build a function project as a container image +func newBuildClient(cfg buildConfig) (*fn.Client, error) { + builder := buildpacks.NewBuilder() + listener := progress.New() + + builder.Verbose = cfg.Verbose + listener.Verbose = cfg.Verbose + + return fn.New( + fn.WithBuilder(builder), + fn.WithProgressListener(listener), + fn.WithRegistry(cfg.Registry), // for image name when --image not provided + fn.WithVerbose(cfg.Verbose)), nil +} + +type buildClientFn func(buildConfig) (*fn.Client, error) + +func NewBuildCmd(clientFn buildClientFn) *cobra.Command { + cmd := &cobra.Command{ + Use: "build", + Short: "Build a function project as a container image", + Long: `Build a function project as a container image This command builds the function project in the current directory or in the directory specified by --path. The result will be a container image that is pushed to a registry. @@ -38,7 +47,7 @@ The func.yaml file is read to determine the image name and registry. If the project has not already been built, either --registry or --image must be provided and the image name is stored in the configuration file. `, - Example: ` + Example: ` # Build from the local directory, using the given registry as target. # The full image name will be determined automatically based on the # project directory name @@ -53,12 +62,28 @@ kn func build # Build with a custom buildpack builder kn func build --builder cnbs/sample-builder:bionic `, - SuggestFor: []string{"biuld", "buidl", "built"}, - PreRunE: bindEnv("image", "path", "builder", "registry", "confirm"), - RunE: runBuild, + SuggestFor: []string{"biuld", "buidl", "built"}, + PreRunE: bindEnv("image", "path", "builder", "registry", "confirm"), + } + + cmd.Flags().StringP("builder", "b", "", "Buildpack builder, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml for subsequent builds.") + cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + cmd.Flags().StringP("image", "i", "", "Full image name in the orm [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE") + cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") + cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)") + + if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList); err != nil { + fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) + } + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runBuild(cmd, args, clientFn) + } + + return cmd } -func runBuild(cmd *cobra.Command, _ []string) (err error) { +func runBuild(cmd *cobra.Command, _ []string, clientFn buildClientFn) (err error) { config, err := newBuildConfig().Prompt() if err != nil { if err == terminal.InterruptErr { @@ -106,26 +131,12 @@ func runBuild(cmd *cobra.Command, _ []string) (err error) { return } - builder := buildpacks.NewBuilder() - builder.Verbose = config.Verbose + client, err := clientFn(config) + if err != nil { + return err + } - listener := progress.New() - listener.Verbose = config.Verbose - defer listener.Done() - - context := cmd.Context() - go func() { - <-context.Done() - listener.Done() - }() - - client := fn.New( - fn.WithVerbose(config.Verbose), - fn.WithRegistry(config.Registry), // for deriving image name when --image not provided explicitly. - fn.WithBuilder(builder), - fn.WithProgressListener(listener)) - - return client.Build(context, config.Path) + return client.Build(cmd.Context(), config.Path) } type buildConfig struct { diff --git a/cmd/create.go b/cmd/create.go index ef37e9d5b..4acd55ff7 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -24,13 +24,15 @@ func init() { // The createClientFn is a client factory which creates a new Client for use by // the create command during normal execution (see tests for alternative client // factories which return clients with various mocks). -func newCreateClient(repositories string, verbose bool) *fn.Client { - return fn.New(fn.WithRepositories(repositories), fn.WithVerbose(verbose)) +func newCreateClient(cfg createConfig) *fn.Client { + return fn.New( + fn.WithRepositories(cfg.Repositories), + fn.WithVerbose(cfg.Verbose)) } // createClientFn is a factory function which returns a Client suitable for // use with the Create command. -type createClientFn func(repositories string, verbose bool) *fn.Client +type createClientFn func(createConfig) *fn.Client // NewCreateCmd creates a create command using the given client creator. func NewCreateCmd(clientFn createClientFn) *cobra.Command { @@ -59,14 +61,10 @@ kn func create --template events myfunc PreRunE: bindEnv("runtime", "template", "repositories", "confirm"), } - cmd.Flags().BoolP("confirm", "c", false, - "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") - cmd.Flags().StringP("runtime", "l", fn.DefaultRuntime, - "Function runtime language/framework. Available runtimes: "+buildpacks.Runtimes()+" (Env: $FUNC_RUNTIME)") - cmd.Flags().StringP("repositories", "r", filepath.Join(configPath(), "repositories"), - "Path to extended template repositories (Env: $FUNC_REPOSITORIES)") - cmd.Flags().StringP("template", "t", fn.DefaultTemplate, - "Function template. Available templates: 'http' and 'events' (Env: $FUNC_TEMPLATE)") + cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + cmd.Flags().StringP("runtime", "l", fn.DefaultRuntime, "Function runtime language/framework. Available runtimes: "+buildpacks.Runtimes()+" (Env: $FUNC_RUNTIME)") + cmd.Flags().StringP("repositories", "r", filepath.Join(configPath(), "repositories"), "Path to extended template repositories (Env: $FUNC_REPOSITORIES)") + cmd.Flags().StringP("template", "t", fn.DefaultTemplate, "Function template. Available templates: 'http' and 'events' (Env: $FUNC_TEMPLATE)") // Register tab-completeion function integration if err := cmd.RegisterFlagCompletionFunc("runtime", CompleteRuntimeList); err != nil { @@ -103,7 +101,7 @@ func runCreate(cmd *cobra.Command, args []string, clientFn createClientFn) (err Template: config.Template, } - client := clientFn(config.Repositories, config.Verbose) + client := clientFn(config) return client.Create(function) } diff --git a/cmd/create_test.go b/cmd/create_test.go index a5bd855e9..75330f788 100644 --- a/cmd/create_test.go +++ b/cmd/create_test.go @@ -17,7 +17,7 @@ func TestCreateValidatesName(t *testing.T) { // Create a new Create command with a fn.Client construtor // which returns a default (noop) client suitable for tests. - cmd := NewCreateCmd(func(string, bool) *fn.Client { + cmd := NewCreateCmd(func(createConfig) *fn.Client { return fn.New() }) diff --git a/cmd/delete.go b/cmd/delete.go index 47a7a0cda..34d7f2d90 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -13,11 +13,33 @@ import ( ) func init() { - root.AddCommand(deleteCmd) + // Create a new delete command with a reference to + // a function which yields an appropriate concrete client instance. + root.AddCommand(NewDeleteCmd(newDeleteClient)) } -func NewDeleteCmd(newRemover func(ns string, verbose bool) (fn.Remover, error)) *cobra.Command { - delCmd := &cobra.Command{ +// newDeleteClient returns an instance of a Client using the +// final config state. +// Testing note: This method is swapped out during testing to allow +// mocking the remover or the client itself to fabricate test states. +func newDeleteClient(cfg deleteConfig) (*fn.Client, error) { + remover, err := knative.NewRemover(cfg.Namespace) + if err != nil { + return nil, err + } + + remover.Verbose = cfg.Verbose + + return fn.New( + fn.WithRemover(remover), + fn.WithVerbose(cfg.Verbose)), nil +} + +// A deleteClientFn is a function which yields a Client instance from a config +type deleteClientFn func(deleteConfig) (*fn.Client, error) + +func NewDeleteCmd(clientFn deleteClientFn) *cobra.Command { + cmd := &cobra.Command{ Use: "delete [NAME]", Short: "Undeploy a function", Long: `Undeploy a function @@ -38,71 +60,65 @@ kn func delete -n apps myfunc SuggestFor: []string{"remove", "rm", "del"}, ValidArgsFunction: CompleteFunctionList, PreRunE: bindEnv("path", "confirm", "namespace"), - RunE: func(cmd *cobra.Command, args []string) (err error) { - config, err := newDeleteConfig(args).Prompt() - if err != nil { - if err == terminal.InterruptErr { - return nil - } - return - } - - var function fn.Function - - // Initialize func with explicit name (when provided) - if len(args) > 0 && args[0] != "" { - pathChanged := cmd.Flags().Changed("path") - if pathChanged { - return fmt.Errorf("Only one of --path and [NAME] should be provided") - } - function = fn.Function{ - Name: args[0], - } - } else { - function, err = fn.NewFunction(config.Path) - if err != nil { - return - } - - // Check if the Function has been initialized - if !function.Initialized() { - return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) - } - } - - ns := config.Namespace - if ns == "" { - ns = function.Namespace - } - - remover, err := newRemover(ns, config.Verbose) - if err != nil { - return - } - - client := fn.New( - fn.WithVerbose(config.Verbose), - fn.WithRemover(remover)) - - return client.Remove(cmd.Context(), function) - }, } - delCmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") - delCmd.Flags().StringP("path", "p", cwd(), "Path to the function project that should be undeployed (Env: $FUNC_PATH)") - delCmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") + cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + cmd.Flags().StringP("path", "p", cwd(), "Path to the function project that should be undeployed (Env: $FUNC_PATH)") + cmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") - return delCmd + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runDelete(cmd, args, clientFn) + } + + return cmd } -var deleteCmd = NewDeleteCmd(func(ns string, verbose bool) (fn.Remover, error) { - r, err := knative.NewRemover(ns) +func runDelete(cmd *cobra.Command, args []string, clientFn deleteClientFn) (err error) { + config, err := newDeleteConfig(args).Prompt() if err != nil { - return nil, err + if err == terminal.InterruptErr { + return nil + } + return } - r.Verbose = verbose - return r, nil -}) + + var function fn.Function + + // Initialize func with explicit name (when provided) + if len(args) > 0 && args[0] != "" { + pathChanged := cmd.Flags().Changed("path") + if pathChanged { + return fmt.Errorf("Only one of --path and [NAME] should be provided") + } + function = fn.Function{ + Name: args[0], + } + } else { + function, err = fn.NewFunction(config.Path) + if err != nil { + return + } + + // Check if the Function has been initialized + if !function.Initialized() { + return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) + } + } + + // If not provided, use the function's extant namespace + if config.Namespace == "" { + config.Namespace = function.Namespace + } + + // Create a client instance from the now-final config + client, err := clientFn(config) + if err != nil { + return err + } + + // Invoke remove using the concrete client impl + return client.Remove(cmd.Context(), function) +} type deleteConfig struct { Name string diff --git a/cmd/delete_test.go b/cmd/delete_test.go index f2b904fa6..10c7295f2 100644 --- a/cmd/delete_test.go +++ b/cmd/delete_test.go @@ -1,48 +1,57 @@ package cmd import ( - "context" "io/ioutil" - "os" - "path/filepath" "testing" fn "github.com/boson-project/func" + "github.com/boson-project/func/mock" ) -type testRemover struct { - invokedWith *string -} +// TestDeleteByName ensures that running delete specifying the name of the Funciton +// explicitly as an argument invokes the remover appropriately. +func TestDeleteByName(t *testing.T) { + var ( + testname = "testname" // explict name with which to create the Funciton + args = []string{testname} // passed as the lone argument + remover = mock.NewRemover() // with a mock remover + ) -func (t *testRemover) Remove(ctx context.Context, name string) error { - t.invokedWith = &name - return nil -} + // Remover fails the test if it receives the incorrect name + // an incorrect name. + remover.RemoveFn = func(n string) error { + if n != testname { + t.Fatalf("expected delete name %v, got %v", testname, n) + } + return nil + } -// test delete outside project just using function name -func TestDeleteCmdWithoutProject(t *testing.T) { - tr := &testRemover{} - cmd := NewDeleteCmd(func(ns string, verbose bool) (fn.Remover, error) { - return tr, nil + // Create a command with a client constructor fn that instantiates a client + // with a the mocked remover. + cmd := NewDeleteCmd(func(_ deleteConfig) (*fn.Client, error) { + return fn.New(fn.WithRemover(remover)), nil }) - cmd.SetArgs([]string{"foo"}) + // Execute the command + cmd.SetArgs(args) err := cmd.Execute() if err != nil { t.Fatal(err) } - if tr.invokedWith == nil { - t.Fatal("fn.Remover has not been invoked") - } - - if *tr.invokedWith != "foo" { - t.Fatalf("expected fn.Remover to be called with 'foo', but was called with '%s'", *tr.invokedWith) + // Fail if remover's .Remove not invoked at all + if !remover.RemoveInvoked { + t.Fatal("fn.Remover not invoked") } } -// test delete from inside project directory (reading func name from func.yaml) -func TestDeleteCmdWithProject(t *testing.T) { +// TestDeleteByProject ensures that running delete with a valid project as its +// context invokes remove and with the correct name (reads name from func.yaml) +func TestDeleteByProject(t *testing.T) { + // from within a new temporary directory + defer fromTempDir(t)() + + // Write a func.yaml config which specifies a name funcYaml := `name: bar namespace: "" runtime: go @@ -54,73 +63,65 @@ builderMap: envs: [] annotations: {} ` - tmpDir, err := ioutil.TempDir("", "bar") - if err != nil { + if err := ioutil.WriteFile("func.yaml", []byte(funcYaml), 0600); err != nil { t.Fatal(err) } - defer os.RemoveAll(tmpDir) - f, err := os.Create(filepath.Join(tmpDir, "func.yaml")) - if err != nil { - t.Fatal(err) - } - defer f.Close() - - _, err = f.WriteString(funcYaml) - if err != nil { - t.Fatal(err) - } - f.Close() - - oldWD, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - defer func() { - err = os.Chdir(oldWD) - if err != nil { - t.Fatal(err) + // A mock remover which fails if the name from the func.yaml is not received. + remover := mock.NewRemover() + remover.RemoveFn = func(n string) error { + if n != "bar" { + t.Fatalf("expected name 'bar', got '%v'", n) } - }() - err = os.Chdir(tmpDir) - if err != nil { - t.Fatal(err) + return nil } - tr := &testRemover{} - cmd := NewDeleteCmd(func(ns string, verbose bool) (fn.Remover, error) { - return tr, nil + // Command with a Client constructor that returns client with the + // mocked remover. + cmd := NewDeleteCmd(func(_ deleteConfig) (*fn.Client, error) { + return fn.New(fn.WithRemover(remover)), nil }) - cmd.SetArgs([]string{"-p", "."}) - err = cmd.Execute() + // Execute the command simulating no arguments. + cmd.SetArgs([]string{}) + err := cmd.Execute() if err != nil { t.Fatal(err) } - if tr.invokedWith == nil { - t.Fatal("fn.Remover has not been invoked") - } - - if *tr.invokedWith != "bar" { - t.Fatalf("expected fn.Remover to be called with 'bar', but was called with '%s'", *tr.invokedWith) + // Also fail if remover's .Remove is not invoked + if !remover.RemoveInvoked { + t.Fatal("fn.Remover not invoked") } } -// test where both name and path are provided -func TestDeleteCmdWithBothPathAndName(t *testing.T) { - tr := &testRemover{} - cmd := NewDeleteCmd(func(ns string, verbose bool) (fn.Remover, error) { - return tr, nil +// TestDeleteNameAndPathExclusivity ensures that providing both a name and a +// path generates an error. +// Providing the --path (-p) flag indicates the name of the funciton to delete +// is to be taken from the Function at the given path. Providing the name as +// an argument as well is therefore redundant and an error. +func TestDeleteNameAndPathExclusivity(t *testing.T) { + + // A mock remover which will be sampled to ensure it is not invoked. + remover := mock.NewRemover() + + // Command with a Client constructor using the mock remover. + cmd := NewDeleteCmd(func(_ deleteConfig) (*fn.Client, error) { + return fn.New(fn.WithRemover(remover)), nil }) - cmd.SetArgs([]string{"foo", "-p", "/adir/"}) + // Execute the command simulating the invalid argument combination of both + // a path and an explicit name. + cmd.SetArgs([]string{"-p", "./testpath", "testname"}) err := cmd.Execute() if err == nil { - t.Fatal("error was expected as both name an path cannot be used together") + // TODO should really either parse the output or use typed errors to ensure it's + // failing for the expected reason. + t.Fatal(err) } - if tr.invokedWith != nil { - t.Fatal("fn.Remove was call when it shouldn't have been") + // Also fail if remover's .Remove is invoked. + if remover.RemoveInvoked { + t.Fatal("fn.Remover invoked despite invalid combination and an error") } } diff --git a/cmd/deploy.go b/cmd/deploy.go index f533a2f9c..796f3773d 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -21,22 +21,46 @@ import ( ) func init() { - root.AddCommand(deployCmd) - deployCmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") - deployCmd.Flags().StringArrayP("env", "e", []string{}, "Environment variable to set in the form NAME=VALUE. "+ - "You may provide this flag multiple times for setting multiple environment variables. "+ - "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") - deployCmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE") - deployCmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") - deployCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") - deployCmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)") - deployCmd.Flags().BoolP("build", "b", true, "Build the image before deploying (Env: $FUNC_BUILD)") + root.AddCommand(NewDeployCmd(newDeployClient)) } -var deployCmd = &cobra.Command{ - Use: "deploy", - Short: "Deploy a function", - Long: `Deploy a function +func newDeployClient(cfg deployConfig) (*fn.Client, error) { + listener := progress.New() + + builder := buildpacks.NewBuilder() + + pusher, err := docker.NewPusher(docker.WithCredentialsProvider(credentialsProvider)) + if err != nil { + return nil, err + } + + deployer, err := knative.NewDeployer(cfg.Namespace) + if err != nil { + return nil, err + } + + listener.Verbose = cfg.Verbose + builder.Verbose = cfg.Verbose + pusher.Verbose = cfg.Verbose + deployer.Verbose = cfg.Verbose + + return fn.New( + fn.WithProgressListener(listener), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + fn.WithDeployer(deployer), + fn.WithRegistry(cfg.Registry), // for deriving name when no --image value + fn.WithVerbose(cfg.Verbose), + ), nil +} + +type deployClientFn func(deployConfig) (*fn.Client, error) + +func NewDeployCmd(clientFn deployClientFn) *cobra.Command { + cmd := &cobra.Command{ + Use: "deploy", + Short: "Deploy a function", + Long: `Deploy a function Builds a container image for the function and deploys it to the connected Knative enabled cluster. The function is picked up from the project in the current directory or from the path provided @@ -47,7 +71,7 @@ in the configuration file. If the function is already deployed, it is updated with a new container image that is pushed to an image registry, and finally the function's Knative service is updated. `, - Example: ` + Example: ` # Build and deploy the function from the current directory's project. The image will be # pushed to "quay.io/myuser/" and deployed as Knative service with the # same name as the function to the currently connected cluster. @@ -57,17 +81,33 @@ kn func deploy --registry quay.io/myuser # the namespace "myns" kn func deploy --image quay.io/myuser/myfunc -n myns `, - SuggestFor: []string{"delpoy", "deplyo"}, - PreRunE: bindEnv("image", "namespace", "path", "registry", "confirm", "build"), - RunE: runDeploy, + SuggestFor: []string{"delpoy", "deplyo"}, + PreRunE: bindEnv("image", "namespace", "path", "registry", "confirm", "build"), + } + + cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + cmd.Flags().StringArrayP("env", "e", []string{}, "Environment variable to set in the form NAME=VALUE. "+ + "You may provide this flag multiple times for setting multiple environment variables. "+ + "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") + cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE") + cmd.Flags().StringP("namespace", "n", "", "Namespace of the function to undeploy. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") + cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") + cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)") + cmd.Flags().BoolP("build", "b", true, "Build the image before deploying (Env: $FUNC_BUILD)") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runDeploy(cmd, args, clientFn) + } + + return cmd } -func runDeploy(cmd *cobra.Command, _ []string) (err error) { - +func runDeploy(cmd *cobra.Command, _ []string, clientFn deployClientFn) (err error) { config, err := newDeployConfig(cmd) if err != nil { return err } + config, err = config.Prompt() if err != nil { if err == terminal.InterruptErr { @@ -120,55 +160,26 @@ func runDeploy(cmd *cobra.Command, _ []string) (err error) { return } - builder := buildpacks.NewBuilder() - builder.Verbose = config.Verbose + // Deafult conig namespace is the function's namespace + if config.Namespace == "" { + config.Namespace = function.Namespace + } - pusher, err := docker.NewPusher(docker.WithCredentialsProvider(credentialsProvider)) + client, err := clientFn(config) if err != nil { if err == terminal.InterruptErr { return nil } return err } - pusher.Verbose = config.Verbose - - ns := config.Namespace - if ns == "" { - ns = function.Namespace - } - - deployer, err := knative.NewDeployer(ns) - if err != nil { - return - } - - listener := progress.New() - defer listener.Done() - - deployer.Verbose = config.Verbose - listener.Verbose = config.Verbose - - context := cmd.Context() - go func() { - <-context.Done() - listener.Done() - }() - - client := fn.New( - fn.WithVerbose(config.Verbose), - fn.WithRegistry(config.Registry), // for deriving image name when --image not provided explicitly. - fn.WithBuilder(builder), - fn.WithPusher(pusher), - fn.WithDeployer(deployer), - fn.WithProgressListener(listener)) if config.Build { - if err := client.Build(context, config.Path); err != nil { + if err := client.Build(cmd.Context(), config.Path); err != nil { return err } } - return client.Deploy(context, config.Path) + return client.Deploy(cmd.Context(), config.Path) // NOTE: Namespace is optional, default is that used by k8s client // (for example kubectl usually uses ~/.kube/config) diff --git a/cmd/describe.go b/cmd/describe.go index b95b69550..8d6ec82c1 100644 --- a/cmd/describe.go +++ b/cmd/describe.go @@ -16,39 +16,62 @@ import ( ) func init() { - root.AddCommand(describeCmd) - describeCmd.Flags().StringP("namespace", "n", "", "Namespace of the function. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") - describeCmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT)") - describeCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") - - err := describeCmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList) - if err != nil { - fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) - } + root.AddCommand(NewDescribeCmd(newDescribeClient)) } -var describeCmd = &cobra.Command{ - Use: "describe ", - Short: "Show details of a function", - Long: `Show details of a function +func newDescribeClient(cfg describeConfig) (*fn.Client, error) { + describer, err := knative.NewDescriber(cfg.Namespace) + if err != nil { + return nil, err + } + + describer.Verbose = cfg.Verbose + + return fn.New( + fn.WithDescriber(describer), + fn.WithVerbose(cfg.Verbose), + ), nil +} + +type describeClientFn func(describeConfig) (*fn.Client, error) + +func NewDescribeCmd(clientFn describeClientFn) *cobra.Command { + cmd := &cobra.Command{ + Use: "describe ", + Short: "Show details of a function", + Long: `Show details of a function Prints the name, route and any event subscriptions for a deployed function in the current directory or from the directory specified with --path. `, - Example: ` + Example: ` # Show the details of a function as declared in the local func.yaml kn func describe # Show the details of the function in the myotherfunc directory with yaml output kn func describe --output yaml --path myotherfunc `, - SuggestFor: []string{"desc", "get"}, - ValidArgsFunction: CompleteFunctionList, - PreRunE: bindEnv("namespace", "output", "path"), - RunE: runDescribe, + SuggestFor: []string{"desc", "get"}, + ValidArgsFunction: CompleteFunctionList, + PreRunE: bindEnv("namespace", "output", "path"), + } + + cmd.Flags().StringP("namespace", "n", "", "Namespace of the function. By default, the namespace in func.yaml is used or the actual active namespace if not set in the configuration. (Env: $FUNC_NAMESPACE)") + cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT)") + cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") + + if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil { + fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) + } + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runDescribe(cmd, args, clientFn) + } + + return cmd } -func runDescribe(cmd *cobra.Command, args []string) (err error) { +func runDescribe(cmd *cobra.Command, args []string, clientFn describeClientFn) (err error) { config := newDescribeConfig(args) function, err := fn.NewFunction(config.Path) @@ -61,16 +84,13 @@ func runDescribe(cmd *cobra.Command, args []string) (err error) { return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) } - describer, err := knative.NewDescriber(config.Namespace) + // Create a client + client, err := clientFn(config) if err != nil { - return + return err } - describer.Verbose = config.Verbose - - client := fn.New( - fn.WithVerbose(config.Verbose), - fn.WithDescriber(describer)) + // Get the description d, err := client.Describe(cmd.Context(), config.Name, config.Path) if err != nil { return diff --git a/cmd/emit.go b/cmd/emit.go index b62aeb2f6..9b7cf54a0 100644 --- a/cmd/emit.go +++ b/cmd/emit.go @@ -1,7 +1,8 @@ package cmd import ( - "fmt" + "context" + "errors" "io/ioutil" fn "github.com/boson-project/func" @@ -13,27 +14,41 @@ import ( ) func init() { - e := cloudevents.NewEmitter() - root.AddCommand(emitCmd) - // TODO: do these env vars make sense? - emitCmd.Flags().StringP("sink", "k", "", "Send the CloudEvent to the function running at [sink]. The special value \"local\" can be used to send the event to a function running on the local host. When provided, the --path flag is ignored (Env: $FUNC_SINK)") - emitCmd.Flags().StringP("source", "s", e.Source, "CloudEvent source (Env: $FUNC_SOURCE)") - emitCmd.Flags().StringP("type", "t", e.Type, "CloudEvent type (Env: $FUNC_TYPE)") - emitCmd.Flags().StringP("id", "i", uuid.NewString(), "CloudEvent ID (Env: $FUNC_ID)") - emitCmd.Flags().StringP("data", "d", "", "Any arbitrary string to be sent as the CloudEvent data. Ignored if --file is provided (Env: $FUNC_DATA)") - emitCmd.Flags().StringP("file", "f", "", "Path to a local file containing CloudEvent data to be sent (Env: $FUNC_FILE)") - emitCmd.Flags().StringP("content-type", "c", "application/json", "The MIME Content-Type for the CloudEvent data (Env: $FUNC_CONTENT_TYPE)") - emitCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory. Ignored when --sink is provided (Env: $FUNC_PATH)") + root.AddCommand(NewEmitCmd(newEmitClient)) } -var emitCmd = &cobra.Command{ - Use: "emit", - Short: "Emit a CloudEvent to a function endpoint", - Long: `Emit event +// create a fn.Client with an instance of a +func newEmitClient(cfg emitConfig) (*fn.Client, error) { + e := cloudevents.NewEmitter() + e.Id = cfg.Id + e.Source = cfg.Source + e.Type = cfg.Type + e.ContentType = cfg.ContentType + e.Data = cfg.Data + if cfg.File != "" { + // See config.Validate for --Data and --file exclusivity enforcement + b, err := ioutil.ReadFile(cfg.File) + if err != nil { + return nil, err + } + e.Data = string(b) + } + + return fn.New(fn.WithEmitter(e)), nil +} + +type emitClientFn func(emitConfig) (*fn.Client, error) + +func NewEmitCmd(clientFn emitClientFn) *cobra.Command { + + cmd := &cobra.Command{ + Use: "emit", + Short: "Emit a CloudEvent to a function endpoint", + Long: `Emit event Emits a CloudEvent, sending it to the deployed function. `, - Example: ` + Example: ` # Send a CloudEvent to the deployed function with no data and default values # for source, type and ID kn func emit @@ -54,66 +69,102 @@ kn func emit --path /path/to/fn -i fn.test # Send a CloudEvent to an arbitrary endpoint kn func emit --sink "http://my.event.broker.com" `, - SuggestFor: []string{"meit", "emti", "send"}, - PreRunE: bindEnv("source", "type", "id", "data", "file", "path", "sink", "content-type"), - RunE: runEmit, + SuggestFor: []string{"meit", "emti", "send"}, + PreRunE: bindEnv("source", "type", "id", "data", "file", "path", "sink", "content-type"), + } + + cmd.Flags().StringP("sink", "k", "", "Send the CloudEvent to the function running at [sink]. The special value \"local\" can be used to send the event to a function running on the local host. When provided, the --path flag is ignored (Env: $FUNC_SINK)") + cmd.Flags().StringP("source", "s", cloudevents.DefaultSource, "CloudEvent source (Env: $FUNC_SOURCE)") + cmd.Flags().StringP("type", "t", cloudevents.DefaultType, "CloudEvent type (Env: $FUNC_TYPE)") + cmd.Flags().StringP("id", "i", uuid.NewString(), "CloudEvent ID (Env: $FUNC_ID)") + cmd.Flags().StringP("data", "d", "", "Any arbitrary string to be sent as the CloudEvent data. Ignored if --file is provided (Env: $FUNC_DATA)") + cmd.Flags().StringP("file", "f", "", "Path to a local file containing CloudEvent data to be sent (Env: $FUNC_FILE)") + cmd.Flags().StringP("content-type", "c", "application/json", "The MIME Content-Type for the CloudEvent data (Env: $FUNC_CONTENT_TYPE)") + cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory. Ignored when --sink is provided (Env: $FUNC_PATH)") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runEmit(cmd, args, clientFn) + } + + return cmd + } -func runEmit(cmd *cobra.Command, args []string) (err error) { +func runEmit(cmd *cobra.Command, _ []string, clientFn emitClientFn) (err error) { config := newEmitConfig() - var endpoint string - if config.Sink != "" { - if config.Sink == "local" { - endpoint = "http://localhost:8080" - } else { - endpoint = config.Sink - } - } else { - var f fn.Function - f, err = fn.NewFunction(config.Path) - if err != nil { - return - } - // What happens if the function hasn't been deployed but they don't run with --local=true - // Maybe we should be thinking about saving the endpoint URL in func.yaml after each deploy - var d *knative.Describer - d, err = knative.NewDescriber("") - if err != nil { - return - } - var desc fn.Description - desc, err = d.Describe(cmd.Context(), f.Name) - if err != nil { - return - } - // Use the first available route - endpoint = desc.Routes[0] + + // Validate things like invalid config combinations. + if err := config.Validate(); err != nil { + return err } - emitter := cloudevents.NewEmitter() - emitter.Source = config.Source - emitter.Type = config.Type - emitter.Id = config.Id - emitter.ContentType = config.ContentType - emitter.Data = config.Data - if config.File != "" { - var buf []byte - if emitter.Data != "" && config.Verbose { - return fmt.Errorf("Only one of --data and --file may be specified \n") - } - buf, err = ioutil.ReadFile(config.File) - if err != nil { - return - } - emitter.Data = string(buf) + // Determine the final endpoint, taking into account the special value "local", + // and sampling the function's current route if not explicitly provided + endpoint, err := endpoint(cmd.Context(), config) + if err != nil { + return err } - client := fn.New( - fn.WithEmitter(emitter), - ) + // Instantiate a client based on the final value of config + client, err := clientFn(config) + if err != nil { + return err + } + + // Emit the event to the endpoint return client.Emit(cmd.Context(), endpoint) } +// endpoint returns the final effective endpoint. +// By default, the contextually active Function is queried for it's current +// address (route). +// If "local" is specified in cfg.Sink, localhost is used. +// Otherwise the value of Sink is used verbatim if defined. +func endpoint(ctx context.Context, cfg emitConfig) (url string, err error) { + var ( + f fn.Function + d fn.Describer + desc fn.Description + ) + + // If the special value "local" was requested, + // use localhost. + if cfg.Sink == "local" { + return "http://localhost:8080", nil + } + + // If a sink was expressly provided, use that verbatim + if cfg.Sink != "" { + return cfg.Sink, nil + } + + // If no sink was specified, use the route to the currently + // contectually active function + if f, err = fn.NewFunction(cfg.Path); err != nil { + return + } + + // TODO: Decide what happens if the function hasn't been deployed but they + // don't run with --local=true. Perhaps an error in .Validate()? + if d, err = knative.NewDescriber(""); err != nil { + return + } + + // Get the current state of the function. + if desc, err = d.Describe(ctx, f.Name); err != nil { + return + } + + // Probably wise to be defensive here: + if len(desc.Routes) == 0 { + err = errors.New("function has no active routes") + return + } + + // The first route should be the destination. + return desc.Routes[0], nil +} + type emitConfig struct { Path string Source string @@ -139,3 +190,11 @@ func newEmitConfig() emitConfig { Verbose: viper.GetBool("verbose"), } } + +func (c emitConfig) Validate() error { + if c.Data != "" && c.File != "" { + return errors.New("Only one of --data or --file may be specified") + } + // TODO: should we verify that sink is a url or "local"? + return nil +} diff --git a/cmd/list.go b/cmd/list.go index d3d5620f1..26ac67e5c 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "encoding/xml" + "errors" "fmt" "io" "os" @@ -17,24 +18,36 @@ import ( ) func init() { - root.AddCommand(listCmd) - listCmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.") - listCmd.Flags().StringP("namespace", "n", "", "Namespace to search for functions. By default, the functions of the actual active namespace are listed. (Env: $FUNC_NAMESPACE)") - listCmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)") - err := listCmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList) - if err != nil { - fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) - } + root.AddCommand(NewListCmd(newListClient)) } -var listCmd = &cobra.Command{ - Use: "list", - Short: "List functions", - Long: `List functions +func newListClient(cfg listConfig) (*fn.Client, error) { + // TODO(lkingland): does an empty namespace mean all namespaces + // or the default namespace as defined in user's config? + lister, err := knative.NewLister(cfg.Namespace) + if err != nil { + return nil, err + } + + lister.Verbose = cfg.Verbose + + return fn.New( + fn.WithLister(lister), + fn.WithVerbose(cfg.Verbose), + ), nil +} + +type listClientFn func(listConfig) (*fn.Client, error) + +func NewListCmd(clientFn listClientFn) *cobra.Command { + cmd := &cobra.Command{ + Use: "list", + Short: "List functions", + Long: `List functions Lists all deployed functions in a given namespace. `, - Example: ` + Example: ` # List all functions in the current namespace with human readable output kn func list @@ -44,40 +57,51 @@ kn func list --namespace test --output yaml # List all functions in all namespaces with JSON output kn func list --all-namespaces --output json `, - SuggestFor: []string{"ls", "lsit"}, - PreRunE: bindEnv("namespace", "output"), - RunE: runList, + SuggestFor: []string{"ls", "lsit"}, + PreRunE: bindEnv("namespace", "output"), + } + + cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.") + cmd.Flags().StringP("namespace", "n", "", "Namespace to search for functions. By default, the functions of the actual active namespace are listed. (Env: $FUNC_NAMESPACE)") + cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)") + + if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil { + fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) + } + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runList(cmd, args, clientFn) + } + + return cmd } -func runList(cmd *cobra.Command, args []string) (err error) { +func runList(cmd *cobra.Command, _ []string, clientFn listClientFn) (err error) { config := newListConfig() - lister, err := knative.NewLister(config.Namespace) - if err != nil { - return - } - lister.Verbose = config.Verbose - - a, err := cmd.Flags().GetBool("all-namespaces") - if err != nil { - return - } - if a { - lister.Namespace = "" + if err := config.Validate(); err != nil { + return err } - client := fn.New( - fn.WithVerbose(config.Verbose), - fn.WithLister(lister)) + client, err := clientFn(config) + if err != nil { + return err + } items, err := client.List(cmd.Context()) if err != nil { return } - if len(items) < 1 { - fmt.Printf("No functions found in %v namespace\n", lister.Namespace) - return + if len(items) == 0 { + // TODO(lkingland): this isn't particularly script friendly. Suggest this + // prints bo only on --verbose. Possible future tweak, as I don't want to + // make functional changes during a refactor. + if config.Namespace != "" && !config.AllNamespaces { + fmt.Printf("No functions found in '%v' namespace\n", config.Namespace) + } else { + fmt.Println("No functions found") + } } write(os.Stdout, listItems(items), config.Output) @@ -89,19 +113,28 @@ func runList(cmd *cobra.Command, args []string) (err error) { // ------------------------------ type listConfig struct { - Namespace string - Output string - Verbose bool + Namespace string + Output string + AllNamespaces bool + Verbose bool } func newListConfig() listConfig { return listConfig{ - Namespace: viper.GetString("namespace"), - Output: viper.GetString("output"), - Verbose: viper.GetBool("verbose"), + Namespace: viper.GetString("namespace"), + Output: viper.GetString("output"), + AllNamespaces: viper.GetBool("all-namespaces"), + Verbose: viper.GetBool("verbose"), } } +func (c listConfig) Validate() error { + if c.Namespace != "" && c.AllNamespaces { + return errors.New("Both --namespace and --all-namespaces specified.") + } + return nil +} + // Output Formatting (serializers) // ------------------------------- diff --git a/cmd/run.go b/cmd/run.go index 85dcdef43..25c9800b1 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -12,35 +12,54 @@ import ( ) func init() { - // Add the run command as a subcommand of root. - root.AddCommand(runCmd) - runCmd.Flags().StringArrayP("env", "e", []string{}, "Environment variable to set in the form NAME=VALUE. "+ - "You may provide this flag multiple times for setting multiple environment variables. "+ - "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") - runCmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") + root.AddCommand(NewRunCmd(newRunClient)) } -var runCmd = &cobra.Command{ - Use: "run", - Short: "Run the function locally", - Long: `Run the function locally +func newRunClient(cfg runConfig) *fn.Client { + runner := docker.NewRunner() + runner.Verbose = cfg.Verbose + return fn.New( + fn.WithRunner(runner), + fn.WithVerbose(cfg.Verbose)) +} + +type runClientFn func(runConfig) *fn.Client + +func NewRunCmd(clientFn runClientFn) *cobra.Command { + + cmd := &cobra.Command{ + Use: "run", + Short: "Run the function locally", + Long: `Run the function locally Runs the function locally in the current directory or in the directory specified by --path flag. The function must already have been built with the 'build' command. `, - Example: ` + Example: ` # Build function's image first kn func build # Run it locally as a container kn func run `, - SuggestFor: []string{"rnu"}, - PreRunE: bindEnv("path"), - RunE: runRun, + SuggestFor: []string{"rnu"}, + PreRunE: bindEnv("path"), + } + + cmd.Flags().StringArrayP("env", "e", []string{}, + "Environment variable to set in the form NAME=VALUE. "+ + "You may provide this flag multiple times for setting multiple environment variables. "+ + "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") + cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return runRun(cmd, args, clientFn) + } + + return cmd } -func runRun(cmd *cobra.Command, args []string) (err error) { +func runRun(cmd *cobra.Command, args []string, clientFn runClientFn) (err error) { config, err := newRunConfig(cmd) if err != nil { return @@ -66,15 +85,9 @@ func runRun(cmd *cobra.Command, args []string) (err error) { return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) } - runner := docker.NewRunner() - runner.Verbose = config.Verbose + client := clientFn(config) - client := fn.New( - fn.WithRunner(runner), - fn.WithVerbose(config.Verbose)) - - err = client.Run(cmd.Context(), config.Path) - return + return client.Run(cmd.Context(), config.Path) } type runConfig struct { @@ -92,10 +105,10 @@ type runConfig struct { EnvToRemove []string } -func newRunConfig(cmd *cobra.Command) (runConfig, error) { +func newRunConfig(cmd *cobra.Command) (c runConfig, err error) { envToUpdate, envToRemove, err := envFromCmd(cmd) if err != nil { - return runConfig{}, err + return } return runConfig{ diff --git a/knative/client.go b/knative/client.go index cdeed75b0..2d5ddd4b2 100644 --- a/knative/client.go +++ b/knative/client.go @@ -13,7 +13,7 @@ import ( ) const ( - DefaultWaitingTimeout = 60 * time.Second + DefaultWaitingTimeout = 120 * time.Second ) func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) { diff --git a/progress/progress.go b/progress/progress.go index 3df368b9f..75342e74f 100644 --- a/progress/progress.go +++ b/progress/progress.go @@ -153,6 +153,13 @@ func (b *Bar) Complete(text string) { b.Done() // stop spinner } +// Stopping indicates the process is stopping, such as having received a context +// cancellation. +func (b *Bar) Stopping() { + // currently stopping is equivalent in effect to Done + b.Done() +} + // Done cancels the write loop if being used. // Call in a defer statement after creation to ensure that the spinner stops func (b *Bar) Done() {