From 4ca5896cf586d75340291bc8800c6d4fbfccec9c Mon Sep 17 00:00:00 2001 From: David Fridrich Date: Tue, 12 Aug 2025 07:00:42 +0200 Subject: [PATCH] Remove container flag - variant II --- cmd/build.go | 2 +- cmd/deploy.go | 32 ++--- cmd/deploy_test.go | 56 ++++---- cmd/root.go | 34 +++++ cmd/run.go | 124 ++++++------------ cmd/run_test.go | 90 ++----------- docs/reference/func_config_git_set.md | 2 +- docs/reference/func_deploy.md | 2 +- docs/reference/func_run.md | 44 +++---- pkg/config/config.go | 12 +- pkg/config/config_test.go | 11 +- test/e2e/scenario_no_container_test.go | 4 +- .../scenario_remote-repository_test.go | 2 +- 13 files changed, 160 insertions(+), 255 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index d64a71a43c..a4c516d4ca 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -98,7 +98,7 @@ EXAMPLES // Globally-Configurable Flags: // Options whose value may be defined globally may also exist on the // contextually relevant function; sets are flattened above via cfg.Apply(f) - cmd.Flags().StringP("builder", "b", cfg.Builder, + cmd.Flags().StringP("builder", "b", defaultBuilder(f), fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. ($FUNC_BUILDER)", KnownBuilders())) cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") diff --git a/cmd/deploy.go b/cmd/deploy.go index fb35e44a19..f535138834 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -6,6 +6,7 @@ import ( "io" "os" "os/exec" + "slices" "strconv" "strings" @@ -155,8 +156,8 @@ EXAMPLES // Globally-Configurable Flags: // Options whose value may be defined globally may also exist on the // contextually relevant function; but sets are flattened via cfg.Apply(f) - cmd.Flags().StringP("builder", "b", cfg.Builder, - fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders())) + cmd.Flags().StringP("builder", "b", defaultBuilder(f), + fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. Defaults to 'host' for python/go, otherwise '%s'", KnownBuilders(), builders.Default)) cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)") @@ -328,18 +329,14 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if err != nil { return } - // image is valid and undigested - if !digested { - f.Deploy.Image = cfg.Image - } + // image is valid (no error); just assign here, both digested and + // undigested images are valid + f.Deploy.Image = cfg.Image } - // If user provided --image with digest, they are requesting that specific - // image to be used which means building phase should be skipped and image - // should be deployed as is - if digested { - f.Deploy.Image = cfg.Image - } else { + // if user provided an undigested image or didnt provide one at all, use + // build route. + if !digested { // NOT digested, build & push the Function unless specified otherwise if f, justBuilt, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return @@ -430,10 +427,8 @@ func NewRegistryValidator(path string) survey.Validator { // ValidateBuilder ensures that the given builder is one that the CLI // knows how to instantiate, returning a builkder.ErrUnknownBuilder otherwise. func ValidateBuilder(name string) (err error) { - for _, known := range KnownBuilders() { - if name == known { - return - } + if slices.Contains(KnownBuilders(), name) { + return } return builders.ErrUnknownBuilder{Name: name, Known: KnownBuilders()} } @@ -729,6 +724,11 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { return errors.New("git settings (--git-url --git-dir and --git-branch) are only applicable when triggering remote deployments (--remote)") } + // dont try to build remotely with the host builder + if c.Remote && c.Builder == "host" { + return errors.New("cannot deploy remotely using the host builder. Please choose different --builder") + } + // Git URL can contain at maximum one '#' urlParts := strings.Split(c.GitURL, "#") if len(urlParts) > 2 { diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index acd804a213..b1f9be8377 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -17,6 +17,7 @@ import ( fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/mock" + "knative.dev/func/pkg/oci" . "knative.dev/func/pkg/testing" ) @@ -230,29 +231,10 @@ func testConfigPrecedence(cmdFn commandConstructor, t *testing.T) { clientFn = NewTestClient(fn.WithBuilder(builder)) ) - // Ensure static default applied via 'builder' - // (a degenerate case, mostly just ensuring config values are not wiped to a - // zero value struct, etc) + // Ensure Global Config applied via config in ./testdata root := FromTempDirectory(t) t.Setenv("XDG_CONFIG_HOME", home) // sets registry.example.com/global f := fn.Function{Runtime: "go", Root: root, Name: "f"} - if f, err = fn.New().Init(f); err != nil { - t.Fatal(err) - } - if err := cmdFn(clientFn).Execute(); err != nil { - t.Fatal(err) - } - if f, err = fn.NewFunction(root); err != nil { - t.Fatal(err) - } - if f.Build.Builder != builders.Default { - t.Fatalf("expected static default builder '%v', got '%v'", builders.Default, f.Build.Builder) - } - - // Ensure Global Config applied via config in ./testdata - root = FromTempDirectory(t) - t.Setenv("XDG_CONFIG_HOME", home) // sets registry.example.com/global - f = fn.Function{Runtime: "go", Root: root, Name: "f"} f, err = fn.New().Init(f) if err != nil { t.Fatal(err) @@ -469,17 +451,24 @@ func testFunctionContext(cmdFn commandConstructor, t *testing.T) { // Build the function explicitly setting the builder to !builders.Default cmd := cmdFn(NewTestClient()) - dflt := cmd.Flags().Lookup("builder").DefValue - // The initial default value should be builders.Default (see global config) - if dflt != builders.Default { - t.Fatalf("expected flag default value '%v', got '%v'", builders.Default, dflt) + dflt := defaultBuilder(f) + // The initial default value should be host for oci-enabled languages or builders.Default + // Includes midstream check so that S2I is not overwriten. + expectedDefault := "" + if oci.IsSupported(f.Runtime) && builders.Default != builders.S2I { + expectedDefault = builders.Host + } else { + expectedDefault = builders.Default + } + if dflt != expectedDefault { + t.Fatalf("expected default value '%v', got '%v'", expectedDefault, dflt) } // Choose the value that is not the default // We must calculate this because downstream changes the default via patches. var builder string - if builders.Default == builders.Pack { + if expectedDefault == builders.Pack { builder = builders.S2I } else { builder = builders.Pack @@ -503,7 +492,7 @@ func testFunctionContext(cmdFn commandConstructor, t *testing.T) { // The command default should now take into account the function when // determining the flag default cmd = cmdFn(NewTestClient()) - dflt = cmd.Flags().Lookup("builder").DefValue + dflt = defaultBuilder(f) if dflt != builder { t.Fatalf("expected flag default to be function's current builder '%v', got '%v'", builder, dflt) @@ -532,7 +521,7 @@ func TestDeploy_GitArgsPersist(t *testing.T) { fn.WithPipelinesProvider(mock.NewPipelinesProvider()), fn.WithRegistry(TestRegistry), )) - cmd.SetArgs([]string{"--remote", "--git-url=" + url, "--git-branch=" + branch, "--git-dir=" + dir, "."}) + cmd.SetArgs([]string{"--builder=pack", "--remote", "--git-url=" + url, "--git-branch=" + branch, "--git-dir=" + dir, "."}) if err := cmd.Execute(); err != nil { t.Fatal(err) } @@ -564,7 +553,8 @@ func TestDeploy_GitArgsUsed(t *testing.T) { dir = "function" ) // Create a new Function in the temp dir - _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) + f := fn.Function{Runtime: "go", Root: root, Build: fn.BuildSpec{Builder: "pack"}} + _, err := fn.New().Init(f) if err != nil { t.Fatal(err) } @@ -601,8 +591,8 @@ func TestDeploy_GitArgsUsed(t *testing.T) { // in the URL is equivalent to providing --git-branch func TestDeploy_GitURLBranch(t *testing.T) { root := FromTempDirectory(t) - - f, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) + ff := fn.Function{Runtime: "go", Root: root, Build: fn.BuildSpec{Builder: "pack"}} + f, err := fn.New().Init(ff) if err != nil { t.Fatal(err) } @@ -1288,6 +1278,7 @@ func TestDeploy_BasicRedeployPipelinesCorrectNamespace(t *testing.T) { Runtime: "go", Root: root, Registry: TestRegistry, + Build: fn.BuildSpec{Builder: "pack"}, } f, err := fn.New().Init(f) if err != nil { @@ -1592,7 +1583,8 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { root := FromTempDirectory(t) // Create a new Function in the temp directory - if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { + f := fn.Function{Runtime: "go", Root: root, Build: fn.BuildSpec{Builder: "pack"}} + if _, err := fn.New().Init(f); err != nil { t.Fatal(err) } @@ -1774,7 +1766,7 @@ func TestDeploy_UnsetFlag(t *testing.T) { root := FromTempDirectory(t) // Create a function - f := fn.Function{Runtime: "go", Root: root, Registry: TestRegistry} + f := fn.Function{Runtime: "go", Root: root, Registry: TestRegistry, Build: fn.BuildSpec{Builder: "pack"}} f, err := fn.New().Init(f) if err != nil { t.Fatal(err) diff --git a/cmd/root.go b/cmd/root.go index 5bc6487f39..7b67bcae73 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -16,9 +16,11 @@ import ( "knative.dev/client/pkg/util" "knative.dev/func/cmd/templates" + "knative.dev/func/pkg/builders" "knative.dev/func/pkg/config" fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" + "knative.dev/func/pkg/oci" ) // DefaultVersion when building source directly (bypassing the Makefile) @@ -143,6 +145,38 @@ func registry() string { return cfg.RegistryDefault() } +// determines the default builder as 'flag default' +// takes into account function context (if already built) +// otherwise, will use dynamic defaulting - use 'host' for specific runtimes +func defaultBuilder(f fn.Function) string { + // using function context + if f.Build.Builder != "" { + return f.Build.Builder + } + + // global config file + cfg, err := config.NewDefault() + if err != nil { + fmt.Fprintf(os.Stderr, "Error loading config file at %v, using defaults", config.File()) + } + if cfg.Builder != "" { + return cfg.Builder + } + + // midstream should not set host builder as default + if builders.Default == builders.S2I { + return builders.S2I + } + + // if oci enabled runtime + if oci.IsSupported(f.Runtime) { + return builders.Host + } + + // static default + return builders.Default +} + // effectivePath to use is that which was provided by --path or FUNC_PATH. // Manually parses flags such that this can be used during (cobra/viper) flag // definition (prior to parsing). diff --git a/cmd/run.go b/cmd/run.go index bcc5b3528f..7f89100ce4 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,38 +38,31 @@ 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 (default when + available), 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. + o Run the function locally using the runtime's default container. $ {{rootCmdUse}} run - o Run the function locally from within its container, forcing a rebuild + o Run the function locally, forcing a rebuild of the container even if no filesysem changes are detected $ {{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 +72,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) @@ -101,8 +94,8 @@ EXAMPLES // Flags // // Globally-Configurable Flags: - cmd.Flags().StringP("builder", "b", cfg.Builder, - fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders())) + cmd.Flags().StringP("builder", "b", defaultBuilder(f), + fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. Defaults to 'host' for python/go, otherwise 'pack'.", KnownBuilders())) cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") @@ -121,8 +114,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,16 +148,13 @@ 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 f fn.Function ) cfg = newRunConfig(cmd) // Will add Prompt on upcoming UX refactor + fmt.Printf("> cfg.Builder %v\n", cfg.Builder) if f, err = fn.NewFunction(cfg.Path); err != nil { return @@ -174,27 +162,16 @@ 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" - } - - // Validate after configure and auto-fix if err = cfg.Validate(cmd, f); err != nil { return } + if f, err = cfg.Configure(f); err != nil { // Updates f with build&run cfg + return + } + fmt.Printf("> post-configure %v\n", f.Build.Builder) + // non-containerized build is only via "host" builder + 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 { @@ -218,11 +195,10 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // Build // - // If requesting to run via the container, build the container if it is + // If using non-host (containerized) builder - 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() if err != nil { return err @@ -234,23 +210,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 - } - } - - 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) @@ -258,9 +228,10 @@ 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)") } } + // assign image here too? no? } // Run @@ -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"), @@ -369,7 +335,7 @@ func newRunConfig(cmd *cobra.Command) (c runConfig) { } // Configure the given function. Updates a function struct with all -// configurable values. Note that the config alrady includes function's +// configurable values. Note that the config already includes function's // current state, as they were passed through via flag defaults. func (c runConfig) Configure(f fn.Function) (fn.Function, error) { var err error @@ -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, path; are not part of function // state, so are not mentioned here in Configure. return f, err } @@ -412,18 +378,14 @@ func (c runConfig) Validate(cmd *cobra.Command, f fn.Function) (err error) { } } - if !c.Container && !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) + // check for oci support on non-containerized build + if c.Builder == "host" && !oci.IsSupported(f.Runtime) { + return fmt.Errorf("the %q runtime currently requires being run in a container, please use a different builder", f.Runtime) } // When the docker runner respects the StartTimeout, this validation check // can be removed - if c.StartTimeout != 0 && c.Container { + if c.StartTimeout != 0 && c.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 10f5b1c1aa..85da3efcf7 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -134,9 +134,8 @@ func TestRun_Run(t *testing.T) { fn.WithRegistry("ghcr.com/reg"), )) cmd.SetArgs(tt.args) // Do not use test command args - // set test case's function instance - f, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + f, err := fn.New().Init(fn.Function{Root: root, Runtime: "node"}) if err != nil { t.Fatal(err) } @@ -210,11 +209,11 @@ func TestRun_Images(t *testing.T) { buildInvoked: false, }, { - name: "digested image without container should fail", - args: []string{"--container=false", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + name: "digested image without container (host builder) should fail", + args: []string{"--builder=host", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, runInvoked: false, buildInvoked: false, - buildError: fmt.Errorf("cannot use digested image with --container=false"), + buildError: fmt.Errorf("cannot use digested image with non-containerized builds"), }, { name: "image should build even with tagged image given", @@ -249,7 +248,8 @@ func TestRun_Images(t *testing.T) { cmd.SetArgs(tt.args) // Do not use test command args // set test case's function instance - _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + f := fn.Function{Root: root, Runtime: "go", Build: fn.BuildSpec{Builder: "pack"}} + _, err := fn.New().Init(f) if err != nil { t.Fatal(err) } @@ -315,14 +315,14 @@ func TestRun_CorrectImage(t *testing.T) { buildInvoked: false, }, { - name: "digested image without container should fail", - args: []string{"--container=false", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + name: "digested image without container (with host builder) should fail", + args: []string{"--builder=host", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, image: "", buildInvoked: false, expectError: true, }, { - name: "image should build even with tagged image given", + name: "image should build with tagged image given", args: []string{"--image", "username/exampleimage:latest"}, image: "username/exampleimage:latest", buildInvoked: true, @@ -357,7 +357,8 @@ func TestRun_CorrectImage(t *testing.T) { cmd.SetArgs(tt.args) // set test case's function instance - _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + f := fn.Function{Root: root, Runtime: "go", Build: fn.BuildSpec{Builder: "pack"}} + _, err := fn.New().Init(f) if err != nil { t.Fatal(err) } @@ -417,7 +418,7 @@ func TestRun_DirectOverride(t *testing.T) { // SETUP THE ENVIRONMENT & SITUATION // create function - _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go", Build: fn.BuildSpec{Builder: "pack"}}) if err != nil { t.Fatal(err) } @@ -639,70 +640,3 @@ func TestCtrBuilder(t *testing.T) { 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_config_git_set.md b/docs/reference/func_config_git_set.md index 6e480265b8..25896ed337 100644 --- a/docs/reference/func_config_git_set.md +++ b/docs/reference/func_config_git_set.md @@ -17,7 +17,7 @@ func config git set ### Options ``` - -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". (default "pack") + -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". --builder-image string Specify a custom builder image for use by the builder other than its default. ($FUNC_BUILDER_IMAGE) --config-cluster Configure cluster resources (credentials and config on the cluster). --config-local Configure local resources (pipeline templates). diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index 1de14a1059..a6d57728c9 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -116,7 +116,7 @@ func deploy --base-image string Override the base image for your function (host builder only) --build string[="true"] Build the function. [auto|true|false]. ($FUNC_BUILD) (default "auto") --build-timestamp Use the actual time as the created time for the docker image. This is only useful for buildpacks builder. - -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". (default "pack") + -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". Defaults to 'host' for python/go, otherwise 'pack' (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) --domain string Domain to use for the function's route. Cluster must be configured with domain matching for the given domain (ignored if unrecognized) ($FUNC_DOMAIN) diff --git a/docs/reference/func_run.md b/docs/reference/func_run.md index 2f9a7cdb37..35705a96c8 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,38 +19,31 @@ 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 (default when + available), 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. + o Run the function locally using the runtime's default container. $ func run - o Run the function locally from within its container, forcing a rebuild + o Run the function locally, forcing a rebuild of the container even if no filesysem changes are detected $ 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' @@ -69,10 +62,9 @@ func run --address string Interface and port on which to bind and listen. Default is 127.0.0.1:8080, or an available port if 8080 is not available. ($FUNC_ADDRESS) --base-image string Override the base image for your function (host builder only) --build string[="true"] Build the function. [auto|true|false]. ($FUNC_BUILD) (default "auto") - -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". (default "pack") + -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". Defaults to 'host' for python/go, otherwise 'pack'. (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/pkg/config/config.go b/pkg/config/config.go index f5f5312f6a..1f888bb54c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -10,7 +10,6 @@ import ( "strings" "gopkg.in/yaml.v2" - "knative.dev/func/pkg/builders" fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) @@ -25,8 +24,10 @@ const ( // DefaultLanguage is intentionaly undefined. DefaultLanguage = "" + // NOTE: Builder is now dynamic. Can reverse when + // host builder becomes the standard instead of pack. // DefaultBuilder is statically defined by the builders package. - DefaultBuilder = builders.Default + //DefaultBuilder = builders.Default ) // Global configuration settings. @@ -48,7 +49,8 @@ type Global struct { // for one which further takes into account the optional config file. func New() Global { return Global{ - Builder: DefaultBuilder, + // NOTE: see note above at 'DefaultBuilder' for why this is commented. + //Builder: DefaultBuilder, Language: DefaultLanguage, // ... } @@ -123,9 +125,7 @@ func (c Global) Apply(f fn.Function) Global { // yes a bit tedious, manually mapping each member (if defined) is simple, // easy to understand and support; with both mapping direction (Apply and // Configure) in one central place here... with tests. - if f.Build.Builder != "" { - c.Builder = f.Build.Builder - } + if f.Runtime != "" { c.Language = f.Runtime } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 8a91f707e4..1d2d7d1465 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -183,9 +183,6 @@ func TestApply(t *testing.T) { // member on the function (example: confirm), and not all members of a // function are globally configurable (example: image). f := fn.Function{ - Build: fn.BuildSpec{ - Builder: "builder", - }, Deploy: fn.DeploySpec{ Namespace: "namespace", }, @@ -194,9 +191,6 @@ func TestApply(t *testing.T) { } cfg := config.Global{}.Apply(f) - if cfg.Builder != "builder" { - t.Error("apply missing map of f.Build.Builder") - } if cfg.Language != "runtime" { t.Error("apply missing map of f.Runtime ") } @@ -215,9 +209,6 @@ func TestApply(t *testing.T) { // empty values in the function context should not zero out // populated values in the global config when applying. cfg.Apply(fn.Function{}) - if cfg.Builder == "" { - t.Error("empty f.Build.Builder should not be mapped") - } if cfg.Language == "" { t.Error("empty f.Runtime should not be mapped") } @@ -226,7 +217,7 @@ func TestApply(t *testing.T) { } } -// TestConfigyre ensures that configuring a function results in every member +// TestConfigure ensures that configuring a function results in every member // of the function in the intersection of the two sets, global config and function // members, to be set to the values of the config. // (See the associated cfg.Apply) diff --git a/test/e2e/scenario_no_container_test.go b/test/e2e/scenario_no_container_test.go index 5f7e76e121..9f8cd23a2d 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 diff --git a/test/oncluster/scenario_remote-repository_test.go b/test/oncluster/scenario_remote-repository_test.go index fdd6a432da..b0b98900a8 100644 --- a/test/oncluster/scenario_remote-repository_test.go +++ b/test/oncluster/scenario_remote-repository_test.go @@ -87,7 +87,7 @@ func TestRemoteRepository(t *testing.T) { knFunc.SourceDir = funcPath - knFunc.Exec("deploy", "--registry", common.GetRegistry(), "--remote") + knFunc.Exec("deploy", "--registry", common.GetRegistry(), "--builder=pack", "--remote") defer knFunc.Exec("delete") result := knFunc.Exec("invoke", "-p", funcPath)