Skip to content

Cleanup: Remove container flag (var I) #2987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
knative-prow merged 1 commit into from
Aug 20, 2025
Merged

Cleanup: Remove container flag (var I) #2987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 35 additions & 74 deletions cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -38,38 +38,32 @@ 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

o Run the function locally from within its container.
$ {{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=<builder>

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'
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -174,28 +162,17 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) {
if !f.Initialized() {
return fn.NewErrNotInitialized(f.Root)
}
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"
if err = cfg.Validate(cmd, f); err != nil {
return
}

// Validate after configure and auto-fix
if err = cfg.Validate(cmd, f); err != nil {
if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
return
}

container := f.Build.Builder != "host"

// Ignore the verbose flag if JSON output
if cfg.JSON {
cfg.Verbose = false
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -234,31 +211,25 @@ 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
}
}

if digested {
// run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go
// it doesnt get saved, just runtime image
// image was parsed and both digested AND undigested imgs are valid
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)
if err != nil {
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)")
}
}
}
Expand Down Expand Up @@ -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

Expand All @@ -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"),
Expand All @@ -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
}
Expand Down Expand Up @@ -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")
}

Expand Down
105 changes: 0 additions & 105 deletions cmd/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"context"
"fmt"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -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 <nil>")
}
}

// 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)
}
}
Loading
Loading