Skip to content

Commit

Permalink
feat: enable host builder via cli (#1748)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lkingland
lkingland committed Jun 20, 2023
1 parent 94582ef commit 51cb15b
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 107 deletions.
104 changes: 68 additions & 36 deletions cmd/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"knative.dev/func/pkg/builders/s2i"
"knative.dev/func/pkg/config"
fn "knative.dev/func/pkg/functions"
"knative.dev/func/pkg/oci"
)

func NewBuildCmd(newClient ClientFactory) *cobra.Command {
Expand Down Expand Up @@ -156,8 +157,9 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro

// TODO: this logic is duplicated with runDeploy. Shouild be in buildConfig
// constructor.
// Checks if there is a difference between defined registry and its value used as a prefix in the image tag
// In case of a mismatch a new image tag is created and used for build
// Checks if there is a difference between defined registry and its value
// used as a prefix in the image tag In case of a mismatch a new image tag is
// created and used for build.
// Do not react if image tag has been changed outside configuration
if f.Registry != "" && !cmd.Flags().Changed("image") && strings.Index(f.Image, "/") > 0 && !strings.HasPrefix(f.Image, f.Registry) {
prfx := f.Registry
Expand All @@ -171,23 +173,14 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
}

// Client
// Concrete implementations (ex builder) vary based on final effective config
o := []fn.Option{fn.WithRegistry(cfg.Registry)}
if f.Build.Builder == builders.Pack {
o = append(o, fn.WithBuilder(pack.NewBuilder(
pack.WithName(builders.Pack),
pack.WithTimestamp(cfg.WithTimestamp),
pack.WithVerbose(cfg.Verbose))))
} else if f.Build.Builder == builders.S2I {
o = append(o, fn.WithBuilder(s2i.NewBuilder(
s2i.WithName(builders.S2I),
s2i.WithVerbose(cfg.Verbose))))
clientOptions, err := cfg.clientOptions()
if err != nil {
return
}

client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, o...)
client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...)
defer done()

// Build and (optionally) push
// Build
buildOptions, err := cfg.buildOptions()
if err != nil {
return
Expand Down Expand Up @@ -236,26 +229,6 @@ type buildConfig struct {
WithTimestamp bool
}

func (c buildConfig) buildOptions() (oo []fn.BuildOption, err error) {
oo = []fn.BuildOption{}

// Platforms
//
// TODO: upgrade --platform to a multi-value field. The individual builder
// implementations are responsible for bubbling an error if they do
// not support this. Pack supports none, S2I supports one, host builder
// supports multi.
if c.Platform != "" {
parts := strings.Split(c.Platform, "/")
if len(parts) != 2 {
return oo, fmt.Errorf("the value for --patform must be in the form [OS]/[Architecture]. eg \"linux/amd64\"")
}
oo = append(oo, fn.BuildWithPlatforms([]fn.Platform{{OS: parts[0], Architecture: parts[1]}}))
}

return
}

// newBuildConfig gathers options into a single build request.
func newBuildConfig() buildConfig {
return buildConfig{
Expand Down Expand Up @@ -369,3 +342,62 @@ func (c buildConfig) Validate() (err error) {

return
}

// clientOptions returns options suitable for instantiating a client based on
// the current state of the build config object.
// This will be unnecessary and refactored away when the host-based OCI
// builder and pusher are the default implementations and the Pack and S2I
// constructors simplified.
//
// TODO: Platform is currently only used by the S2I builder. This should be
// a multi-valued argument which passes through to the "host" builder (which
// supports multi-arch/platform images), and throw an error if either trying
// to specify a platform for buildpacks, or trying to specify more than one
// for S2I.
//
// TODO: As a further optimization, it might be ideal to only build the
// image necessary for the target cluster, since the end product of a function
// deployment is not the contiainer, but rather the running service.
func (c buildConfig) clientOptions() ([]fn.Option, error) {
o := []fn.Option{fn.WithRegistry(c.Registry)}
if c.Builder == builders.Host {
o = append(o,
fn.WithBuilder(oci.NewBuilder(builders.Host, c.Verbose)),
fn.WithPusher(oci.NewPusher(false, c.Verbose)))
} else if c.Builder == builders.Pack {
o = append(o,
fn.WithBuilder(pack.NewBuilder(
pack.WithName(builders.Pack),
pack.WithTimestamp(c.WithTimestamp),
pack.WithVerbose(c.Verbose))))
} else if c.Builder == builders.S2I {
o = append(o,
fn.WithBuilder(s2i.NewBuilder(
s2i.WithName(builders.S2I),
s2i.WithVerbose(c.Verbose))))
} else {
return o, builders.ErrUnknownBuilder{Name: c.Builder, Known: KnownBuilders()}
}
return o, nil
}

// buildOptions returns options for use with the client.Build request
func (c buildConfig) buildOptions() (oo []fn.BuildOption, err error) {
oo = []fn.BuildOption{}

// Platforms
//
// TODO: upgrade --platform to a multi-value field. The individual builder
// implementations are responsible for bubbling an error if they do
// not support this. Pack supports none, S2I supports one, host builder
// supports multi.
if c.Platform != "" {
parts := strings.Split(c.Platform, "/")
if len(parts) != 2 {
return oo, fmt.Errorf("the value for --patform must be in the form [OS]/[Architecture]. eg \"linux/amd64\"")
}
oo = append(oo, fn.BuildWithPlatforms([]fn.Platform{{OS: parts[0], Architecture: parts[1]}}))
}

return
}
86 changes: 47 additions & 39 deletions cmd/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"io"
"os"
"strconv"
"strings"

Expand All @@ -13,8 +14,6 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
"knative.dev/client-pkg/pkg/util"
"knative.dev/func/pkg/builders"
"knative.dev/func/pkg/builders/buildpacks"
"knative.dev/func/pkg/builders/s2i"
"knative.dev/func/pkg/config"
fn "knative.dev/func/pkg/functions"
"knative.dev/func/pkg/k8s"
Expand Down Expand Up @@ -251,26 +250,11 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
// Informative non-error messages regarding the final deployment request
printDeployMessages(cmd.OutOrStdout(), cfg)

// Client
// Concrete implementations (ex builder) vary based on final effective cfg.
var builder fn.Builder
if f.Build.Builder == builders.Pack {
builder = buildpacks.NewBuilder(
buildpacks.WithName(builders.Pack),
buildpacks.WithVerbose(cfg.Verbose),
buildpacks.WithTimestamp(cfg.Timestamp),
)
} else if f.Build.Builder == builders.S2I {
builder = s2i.NewBuilder(
s2i.WithName(builders.S2I),
s2i.WithVerbose(cfg.Verbose))
} else {
return builders.ErrUnknownBuilder{Name: f.Build.Builder, Known: KnownBuilders()}
clientOptions, err := cfg.clientOptions()
if err != nil {
return
}

client, done := newClient(ClientConfig{Namespace: f.Deploy.Namespace, Verbose: cfg.Verbose},
fn.WithRegistry(cfg.Registry),
fn.WithBuilder(builder))
client, done := newClient(ClientConfig{Namespace: f.Deploy.Namespace, Verbose: cfg.Verbose}, clientOptions...)
defer done()

// Deploy
Expand All @@ -281,15 +265,12 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
return
}
} else {
if shouldBuild(cfg.Build, f, client) { // --build or "auto" with FS changes
buildOptions, err := cfg.buildOptions()
if err != nil {
return err
}

if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil {
return err
}
var buildOptions []fn.BuildOption
if buildOptions, err = cfg.buildOptions(); err != nil {
return
}
if f, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil {
return
}
if cfg.Push {
if f, err = client.Push(cmd.Context(), f); err != nil {
Expand All @@ -313,16 +294,30 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
return f.Stamp()
}

// shouldBuild returns true if the value of the build option is a truthy value,
// or if it is the literal "auto" and the function reports as being currently
// unbuilt. Invalid errors are not reported as this is the purview of
// deployConfig.Validate
func shouldBuild(buildCfg string, f fn.Function, client *fn.Client) bool {
if buildCfg == "auto" {
return !f.Built() // first build or modified filesystem
// build when flag == 'auto' and the function is out-of-date, or when the
// flag value is explicitly truthy such as 'true' or '1'. Error if flag
// is neither 'auto' nor parseable as a boolean. Return CLI-specific error
// message verbeage suitable for both Deploy and Run commands which feature an
// optional build step.
func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, buildOptions []fn.BuildOption) (fn.Function, error) {
var err error
if flag == "auto" {
if f.Built() {
fmt.Fprintln(cmd.OutOrStdout(), "function up-to-date. Force rebuild with --build")
} else {
if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil {
return f, err
}
}
} else if build, _ := strconv.ParseBool(flag); build {
if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil {
return f, err
}
} else if _, err = strconv.ParseBool(flag); err != nil {
return f, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized. Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag)

}
build, _ := strconv.ParseBool(buildCfg)
return build
return f, nil
}

func NewRegistryValidator(path string) survey.Validator {
Expand Down Expand Up @@ -368,6 +363,19 @@ func KnownBuilders() builders.Known {
// the set of builders enumerated in the builders pacakage.
// However, future third-party integrations may support less than, or more
// builders, and certain environmental considerations may alter this list.

// Also a good place to stick feature-flags; to wit:
enable_host, _ := strconv.ParseBool(os.Getenv("FUNC_ENABLE_HOST_BUILDER"))
if !enable_host {
bb := []string{}
for _, b := range builders.All() {
if b != builders.Host {
bb = append(bb, b)
}
}
return bb
}

return builders.All()
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1516,9 +1516,9 @@ func TestDeploy_UnsetFlag(t *testing.T) {
}

// Test_ValidateBuilder tests that the bulder validation accepts the
// accepts === the set of known builders.
// the set of known builders, and spot-checks an error is thrown for unknown.
func Test_ValidateBuilder(t *testing.T) {
for _, name := range builders.All() {
for _, name := range KnownBuilders() {
if err := ValidateBuilder(name); err != nil {
t.Fatalf("expected builder '%v' to be valid, but got error: %v", name, err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/invoke_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func TestInvoke(t *testing.T) {
fmt.Fprintf(os.Stderr, "error serving: %v", err)
}
}()
_, port, _ := net.SplitHostPort(l.Addr().String())
host, port, _ := net.SplitHostPort(l.Addr().String())
errs := make(chan error, 10)
stop := func() error { _ = s.Close(); return nil }
return fn.NewJob(f, "127.0.0.1", port, errs, stop, false)
return fn.NewJob(f, host, port, errs, stop, false)
}

// Run the mock http service function interloper
Expand Down
42 changes: 15 additions & 27 deletions cmd/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ import (
"github.com/ory/viper"
"github.com/spf13/cobra"

"knative.dev/func/pkg/builders"
pack "knative.dev/func/pkg/builders/buildpacks"
"knative.dev/func/pkg/builders/s2i"
"knative.dev/func/pkg/config"
"knative.dev/func/pkg/docker"
fn "knative.dev/func/pkg/functions"
Expand Down Expand Up @@ -148,10 +145,10 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
if cfg, err = newRunConfig(cmd).Prompt(); err != nil {
return
}
if err = cfg.Validate(cmd); err != nil {
if f, err = fn.NewFunction(cfg.Path); err != nil {
return
}
if f, err = fn.NewFunction(cfg.Path); err != nil {
if err = cfg.Validate(cmd, f); err != nil {
return
}
if !f.Initialized() {
Expand All @@ -176,39 +173,30 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
}

// Client
//
// Builder and runner implementations are based on the value of f.Build.Builder, and
//
o := []fn.Option{}
if f.Build.Builder == builders.Pack {
o = append(o, fn.WithBuilder(pack.NewBuilder(
pack.WithName(builders.Pack),
pack.WithVerbose(cfg.Verbose))))
} else if f.Build.Builder == builders.S2I {
o = append(o, fn.WithBuilder(s2i.NewBuilder(
s2i.WithName(builders.S2I),
s2i.WithVerbose(cfg.Verbose))))
clientOptions, err := cfg.clientOptions()
if err != nil {
return
}
if cfg.Container {
o = append(o, fn.WithRunner(docker.NewRunner(cfg.Verbose, os.Stdout, os.Stderr)))
clientOptions = append(clientOptions, fn.WithRunner(docker.NewRunner(cfg.Verbose, os.Stdout, os.Stderr)))
}
if cfg.StartTimeout != 0 {
o = append(o, fn.WithStartTimeout(cfg.StartTimeout))
clientOptions = append(clientOptions, fn.WithStartTimeout(cfg.StartTimeout))
}

client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, o...)
client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...)
defer done()

// Build
//
// 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 && shouldBuild(cfg.Build, f, client) {
if cfg.Container {
buildOptions, err := cfg.buildOptions()
if err != nil {
return err
}
if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil {
if f, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil {
return err
}
}
Expand Down Expand Up @@ -321,7 +309,7 @@ func (c runConfig) Prompt() (runConfig, error) {
return c, nil
}

func (c runConfig) Validate(cmd *cobra.Command) (err error) {
func (c runConfig) Validate(cmd *cobra.Command, f fn.Function) (err error) {
// Bubble
if err = c.buildConfig.Validate(); err != nil {
return
Expand All @@ -335,13 +323,13 @@ func (c runConfig) Validate(cmd *cobra.Command) (err error) {
}

// There is currently no local host runner implemented, so specifying
// --container=false should always return an informative error to the user
// such that they do not receive the rather cryptic "no runner defined"
// error from a Client instance which was instantiated with no runner.
// --container=false should return an informative error for runtimes other
// than Go that is more helpful than the cryptic, though correct, error
// from the Client that it was instantated without a runner.
// TODO: modify this check when the local host runner is available to
// only generate this error when --container==false && the --language is
// not yet implemented.
if !c.Container {
if !c.Container && f.Runtime != "go" {
return errors.New("the ability to run functions outside of a container via 'func run' is coming soon.")
}

Expand Down
Loading

0 comments on commit 51cb15b

Please sign in to comment.