registry update change on deploy, some prints

Signed-off-by: gauron99 <[email protected]>

cmd/build.go

@@ -176,7 +176,6 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 			return
 		}
 	}
-	fmt.Printf("WRITING IN RUNBUILD: %s\n", f.Build.Image)
 	if err = f.Write(); err != nil {
 		return
 	}

cmd/deploy.go

@@ -252,7 +252,6 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 
 	// Informative non-error messages regarding the final deployment request
 	printDeployMessages(cmd.OutOrStdout(), f)
-	// print stuff here
 
 	clientOptions, err := cfg.clientOptions()
 	if err != nil {
@@ -325,38 +324,19 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 // 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
-
-	// TODO: gauron99 - This checks whether a registry has changed on a subsequent build
-	// decision (Like on func deploy --registry=X when already built image is Y)
-	// This can probably be added into f.Built() function for simpler and more global
-	// implementation
-	registryChangedSubsequently := false
-	if !strings.Contains(f.Build.Image, f.Registry) && f.Image == "" && f.Build.Image != "" {
-		registryChangedSubsequently = true
-	}
-
-	build, _ := strconv.ParseBool(flag)
+	fmt.Printf("build func: flag:'%s'; fBI: '%s'; fR: '%s'\n", flag, f.Build.Image, f.Registry)
 	if flag == "auto" {
-		if f.Built() && !registryChangedSubsequently {
+		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 {
+	} else if build, _ := strconv.ParseBool(flag); build {
 		if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil {
 			return f, err
 		}
-	} else if !build && registryChangedSubsequently {
-		// This else-if-branch solves edge case problem.
-		// This happens when --registry is given with deploy when a different registry
-		// has been used to build image previously.
-		// Ex.:
-		// step 1) Init -> Build with: --registry example.com/alice
-		// step 2) Deploy with: --registry example.com/fred --build=false
-		fmt.Fprintf(cmd.OutOrStdout(), "Error: --build flag was disabled but you most likely provided new registry '%v'. Enable build (--build=true) to override or build beforehand with your new registry to apply it\n", f.Registry)
-		return f, fmt.Errorf("new registry '%v' was provided but build was disabled, cannot update image name", f.Registry)
 	} 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)
 

cmd/deploy_test.go

@@ -1550,10 +1550,9 @@ func Test_ValidateBuilder(t *testing.T) {
 	}
 }
 
-// TestReDeploy_ErrorOnRegistryChangeWithoutBuild tests that subsequent deploy
-// with different --registry and --build disabled throws an error because client
-// wasnt able to build on a registry change
-func TestReDeploy_ErrorOnRegistryChangeWithoutBuild(t *testing.T) {
+// TestReDeploy_OnRegistryChange tests that after deployed image with registry X,
+// subsequent deploy with registry Y triggers build
+func TestReDeploy_OnRegistryChange(t *testing.T) {
 	root := fromTempDirectory(t)
 
 	// Create a basic go Function
@@ -1575,40 +1574,75 @@ func TestReDeploy_ErrorOnRegistryChangeWithoutBuild(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// change namespace and deploy again
+	// change registry and deploy again
 	newRegistry := "example.com/fred"
 
-	// Redeploy the function without specifying namespace.
 	cmd := NewDeployCmd(NewTestClient(
 		fn.WithDeployer(mock.NewDeployer()),
 	))
 
-	cmd.SetArgs([]string{"--registry=" + newRegistry, "--build=false"})
+	cmd.SetArgs([]string{"--registry=" + newRegistry})
 
-	stdout := strings.Builder{}
-	cmd.SetOut(&stdout)
+	// Second: Deploy with different registry and expect new build
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
 
-	// Second: Deploy with different registry and build disabled (check err here)
-	err = cmd.Execute()
+	// ASSERT
+	expectF, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal("couldnt load function from path")
+	}
 
-	expectedPrint := fmt.Sprintf("Error: --build flag was disabled but you most likely provided new registry '%v'. Enable build (--build=true) to override or build beforehand with your new registry to apply it\n", newRegistry)
-	expectedError := fmt.Sprintf("new registry '%v' was provided but build was disabled, cannot update image name", newRegistry)
+	if !strings.Contains(expectF.Build.Image, newRegistry) {
+		t.Fatalf("expected built image '%s' to contain new registry '%s'\n", expectF.Build.Image, newRegistry)
+	}
+}
+func TestReDeploy_OnRegistryChangeWithBuildFalse(t *testing.T) {
+	root := fromTempDirectory(t)
 
-	// ASSERT
+	// Create a basic go Function
+	f := fn.Function{
+		Runtime: "go",
+		Root:    root,
+	}
+	_, err := fn.New().Init(f)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	// Ensure output contained error message
-	if !strings.Contains(stdout.String(), expectedPrint) {
-		t.Log("STDOUT:\n" + stdout.String())
-		t.Fatalf("Expected message not found:\n%v", expectedPrint)
+	// create build cmd
+	cmdBuild := NewBuildCmd(NewTestClient(fn.WithBuilder(mock.NewBuilder())))
+	cmdBuild.SetArgs([]string{"--registry=" + TestRegistry})
+
+	// First: prebuild Function
+	if err := cmdBuild.Execute(); err != nil {
+		t.Fatal(err)
 	}
 
-	// ensure we get the right error
-	if err == nil {
-		t.Fatal("expected error but got nil")
+	// change registry and deploy again
+	newRegistry := "example.com/fred"
+
+	cmd := NewDeployCmd(NewTestClient(
+		fn.WithDeployer(mock.NewDeployer()),
+	))
+
+	cmd.SetArgs([]string{"--registry=" + newRegistry, "--build=false"})
+
+	// Second: Deploy with different registry and expect 'not built' error because
+	// registry has changed but build is disabled
+	if err := cmd.Execute(); err == nil {
+		t.Fatal(err)
+	}
+
+	// ASSERT
+	expectF, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal("couldnt load function from path")
 	}
 
-	if err.Error() != expectedError {
-		t.Fatalf("expected error message '%v' but got '%v'", expectedError, err.Error())
+	if !strings.Contains(expectF.Build.Image, TestRegistry) {
+		t.Fatal("expected registry to NOT change since --build=false")
 	}
 }
 

pkg/functions/function.go

@@ -669,7 +669,23 @@ func (f Function) Built() bool {
 		fmt.Fprintf(os.Stderr, "error calculating function's fingerprint: %v\n", err)
 		return false
 	}
-	return stamp == hash
+
+	if stamp != hash {
+		return false
+	}
+
+	// Special case of registry change on a subsequent deploy attempt should
+	// result in unbuilt image, forcing a rebuild if possible
+	// Example: Deploy with image using registry X. Then subsequently deploy with
+	// --registry=Y, changing registry resulting in unmatched Registry and Build.Image.
+
+	// If f.Image is specified, registry is overridden -- meaning its not taken into
+	// consideration and can be different from actually built image.
+	if !strings.Contains(f.Build.Image, f.Registry) && f.Image == "" {
+		fmt.Fprintf(os.Stderr, "Warning: registry '%s' does not match currently built image '%s' and no direct image override was provided via --image\n", f.Registry, f.Build.Image)
+		return false
+	}
+	return true
 }
 
 // BuildStamp accesses the current (last) build stamp for the function.