fix: on cluster build - populate image name if not set (#1213)

Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>

Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>

client.go

@@ -728,6 +728,7 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) {
 }
 
 // RunPipeline runs a Pipeline to Build and deploy the function at path.
+// In a parameter accepts git configuration options that we don't want to persist into func.yaml.
 func (c *Client) RunPipeline(ctx context.Context, path string, git Git) (err error) {
 	go func() {
 		<-ctx.Done()
@@ -739,6 +740,29 @@ func (c *Client) RunPipeline(ctx context.Context, path string, git Git) (err err
 		err = fmt.Errorf("failed to laod function: %w", err)
 		return
 	}
+
+	// Default function registry to the client's global registry
+	if f.Registry == "" {
+		f.Registry = c.registry
+	}
+
+	// If no image name has been yet defined (not yet built/deployed), calculate.
+	// Image name is stored on the function for later use by deploy, etc.
+	// TODO: write this to .func/build instead, and populate f.Image on deploy
+	// such that local builds do not dirty the work tree.
+	if f.Image == "" {
+		if f.Image, err = f.ImageName(); err != nil {
+			return
+		}
+		//Write (save) - Serialize the function to disk
+		//Will now contain populated image name.
+		if err = f.Write(); err != nil {
+			return
+		}
+	}
+
+	// Git configuration options specified as arguments don't get saved to func.yaml,
+	// but are used in the pipeline invocation
 	f.Git = git
 
 	// Build and deploy function using Pipeline
@@ -880,7 +904,9 @@ func (c *Client) Remove(ctx context.Context, cfg Function, deleteAll bool) error
 // instance is preferred, with the remote function triggered if there is no
 // locally running instance.
 // Example:
-//  myClient.Invoke(myContext, myFunction, "local", NewInvokeMessage())
+//
+//	myClient.Invoke(myContext, myFunction, "local", NewInvokeMessage())
+//
 // The message sent to the function is defined by the invoke message.
 // See NewInvokeMessage for its defaults.
 // Functions are invoked in a manner consistent with the settings defined in

client_test.go

@@ -348,8 +348,10 @@ func TestClient_New_Named(t *testing.T) {
 // Registry is the namespace at the container image registry.
 // If not prepended with the registry, it will be defaulted:
 // Examples:  "docker.io/alice"
-//            "quay.io/bob"
-//            "charlie" (becomes [DefaultRegistry]/charlie
+//
+//	"quay.io/bob"
+//	"charlie" (becomes [DefaultRegistry]/charlie
+//
 // At this time a registry namespace is required as we rely on a third-party
 // registry in all cases.  When we support in-cluster container registries,
 // this configuration parameter will become optional.
@@ -976,7 +978,7 @@ func TestClient_List_OutsideRoot(t *testing.T) {
 	}
 }
 
-// TestClient_Deploy_Image ensures that initilally the function's image
+// TestClient_Deploy_Image ensures that initially the function's image
 // member has no value (not initially deployed); the value is populated
 // upon deployment with a value derived from the function's name and currently
 // effective client registry; that the value of f.Image will take precidence
@@ -1053,6 +1055,82 @@ func TestClient_Deploy_Image(t *testing.T) {
 	}
 }
 
+// TestClient_Pipelines_Deploy_Image ensures that initially the function's image
+// member has no value (not initially deployed); the value is populated
+// upon pipeline run execution with a value derived from the function's name and currently
+// effective client registry; that the value of f.Image will take precidence
+// over .Registry, which is used to calculate a default value for image.
+func TestClient_Pipelines_Deploy_Image(t *testing.T) {
+	root, rm := Mktemp(t)
+	defer rm()
+
+	client := fn.New(
+		fn.WithPipelinesProvider(mock.NewPipelinesProvider()),
+		fn.WithRegistry("example.com/alice"))
+
+	repoUrl := "http://example-git.com/alice/myfunc.gi"
+	git := fn.Git{
+		URL: &repoUrl,
+	}
+
+	err := client.Create(fn.Function{Name: "myfunc", Runtime: "node", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Upon initial creation, the value of .Image is empty
+	if f.Image != "" {
+		t.Fatalf("new function should have no image, got '%v'", f.Image)
+	}
+
+	// Upon pipeline run, the function should be populated;
+	if err = client.RunPipeline(context.Background(), root, git); err != nil {
+		t.Fatal(err)
+	}
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected := "example.com/alice/myfunc:latest"
+	if f.Image != expected {
+		t.Fatalf("expected image '%v', got '%v'", expected, f.Image)
+	}
+	expected = "example.com/alice"
+	if f.Registry != "example.com/alice" {
+		t.Fatalf("expected registry '%v', got '%v'", expected, f.Registry)
+	}
+
+	// The value of .Image always takes precidence
+	f.Image = "registry2.example.com/bob/myfunc:latest"
+	if err = f.Write(); err != nil {
+		t.Fatal(err)
+	}
+	// Upon pipeline run, the function should be populated;
+	if err = client.RunPipeline(context.Background(), root, git); err != nil {
+		t.Fatal(err)
+	}
+	expected = "registry2.example.com/bob/myfunc:latest"
+	if f.Image != expected {
+		t.Fatalf("expected image '%v', got '%v'", expected, f.Image)
+	}
+	expected = "example.com/alice"
+	if f.Registry != "example.com/alice" {
+		// Note that according to current logic, the function's defined registry
+		// may be inaccurate.  Consider an initial deploy to registryA, followed by
+		// an explicit mutaiton of the function's .Image member.
+		// This could either remain as a documented nuance:
+		//   'The value of f.Registry is only used in the event an image name
+		//    need be derived (f.Image =="")
+		// Or we could update .Registry to always be in sync by parsing the .Image
+		t.Fatalf("expected registry '%v', got '%v'", expected, f.Registry)
+	}
+}
+
 // TestClient_Deploy_UnbuiltErrors ensures that a call to deploy a function which was not
 // fully created (ie. was only initialized, not actually built and deploys)
 // yields an expected, and informative, error.

pipelines/tekton/pipeplines_provider.go

@@ -121,7 +121,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) error {
 
 	registry, err := docker.GetRegistry(f.Image)
 	if err != nil {
-		return err
+		return fmt.Errorf("problem in resolving image registry name: %v", err)
 	}
 
 	pp.progressListener.Stopping()