feat: enable scaffolding for host builds (#1750)

* feat: enable scaffolding in builder

* fix a few typos

* error text formatting

Co-authored-by: Lance Ball <lball@redhat.com>

* error text formatting

Co-authored-by: Lance Ball <lball@redhat.com>

* remove test job stop defer

---------

Co-authored-by: Lance Ball <lball@redhat.com>

pkg/functions/client.go

@@ -17,6 +17,8 @@ import (
 	"time"
 
 	"gopkg.in/yaml.v2"
+
+	"knative.dev/func/pkg/scaffolding"
 	"knative.dev/func/pkg/utils"
 )
 
@@ -620,47 +622,11 @@ func (c *Client) Build(ctx context.Context, f Function) (Function, error) {
 // It also updates the included symlink to function source 'f' to point to
 // the current function's source.
 func (c *Client) Scaffold(ctx context.Context, f Function, dest string) (err error) {
-	// First get a reference to the repository containing the scaffolding to use
-	//
-	// TODO: In order to support extensible scaffolding from external repositories,
-	// Retain the repository reference from which a Function was initialized
-	// in order to re-read out its scaffolding later.  This can be the locally-
-	// installed repository name or the remote reference URL.  There are benefits
-	// and detriments either way.  A third option would be to store the
-	// scaffolding locally, but this also has downsides.
-	//
-	//  If function creatd from a local repository named:
-	//     repo = repoFromURL(f.RepoURL)
-	//  If function created from a remote reference:
-	//    c.Repositories().Get(f.RepoName)
-	//  If function not created from an external repository:
-	repo, err := c.Repositories().Get(DefaultRepositoryName)
-	if err != nil {
-		return
-	}
-
-	// Detect the method signature
-	s, err := functionSignature(f)
+	repo, err := NewRepository("", "") // default (embedded) repository
 	if err != nil {
 		return
 	}
-
-	// Write Scaffolding from the Repository into the destination
-	if err = repo.WriteScaffolding(ctx, f, s, dest); err != nil {
-		return
-	}
-
-	// Replace the 'f' link of the scaffolding (which is now incorrect) to
-	// link to the function's root.
-	src, err := filepath.Rel(dest, f.Root)
-	if err != nil {
-		return fmt.Errorf("error determining relative path to function source %w", err)
-	}
-	_ = os.Remove(filepath.Join(dest, "f"))
-	if err = os.Symlink(src, filepath.Join(dest, "f")); err != nil {
-		return fmt.Errorf("error linking scaffolding to function source %w", err)
-	}
-	return
+	return scaffolding.Write(dest, f.Root, f.Runtime, f.Invoke, repo.FS())
 }
 
 // printBuildActivity is a helper for ensuring the user gets feedback from

pkg/functions/client_test.go

@@ -1757,3 +1757,66 @@ func TestClient_CreateMigration(t *testing.T) {
 		t.Fatal("freshly created function should have the latest migration")
 	}
 }
+
+// TestClient_RunReadiness ensures that the run task awaits a ready response
+// from the job before returning.
+func TestClient_RunReadiness(t *testing.T) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	root, cleanup := Mktemp(t)
+	defer cleanup()
+
+	client := fn.New(fn.WithBuilder(oci.NewBuilder("", true)), fn.WithVerbose(true))
+
+	// Initialize
+	f, err := client.Init(fn.Function{Root: root, Runtime: "go", Registry: TestRegistry})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Replace the implementation with the test implementation which will
+	// return a non-200 response for the first few seconds.  This confirms
+	// the client is waiting and retrying.
+	// TODO: we need an init option which skips writing example source-code.
+	_ = os.Remove(filepath.Join(root, "function.go"))
+	_ = os.Remove(filepath.Join(root, "function_test.go"))
+	_ = os.Remove(filepath.Join(root, "handle.go"))
+	_ = os.Remove(filepath.Join(root, "handle_test.go"))
+	src, err := os.Open(filepath.Join(cwd, "testdata", "testClientRunReadiness", "f.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	dst, err := os.Create(filepath.Join(root, "f.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err = io.Copy(dst, src); err != nil {
+		t.Fatal(err)
+	}
+	src.Close()
+	dst.Close()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Build
+	if f, err = client.Build(ctx, f); err != nil {
+		t.Fatal(err)
+	}
+
+	// Run
+	// The function returns a non-200 from its readiness handler at first.
+	// Since we already confirmed in another test that a timeout awaiting a
+	// 200 response from this endpoint does indeed fail the run task, this
+	// delayed 200 confirms there is a retry in place.
+	job, err := client.Run(ctx, f)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := job.Stop(); err != nil {
+		t.Fatalf("err on job stop. %v", err)
+	}
+}

pkg/functions/job.go

@@ -45,6 +45,9 @@ func NewJob(f Function, host, port string, errs chan error, onStop func() error,
 	if j.Errors == nil {
 		j.Errors = make(chan error, 1)
 	}
+	if j.onStop == nil {
+		j.onStop = func() error { return nil }
+	}
 	if err = cleanupJobDirs(j); err != nil {
 		return
 	}

pkg/functions/repository.go

@@ -1,7 +1,6 @@
 package functions
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"net/url"
@@ -156,6 +155,11 @@ func NewRepository(name, uri string) (r Repository, err error) {
 	return
 }
 
+// FS returns the underlying filesystem of this repository.
+func (r Repository) FS() filesystem.Filesystem {
+	return r.fs
+}
+
 // filesystemFromURI returns a filesystem from the data located at the
 // given URI.  If URI is not provided, indicates the embedded repo should
 // be loaded.  URI can be a remote git repository (http:// https:// etc.),
@@ -525,30 +529,6 @@ func (r *Repository) Write(dest string) (err error) {
 	return filesystem.CopyFromFS(".", dest, fs)
 }
 
-// WriteScaffolding code to the given path.
-//
-// Scaffolding is a language-level operation which first detects the method
-// signature used by the function's source code and then writes the
-// appropriate scaffolding.
-//
-// NOTE: Scaffoding is not per-template, because a template is merely an
-// example starting point for a Function implementation and should have no
-// bearing on the shape that function can eventually take.  The language,
-// and optionally invocation hint (For cloudevents) are used for this.  For
-// example, there can be multiple templates which exemplify a given method
-// signature, and the implementation can be switched at any time by the author.
-// Language, by contrast, is fixed at time of initialization.
-func (r *Repository) WriteScaffolding(ctx context.Context, f Function, s Signature, dest string) error {
-	if r.fs == nil {
-		return errors.New("repository has no filesystem")
-	}
-	path := fmt.Sprintf("%v/scaffolding/%v", f.Runtime, s.String()) // fs uses / on all OSs
-	if _, err := r.fs.Stat(path); err != nil {
-		return fmt.Errorf("no scaffolding found for '%v' signature '%v'. %v.", f.Runtime, s, err)
-	}
-	return filesystem.CopyFromFS(path, dest, r.fs)
-}
-
 // URL attempts to read the remote git origin URL of the repository.  Best
 // effort; returns empty string if the repository is not a git repo or the repo
 // has been mutated beyond recognition on disk (ex: removing the origin remote)

pkg/functions/testdata/testClientRunReadiness/f.go

@@ -0,0 +1,30 @@
+package f
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"time"
+)
+
+type F struct {
+	Created time.Time
+}
+
+func New() *F {
+	return &F{time.Now()}
+}
+
+func (f *F) Handle(_ context.Context, w http.ResponseWriter, r *http.Request) {
+	fmt.Println("Request received")
+	fmt.Fprintf(w, "Request received\n")
+}
+
+func (f *F) Ready(ctx context.Context) (bool, error) {
+	// Emulate a function which does not start immediately
+	if time.Now().After(f.Created.Add(600 * time.Millisecond)) {
+		return true, nil
+	}
+	return false, errors.New("still starting up")
+}

pkg/functions/testdata/testClientRunReadiness/go.mod

@@ -0,0 +1,3 @@
+module function
+
+go 1.17