Remove leaking logs from SDK

This commit removes leaking logs from the SDK (proxy package) of the CLI by removing custom print statements within proxy package. It updates method signature for DeployFunction which now returns deployOutput text along with status code. Fixes: openfaas#853 Signed-off-by: Vivek Singh <[email protected]>
viveksyngh · Feb 23, 2021 · bb4bc30 · bb4bc30
1 parent 6b5e7a1
commit bb4bc30
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 52 deletions.
diff --git a/commands/deploy.go b/commands/deploy.go
@@ -276,7 +276,9 @@ Error: %s`, fprocessErr.Error())
 			if msg := checkTLSInsecure(services.Provider.GatewayURL, deploySpec.TLSInsecure); len(msg) > 0 {
 				fmt.Println(msg)
 			}
-			statusCode := proxyClient.DeployFunction(ctx, deploySpec)
+			statusCode, deployOutputStr := proxyClient.DeployFunction(ctx, deploySpec)
+			fmt.Println()
+			fmt.Println(deployOutputStr)
 			if badStatusCode(statusCode) {
 				failedStatusCodes[k] = statusCode
 			}
@@ -375,8 +377,9 @@ func deployImage(
 		fmt.Println(msg)
 	}
 
-	statusCode = client.DeployFunction(ctx, deploySpec)
-
+	statusCode, deployOutputStr := client.DeployFunction(ctx, deploySpec)
+	fmt.Println()
+	fmt.Println(deployOutputStr)
 	return statusCode, nil
 }
 

diff --git a/proxy/delete.go b/proxy/delete.go
@@ -30,14 +30,12 @@ func (c *Client) DeleteFunction(ctx context.Context, functionName string, namesp
 
 	req, err := c.newRequest(http.MethodDelete, deleteEndpoint, reader)
 	if err != nil {
-		fmt.Println(err)
 		return err
 	}
 	delRes, delErr := c.doRequest(ctx, req)
 
 	if delErr != nil {
-		fmt.Printf("Error removing existing function: %s, gateway=%s, functionName=%s\n", delErr.Error(), c.GatewayURL.String(), functionName)
-		return delErr
+		return fmt.Errorf("error removing existing function: %s, gateway=%s, functionName=%s", delErr.Error(), c.GatewayURL.String(), functionName)
 	}
 
 	if delRes.Body != nil {
@@ -46,7 +44,6 @@ func (c *Client) DeleteFunction(ctx context.Context, functionName string, namesp
 
 	switch delRes.StatusCode {
 	case http.StatusOK, http.StatusCreated, http.StatusAccepted:
-		fmt.Println("Removing old function.")
 	case http.StatusNotFound:
 		err = fmt.Errorf("No existing function to remove")
 	case http.StatusUnauthorized:

diff --git a/proxy/delete_test.go b/proxy/delete_test.go
@@ -21,13 +21,10 @@ func Test_DeleteFunction(t *testing.T) {
 	cliAuth := NewTestAuth(nil)
 	proxyClient, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout)
 
-	stdout := test.CaptureStdout(func() {
-		proxyClient.DeleteFunction(context.Background(), "function-to-delete", "")
-	})
+	err := proxyClient.DeleteFunction(context.Background(), "function-to-delete", "")
 
-	r := regexp.MustCompile(`(?m:Removing old function.)`)
-	if !r.MatchString(stdout) {
-		t.Fatalf("Want: %s, got: %s", "Removing old function", stdout)
+	if err != nil {
+		t.Fatalf("Got error: %s", err.Error())
 	}
 }
 

diff --git a/proxy/deploy.go b/proxy/deploy.go
@@ -59,21 +59,15 @@ func generateFuncStr(spec *DeployFunctionSpec) string {
 
 // DeployFunction first tries to deploy a function and if it exists will then attempt
 // a rolling update. Warnings are suppressed for the second API call (if required.)
-func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) int {
+func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) (int, string) {
 
-	rollingUpdateInfo := fmt.Sprintf("Function %s already exists, attempting rolling-update.", spec.FunctionName)
 	statusCode, deployOutput := c.deploy(context, spec, spec.Update)
 
 	if spec.Update == true && statusCode == http.StatusNotFound {
 		// Re-run the function with update=false
-
 		statusCode, deployOutput = c.deploy(context, spec, false)
-	} else if statusCode == http.StatusOK {
-		fmt.Println(rollingUpdateInfo)
 	}
-	fmt.Println()
-	fmt.Println(deployOutput)
-	return statusCode
+	return statusCode, deployOutput
 }
 
 // deploy a function to an OpenFaaS gateway over REST

diff --git a/proxy/deploy_test.go b/proxy/deploy_test.go
@@ -6,11 +6,10 @@ package proxy
 import (
 	"context"
 	"net/http"
+	"regexp"
 
 	"testing"
 
-	"regexp"
-
 	"github.com/openfaas/faas-cli/test"
 )
 
@@ -22,6 +21,7 @@ type deployProxyTest struct {
 	replace             bool
 	update              bool
 	expectedOutput      string
+	expectedStatus      int
 }
 
 func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) {
@@ -34,32 +34,34 @@ func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) {
 	cliAuth := NewTestAuth(nil)
 	proxyClient, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout)
 
-	stdout := test.CaptureStdout(func() {
-		proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{
-			"fprocess",
-			"function",
-			"image",
-			"dXNlcjpwYXNzd29yZA==",
-			"language",
-			deployTest.replace,
-			nil,
-			"network",
-			[]string{},
-			deployTest.update,
-			[]string{},
-			map[string]string{},
-			map[string]string{},
-			FunctionResourceRequest{},
-			false,
-			tlsNoVerify,
-			"",
-			"",
-		})
+	statusCode, deployOutputStr := proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{
+		"fprocess",
+		"function",
+		"image",
+		"dXNlcjpwYXNzd29yZA==",
+		"language",
+		deployTest.replace,
+		nil,
+		"network",
+		[]string{},
+		deployTest.update,
+		[]string{},
+		map[string]string{},
+		map[string]string{},
+		FunctionResourceRequest{},
+		false,
+		tlsNoVerify,
+		"",
+		"",
 	})
 
+	if statusCode != deployTest.expectedStatus {
+		t.Fatalf("Got: %d, expected: %d", statusCode, deployTest.expectedStatus)
+	}
+
 	r := regexp.MustCompile(deployTest.expectedOutput)
-	if !r.MatchString(stdout) {
-		t.Fatalf("Output not matched: %s", stdout)
+	if !r.MatchString(deployOutputStr) {
+		t.Fatalf("Output not matched: %s", deployOutputStr)
 	}
 }
 
@@ -71,20 +73,23 @@ func Test_RunDeployProxyTests(t *testing.T) {
 			replace:             true,
 			update:              false,
 			expectedOutput:      `(?m:Deployed)`,
+			expectedStatus:      http.StatusOK,
 		},
 		{
 			title:               "404_Deploy",
 			mockServerResponses: []int{http.StatusOK, http.StatusNotFound},
 			replace:             true,
 			update:              false,
 			expectedOutput:      `(?m:Unexpected status: 404)`,
+			expectedStatus:      http.StatusNotFound,
 		},
 		{
 			title:               "UpdateFailedDeployed",
 			mockServerResponses: []int{http.StatusNotFound, http.StatusOK},
 			replace:             false,
 			update:              true,
 			expectedOutput:      `(?m:Deployed)`,
+			expectedStatus:      http.StatusOK,
 		},
 	}
 	for _, tst := range deployProxyTests {

diff --git a/proxy/invoke.go b/proxy/invoke.go
@@ -5,7 +5,6 @@ package proxy
 
 import (
 	"bytes"
-	"os"
 
 	"fmt"
 	"io/ioutil"
@@ -53,8 +52,6 @@ func InvokeFunction(gateway string, name string, bytesIn *[]byte, contentType st
 
 	req, err := http.NewRequest(httpMethod, gatewayURL, reader)
 	if err != nil {
-		fmt.Println()
-		fmt.Println(err)
 		return nil, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", gateway)
 	}
 
@@ -71,8 +68,6 @@ func InvokeFunction(gateway string, name string, bytesIn *[]byte, contentType st
 	res, err := client.Do(req)
 
 	if err != nil {
-		fmt.Println()
-		fmt.Println(err)
 		return nil, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", gateway)
 	}
 
@@ -82,7 +77,6 @@ func InvokeFunction(gateway string, name string, bytesIn *[]byte, contentType st
 
 	switch res.StatusCode {
 	case http.StatusAccepted:
-		fmt.Fprintf(os.Stderr, "Function submitted asynchronously.\n")
 	case http.StatusOK:
 		var readErr error
 		resBytes, readErr = ioutil.ReadAll(res.Body)