From f6e60216930fe87f963434466a877046a0cfb917 Mon Sep 17 00:00:00 2001 From: David Fridrich <49119790+gauron99@users.noreply.github.com> Date: Wed, 20 Aug 2025 15:18:49 +0200 Subject: [PATCH] Remove container flag - variant I (#2987) --- cmd/run.go | 111 ++++++++----------------- cmd/run_test.go | 105 ----------------------- docs/reference/func_run.md | 43 ++++------ test/e2e/scenario_no_container_test.go | 4 +- 4 files changed, 56 insertions(+), 207 deletions(-) diff --git a/cmd/run.go b/cmd/run.go index bcc5b3528..1ca01cd75 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -28,8 +28,8 @@ NAME {{rootCmdUse}} run - Run a function locally SYNOPSIS - {{rootCmdUse}} run [-t|--container] [-r|--registry] [-i|--image] [-e|--env] - [--build] [-b|--builder] [--builder-image] [-c|--confirm] + {{rootCmdUse}} run [-r|--registry] [-i|--image] [-e|--env] [--build] + [-b|--builder] [--builder-image] [-c|--confirm] [--address] [--json] [-v|--verbose] DESCRIPTION @@ -38,26 +38,19 @@ DESCRIPTION Values provided for flags are not persisted to the function's metadata. Containerized Runs - The --container flag indicates that the function's container should be - run rather than running the source code directly. This may require that - the function's container first be rebuilt. Building the container on or - off can be altered using the --build flag. The value --build=auto - can be used to indicate the function should be run in a container, with - the container automatically built if necessary. - - The --container flag defaults to true if the builder defined for the - function is a containerized builder such as Pack or S2I, and in the case - where the function's runtime requires containerized builds (is not yet - supported by the Host builder. + You can build your function in a container using the Pack or S2i builders. + On the contrary, non-containerized run is achieved via Host builder which + will use your host OS' environment to build the function. This builder is + currently enabled for Go and Python. Building defaults to using the Host + builder when available. You can alter this by using the --builder flag + eg: --builder=s2i. Process Scaffolding - This is an Experimental Feature currently available only to Go projects. - When running a function with --container=false (host-based runs), the - function is first wrapped code which presents it as a process. - This "scaffolding" is transient, written for each build or run, and should - in most cases be transparent to a function author. However, to customize, - or even completely replace this scafolding code, see the 'scaffold' - subcommand. + This is an Experimental Feature currently available only to Go and Python + projects. When running a function with --builder=host, the function is + first wrapped with code which presents it as a process. This "scaffolding" + is transient, written for each build or run, and should in most cases be + transparent to a function author. EXAMPLES @@ -65,11 +58,12 @@ EXAMPLES $ {{rootCmdUse}} run o Run the function locally from within its container, forcing a rebuild - of the container even if no filesysem changes are detected - $ {{rootCmdUse}} run --build + of the container even if no filesystem changes are detected. There are 2 + builders available for containerized build - 'pack' and 's2i'. + $ {{rootCmdUse}} run --build= - o Run the function locally on the host with no containerization (Go only). - $ {{rootCmdUse}} run --container=false + o Run the function locally on the host with no containerization (Go/Python only). + $ {{rootCmdUse}} run --builder=host o Run the function locally on a specific address. $ {{rootCmdUse}} run --address='[::]:8081' @@ -79,7 +73,7 @@ EXAMPLES `, SuggestFor: []string{"rnu"}, PreRunE: bindEnv("build", "builder", "builder-image", "base-image", - "confirm", "container", "env", "image", "path", "registry", + "confirm", "env", "image", "path", "registry", "start-timeout", "verbose", "address", "json"), RunE: func(cmd *cobra.Command, _ []string) error { return runRun(cmd, newClient) @@ -121,8 +115,6 @@ EXAMPLES "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().Duration("start-timeout", f.Run.StartTimeout, fmt.Sprintf("time this function needs in order to start. If not provided, the client default %v will be in effect. ($FUNC_START_TIMEOUT)", fn.DefaultStartTimeout)) - cmd.Flags().BoolP("container", "t", runContainerizedByDefault(f), - "Run the function in a container. ($FUNC_CONTAINER)") // TODO: Without the "Host" builder enabled, this code-path is unreachable, // so remove hidden flag when either the Host builder path is available, @@ -157,10 +149,6 @@ EXAMPLES return cmd } -func runContainerizedByDefault(f fn.Function) bool { - return f.Build.Builder == "pack" || f.Build.Builder == "s2i" || !oci.IsSupported(f.Runtime) -} - func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { var ( cfg runConfig @@ -174,27 +162,16 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if !f.Initialized() { return fn.NewErrNotInitialized(f.Root) } + + if err = cfg.Validate(cmd, f); err != nil { + return + } + if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg return } - // Smart auto-fix logic for builder/container compatibility - // This fixes the original bug where --builder=pack doesn't default to container=true - - // Case 1: Containerized builders (pack/s2i) should force container=true when not explicitly set - if (f.Build.Builder == "pack" || f.Build.Builder == "s2i") && !cfg.Container && !cmd.Flags().Changed("container") { - cfg.Container = true - } - - // Case 2: container=false should auto-select host builder when no builder explicitly set - if !cfg.Container && cmd.Flags().Changed("container") && !cmd.Flags().Changed("builder") { - f.Build.Builder = "host" - } - - // Validate after configure and auto-fix - if err = cfg.Validate(cmd, f); err != nil { - return - } + container := f.Build.Builder != "host" // Ignore the verbose flag if JSON output if cfg.JSON { @@ -206,7 +183,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if err != nil { return } - if cfg.Container { + if container { clientOptions = append(clientOptions, fn.WithRunner(docker.NewRunner(cfg.Verbose, os.Stdout, os.Stderr))) } if cfg.StartTimeout != 0 { @@ -220,7 +197,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // // If requesting to run via the container, build the container if it is // either out-of-date or a build was explicitly requested. - if cfg.Container { + if container { var digested bool buildOptions, err := cfg.buildOptions() @@ -234,23 +211,17 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if err != nil { return err } - if !digested { - // assign valid undigested image - f.Build.Image = cfg.Image - } + // image was parsed and both digested AND undigested imgs are valid + f.Build.Image = cfg.Image } - if digested { - // run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go - // it doesnt get saved, just runtime image - f.Build.Image = cfg.Image - } else { - + // actual build step + if !digested { if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return err } } - } else { + } else { // if !container // dont run digested image without a container if cfg.Image != "" { digested, err := isDigested(cfg.Image) @@ -258,7 +229,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { return err } if digested { - return fmt.Errorf("cannot use digested image with --container=false") + return fmt.Errorf("cannot use digested image with non-containerized builds (--builder=host)") } } } @@ -330,10 +301,6 @@ type runConfig struct { // Can be 'auto' or a truthy value. Build string - // Container indicates the function should be run in a container. - // Requires the container be built. - Container bool - // Env variables. may include removals using a "-" Env []string @@ -353,7 +320,6 @@ func newRunConfig(cmd *cobra.Command) (c runConfig) { buildConfig: newBuildConfig(), Build: viper.GetString("build"), Env: viper.GetStringSlice("env"), - Container: viper.GetBool("container"), StartTimeout: viper.GetDuration("start-timeout"), Address: viper.GetString("address"), JSON: viper.GetBool("json"), @@ -379,7 +345,7 @@ func (c runConfig) Configure(f fn.Function) (fn.Function, error) { f.Run.Envs, err = applyEnvs(f.Run.Envs, c.Env) - // The other members; build, path, and container; are not part of function + // The other members; build and path; are not part of function // state, so are not mentioned here in Configure. return f, err } @@ -412,18 +378,13 @@ func (c runConfig) Validate(cmd *cobra.Command, f fn.Function) (err error) { } } - if !c.Container && !oci.IsSupported(f.Runtime) { + if f.Build.Builder == "host" && !oci.IsSupported(f.Runtime) { return fmt.Errorf("the %q runtime currently requires being run in a container", f.Runtime) } - // Validate that containerized builders (pack/s2i) cannot be used with container=false - if (f.Build.Builder == "pack" || f.Build.Builder == "s2i") && !c.Container { - return fmt.Errorf("builder %q requires container mode but --container=false was set", f.Build.Builder) - } - // When the docker runner respects the StartTimeout, this validation check // can be removed - if c.StartTimeout != 0 && c.Container { + if c.StartTimeout != 0 && f.Build.Builder != "host" { return errors.New("the ability to specify the startup timeout for containerized runs is coming soon") } diff --git a/cmd/run_test.go b/cmd/run_test.go index 10f5b1c1a..08218cdab 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "path/filepath" "testing" "time" @@ -602,107 +601,3 @@ func TestRun_BaseImage(t *testing.T) { }) } } - -// TestCtrBuilder tests that pack builder auto-forces container mode -func TestCtrBuilder(t *testing.T) { - var runner = mock.NewRunner() - var builder = mock.NewBuilder() - tmp := t.TempDir() - - fnPath := filepath.Join(tmp, "fn") - f, err := fn.New().Init(fn.Function{Root: fnPath, Runtime: "python"}) - if err != nil { - t.Fatal(err) - } - err = f.Write() - if err != nil { - t.Fatal(err) - } - t.Chdir(fnPath) - - cmd := NewRunCmd(NewTestClient( - fn.WithRunner(runner), - fn.WithBuilder(builder), - fn.WithRegistry(TestRegistry), - )) - cmd.SetArgs([]string{"--builder=pack", "--build=1"}) - ctx, cancel := context.WithCancel(context.Background()) - time.AfterFunc(time.Second, func() { - cancel() - }) - err = cmd.ExecuteContext(ctx) - if err != nil { - t.Fatal(err) - } - // verify that builder was called since `--builder=pack` was provided - if !builder.BuildInvoked { - t.Error("builder has not been invoked") - } -} - -// TestCtrBuilderConflict tests that pack builder with explicit container=false shows validation error -func TestCtrBuilderConflict(t *testing.T) { - var runner = mock.NewRunner() - var builder = mock.NewBuilder() - tmp := t.TempDir() - - fnPath := filepath.Join(tmp, "fn") - f, err := fn.New().Init(fn.Function{Root: fnPath, Runtime: "python"}) - if err != nil { - t.Fatal(err) - } - err = f.Write() - if err != nil { - t.Fatal(err) - } - t.Chdir(fnPath) - - cmd := NewRunCmd(NewTestClient( - fn.WithRunner(runner), - fn.WithBuilder(builder), - fn.WithRegistry(TestRegistry), - )) - cmd.SetArgs([]string{"--builder=pack", "--build=1", "--container=0"}) - ctx, cancel := context.WithCancel(context.Background()) - time.AfterFunc(time.Second, func() { - cancel() - }) - err = cmd.ExecuteContext(ctx) - // since explicit flag `--builder=pack` and `--container=0` are contradiction we want error - if err == nil { - t.Error("error expected but got ") - } -} - -// TestSmartBuilderSelection tests that container=false auto-selects host builder -func TestSmartBuilderSelection(t *testing.T) { - var runner = mock.NewRunner() - var builder = mock.NewBuilder() - tmp := t.TempDir() - - fnPath := filepath.Join(tmp, "fn") - f, err := fn.New().Init(fn.Function{Root: fnPath, Runtime: "go"}) - if err != nil { - t.Fatal(err) - } - err = f.Write() - if err != nil { - t.Fatal(err) - } - t.Chdir(fnPath) - - cmd := NewRunCmd(NewTestClient( - fn.WithRunner(runner), - fn.WithBuilder(builder), - fn.WithRegistry(TestRegistry), - )) - cmd.SetArgs([]string{"--container=false", "--build=1"}) - ctx, cancel := context.WithCancel(context.Background()) - time.AfterFunc(time.Second, func() { - cancel() - }) - err = cmd.ExecuteContext(ctx) - if err != nil { - t.Fatal(err) - } -} diff --git a/docs/reference/func_run.md b/docs/reference/func_run.md index 2f9a7cdb3..20092f91d 100644 --- a/docs/reference/func_run.md +++ b/docs/reference/func_run.md @@ -9,8 +9,8 @@ NAME func run - Run a function locally SYNOPSIS - func run [-t|--container] [-r|--registry] [-i|--image] [-e|--env] - [--build] [-b|--builder] [--builder-image] [-c|--confirm] + func run [-r|--registry] [-i|--image] [-e|--env] [--build] + [-b|--builder] [--builder-image] [-c|--confirm] [--address] [--json] [-v|--verbose] DESCRIPTION @@ -19,26 +19,19 @@ DESCRIPTION Values provided for flags are not persisted to the function's metadata. Containerized Runs - The --container flag indicates that the function's container should be - run rather than running the source code directly. This may require that - the function's container first be rebuilt. Building the container on or - off can be altered using the --build flag. The value --build=auto - can be used to indicate the function should be run in a container, with - the container automatically built if necessary. - - The --container flag defaults to true if the builder defined for the - function is a containerized builder such as Pack or S2I, and in the case - where the function's runtime requires containerized builds (is not yet - supported by the Host builder. + You can build your function in a container using the Pack or S2i builders. + On the contrary, non-containerized run is achieved via Host builder which + will use your host OS' environment to build the function. This builder is + currently enabled for Go and Python. Building defaults to using the Host + builder when available. You can alter this by using the --builder flag + eg: --builder=s2i. Process Scaffolding - This is an Experimental Feature currently available only to Go projects. - When running a function with --container=false (host-based runs), the - function is first wrapped code which presents it as a process. - This "scaffolding" is transient, written for each build or run, and should - in most cases be transparent to a function author. However, to customize, - or even completely replace this scafolding code, see the 'scaffold' - subcommand. + This is an Experimental Feature currently available only to Go and Python + projects. When running a function with --builder=host, the function is + first wrapped with code which presents it as a process. This "scaffolding" + is transient, written for each build or run, and should in most cases be + transparent to a function author. EXAMPLES @@ -46,11 +39,12 @@ EXAMPLES $ func run o Run the function locally from within its container, forcing a rebuild - of the container even if no filesysem changes are detected - $ func run --build + of the container even if no filesystem changes are detected. There are 2 + builders available for containerized build - 'pack' and 's2i'. + $ func run --build= - o Run the function locally on the host with no containerization (Go only). - $ func run --container=false + o Run the function locally on the host with no containerization (Go/Python only). + $ func run --builder=host o Run the function locally on a specific address. $ func run --address='[::]:8081' @@ -72,7 +66,6 @@ func run -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". (default "pack") --builder-image string Specify a custom builder image for use by the builder other than its default. ($FUNC_BUILDER_IMAGE) -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) - -t, --container Run the function in a container. ($FUNC_CONTAINER) (default true) -e, --env stringArray 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-). -h, --help help for run -i, --image string Full image name in the form [registry]/[namespace]/[name]:[tag]. This option takes precedence over --registry. Specifying tag is optional. ($FUNC_IMAGE) diff --git a/test/e2e/scenario_no_container_test.go b/test/e2e/scenario_no_container_test.go index 5f7e76e12..9f8cd23a2 100644 --- a/test/e2e/scenario_no_container_test.go +++ b/test/e2e/scenario_no_container_test.go @@ -21,7 +21,7 @@ import ( ) // TestFunctionRunWithoutContainer tests the func runs on host without container (golang funcs only) -// In other words, it tests `func run --container=false` +// In other words, it tests `func run --builder=host` func TestFunctionRunWithoutContainer(t *testing.T) { var funcName = "func-no-container" @@ -62,7 +62,7 @@ func TestFunctionRunWithoutContainer(t *testing.T) { } // Run without container (scaffolding) - knFuncTerm1.Exec("run", "--container=false", "--verbose", "--path", funcPath, "--registry", common.GetRegistry()) + knFuncTerm1.Exec("run", "--builder=host", "--verbose", "--path", funcPath, "--registry", common.GetRegistry()) }() knFuncRunCompleted := false