Skip to content
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.

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

feat: deploy command global config with function context #1434

Merged
merged 18 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
79 changes: 53 additions & 26 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) {

// Functions must be built (have an associated image) before being deployed.
// Note that externally built images may be specified in the func.yaml
if !f.HasImage() {
if !Built(f.Root) {
return ErrNotBuilt
}

Expand All @@ -735,14 +735,29 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) {
// Deploy a new or Update the previously-deployed function
c.progressListener.Increment("⬆️ Deploying function to the cluster")
result, err := c.deployer.Deploy(ctx, f)
if err != nil {
fmt.Printf("deploy error: %v\n", err)
return
}

// Update the function with the namespace into which the function was
// deployed
f.Deploy.Namespace = result.Namespace

// saves namespace to function's metadata (func.yaml)
if err = f.Write(); err != nil {
return
}

if result.Status == Deployed {
c.progressListener.Increment(fmt.Sprintf("✅ Function deployed in namespace %q and exposed at URL: \n %v", result.Namespace, result.URL))
} else if result.Status == Updated {
c.progressListener.Increment(fmt.Sprintf("✅ Function updated in namespace %q and exposed at URL: \n %v", result.Namespace, result.URL))
}

return err
// Metadata generated from deploying (namespace) should not trigger a rebuild
// through a staleness check, so update the build stamp we checked earlier.
return updateBuildStamp(f)
}

// RunPipeline runs a Pipeline to build and deploy the function.
Expand Down Expand Up @@ -956,29 +971,40 @@ func (c *Client) Push(ctx context.Context, path string) (err error) {
return
}

if !f.HasImage() {
if !Built(f.Root) {
return ErrNotBuilt
}

imageDigest, err := c.pusher.Push(ctx, f)
if err != nil {
if f.ImageDigest, err = c.pusher.Push(ctx, f); err != nil {
return
}

// Record the Image Digest pushed.
f.ImageDigest = imageDigest
return f.Write()
if err = f.Write(); err != nil { // saves digest to f's metadata (func.yaml)
return
}

// Metadata generated from pushing (ImageDigest) should not trigger a rebuild
// through a staleness check, so update the build stamp we checked earlier.
return updateBuildStamp(f)
}

// Built returns true if the given path contains a function which has been
// built without any filesystem modifications since (is not stale).
func (c *Client) Built(path string) bool {
func Built(path string) bool {
f, err := NewFunction(path)
if err != nil {
return false
}

// Missing a build image always means !Built (but does not satisfy staleness
// If there is no build stamp, it is not built.
// This case should be redundant with the below check for an image, but is
// temporarily necessary (see the long-winded caviat note below).
stamp := buildStamp(path)
if stamp == "" {
return false
}

// Missing an image name always means !Built (but does not satisfy staleness
// checks).
// NOTE: This will be updated in the future such that a build does not
// automatically update the function's serialized, source-controlled state,
Expand All @@ -989,16 +1015,7 @@ func (c *Client) Built(path string) bool {
// An example of how this bug manifests is that every rebuild of a function
// registers the func.yaml as being dirty for source-control purposes, when
// this should only happen on deploy.
if !f.HasImage() {
return false
}

buildstampPath := filepath.Join(path, RunDataDir, buildstamp)

// If there is no build stamp, it is also not built.
// This case should be redundant with the above check for an image, but is
// temporarily necessary (see the long-winded caviat note above).
if _, err := os.Stat(buildstampPath); err != nil {
if f.Image == "" {
return false
}

Expand All @@ -1008,14 +1025,10 @@ func (c *Client) Built(path string) bool {
fmt.Fprintf(os.Stderr, "error calculating function's fingerprint: %v\n", err)
return false
}
b, err := os.ReadFile(buildstampPath)
if err != nil {
fmt.Fprintf(os.Stderr, "error reading function's fingerprint: %v\n", err)

if stamp != hash {
return false
}
if string(b) != hash {
return false // changes detected
}

// Function has a populated image, existing buildstamp, and the calculated
// fingerprint has not changed.
Expand All @@ -1025,6 +1038,20 @@ func (c *Client) Built(path string) bool {
return true
}

// buildStamp returns the current (last) build stamp for the function
// at the given path, if it can be found.
func buildStamp(path string) string {
buildstampPath := filepath.Join(path, RunDataDir, buildstamp)
if _, err := os.Stat(buildstampPath); err != nil {
return ""
}
b, err := os.ReadFile(buildstampPath)
if err != nil {
return ""
}
return string(b)
}

// fingerprint returns a hash of the filenames and modification timestamps of
// the files within a function's root.
func fingerprint(f Function) (string, error) {
Expand Down
1 change: 0 additions & 1 deletion client_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
// ## Cluster Cleanup
//
// The test cluster and most resources can be removed with:

// ./hack/delete.sh
//
// NOTE: Downloaded images are not removed.
Expand Down
18 changes: 9 additions & 9 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1530,23 +1530,23 @@ func TestClient_BuiltStamps(t *testing.T) {
client := fn.New(fn.WithBuilder(builder), fn.WithRegistry(TestRegistry))

// paths that do not contain a function are !Built - Degenerate case
if client.Built(root) {
if fn.Built(root) {
t.Fatal("path not containing a function returned as being built")
}

// a freshly-created function should be !Built
if err := client.Init(fn.Function{Runtime: TestRuntime, Root: root}); err != nil {
t.Fatal(err)
}
if client.Built(root) {
if fn.Built(root) {
t.Fatal("newly created function returned Built==true")
}

// a function which was successfully built should return as being Built
if err := client.Build(context.Background(), root); err != nil {
t.Fatal(err)
}
if !client.Built(root) {
if !fn.Built(root) {
t.Fatal("freshly built function should return Built==true")
}
}
Expand Down Expand Up @@ -1597,7 +1597,7 @@ func TestClient_BuiltDetects(t *testing.T) {
}

// Prior to a filesystem edit, it will be Built.
if !client.Built(root) {
if !fn.Built(root) {
t.Fatal("freshly built function reported Built==false (1)")
}

Expand All @@ -1612,15 +1612,15 @@ func TestClient_BuiltDetects(t *testing.T) {
// Release thread and wait to ensure that the clock advances even in constrained CI environments
time.Sleep(100 * time.Millisecond)

if client.Built(root) {
if fn.Built(root) {
t.Fatal("client did not detect file timestamp change as indicating build staleness")
}

// Build and double-check Built has been reset
if err := client.Build(ctx, root); err != nil {
t.Fatal(err)
}
if !client.Built(root) {
if !fn.Built(root) {
t.Fatal("freshly built function reported Built==false (2)")
}

Expand All @@ -1632,15 +1632,15 @@ func TestClient_BuiltDetects(t *testing.T) {
f.Close()

// The system should now detect the function is stale
if client.Built(root) {
if fn.Built(root) {
t.Fatal("client did not detect an added file as indicating build staleness")
}

// Build and double-check Built has been reset
if err := client.Build(ctx, root); err != nil {
t.Fatal(err)
}
if !client.Built(root) {
if !fn.Built(root) {
t.Fatal("freshly built function reported Built==false (3)")
}

Expand All @@ -1649,7 +1649,7 @@ func TestClient_BuiltDetects(t *testing.T) {
if err := os.Remove(filepath.Join(root, testfile)); err != nil {
t.Fatal(err)
}
if client.Built(root) {
if fn.Built(root) {
t.Fatal("client did not detect a removed file as indicating build staleness")
}
}
25 changes: 13 additions & 12 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ EXAMPLES
// 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,
fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", KnownBuilders()))
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. (Env: $FUNC_BUILDER)", KnownBuilders()))
cmd.Flags().BoolP("confirm", "c", cfg.Confirm,
"Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
cmd.Flags().StringP("registry", "r", cfg.Registry,
"Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined (Env: $FUNC_REGISTRY)")
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. (Env: $FUNC_REGISTRY)")

// Function-Context Flags:
// Options whose value is available on the function with context only
Expand Down Expand Up @@ -130,25 +130,26 @@ EXAMPLES
}

func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) {
if err = config.CreatePaths(); err != nil {
return // see docker/creds potential mutation of auth.json
var (
cfg buildConfig
f fn.Function
)
if err = config.CreatePaths(); err != nil { // for possible auth.json usage
return
}

cfg, err := newBuildConfig().Prompt()
if err != nil {
if cfg, err = newBuildConfig().Prompt(); err != nil {
return
}

if err = cfg.Validate(); err != nil {
return
}

f, err := fn.NewFunction(cfg.Path)
if err != nil {
if f, err = fn.NewFunction(cfg.Path); err != nil {
return
}
f = cfg.Configure(f) // Updates f at path to include build request values

// 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
// Do not react if image tag has been changed outside configuration
Expand Down Expand Up @@ -249,7 +250,7 @@ func newBuildConfig() buildConfig {

// Configure the given function. Updates a function struct with all
// configurable values. Note that buildConfig already includes function's
// current values, as they were passed through vi flag defaults, so overwriting
// current values, as they were passed through via flag defaults, so overwriting
// is a noop.
func (c buildConfig) Configure(f fn.Function) fn.Function {
f = c.Global.Configure(f)
Expand Down
Loading