Remove leaking log from deploy

This commit removing lekaing logs from deploy API in SDK. It also returns typed errors from API resoponse and allows as to check for some custom errors which functions like `IsNotFound` or `IsUnauthorized`. This also updates DeployFuntion API to return a typed response and `http.Response`. Signed-off-by: Vivek Singh <[email protected]>
openfaas · Apr 30, 2021 · a87570f · a87570f
1 parent 927cc77
commit a87570f
Show file tree

Hide file tree

Showing 4 changed files with 318 additions and 70 deletions.
diff --git a/commands/deploy.go b/commands/deploy.go
@@ -276,9 +276,15 @@ Error: %s`, fprocessErr.Error())
 			if msg := checkTLSInsecure(services.Provider.GatewayURL, deploySpec.TLSInsecure); len(msg) > 0 {
 				fmt.Println(msg)
 			}
-			statusCode := proxyClient.DeployFunction(ctx, deploySpec)
-			if badStatusCode(statusCode) {
-				failedStatusCodes[k] = statusCode
+			dRes, res, err := proxyClient.DeployFunction(ctx, deploySpec)
+
+			if err == nil {
+				fmt.Println(dRes.Message)
+				fmt.Printf("URL: %s\n\n", dRes.URL)
+			}
+
+			if badStatusCode(res.StatusCode) {
+				failedStatusCodes[k] = res.StatusCode
 			}
 		}
 	} else {
@@ -375,9 +381,14 @@ func deployImage(
 		fmt.Println(msg)
 	}
 
-	statusCode = client.DeployFunction(ctx, deploySpec)
+	dRes, res, err := client.DeployFunction(ctx, deploySpec)
+	if err != nil {
+		return res.StatusCode, err
+	}
 
-	return statusCode, nil
+	fmt.Println(dRes.Message)
+	fmt.Printf("URL: %s\n\n", dRes.URL)
+	return res.StatusCode, nil
 }
 
 func mergeSlice(values []string, overlay []string) []string {

diff --git a/proxy/deploy.go b/proxy/deploy.go
@@ -8,7 +8,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"time"
 
@@ -57,29 +56,48 @@ func generateFuncStr(spec *DeployFunctionSpec) string {
 	return spec.FunctionName
 }
 
+type DeployResponse struct {
+	Message       string
+	RollingUpdate bool
+	URL           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) (*DeployResponse, *http.Response, error) {
 
 	rollingUpdateInfo := fmt.Sprintf("Function %s already exists, attempting rolling-update.", spec.FunctionName)
-	statusCode, deployOutput := c.deploy(context, spec, spec.Update)
+	res, err := c.deploy(context, spec, spec.Update)
 
-	if spec.Update == true && statusCode == http.StatusNotFound {
+	if err != nil && IsUnknown(err) {
+		return nil, res, err
+	}
+
+	var deployResponse DeployResponse
+	if err == nil {
+		deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status)
+		deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
+	}
+
+	if spec.Update == true && IsNotFound(err) {
 		// Re-run the function with update=false
+		res, err = c.deploy(context, spec, false)
+		if err == nil {
+			deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status)
+			deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
+		}
+
+	} else if res.StatusCode == http.StatusOK {
+		deployResponse.Message += rollingUpdateInfo
+		deployResponse.RollingUpdate = true
 
-		statusCode, deployOutput = c.deploy(context, spec, false)
-	} else if statusCode == http.StatusOK {
-		fmt.Println(rollingUpdateInfo)
 	}
-	fmt.Println()
-	fmt.Println(deployOutput)
-	return statusCode
+
+	return &deployResponse, res, err
 }
 
 // deploy a function to an OpenFaaS gateway over REST
-func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (int, string) {
-
-	var deployOutput string
+func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (*http.Response, error) {
 	// Need to alter Gateway to allow nil/empty string as fprocess, to avoid this repetition.
 	var fprocessTemplate string
 	if len(spec.FProcess) > 0 {
@@ -146,37 +164,21 @@ func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, updat
 	request, err = c.newRequest(method, "/system/functions", reader)
 
 	if err != nil {
-		deployOutput += fmt.Sprintln(err)
-		return http.StatusInternalServerError, deployOutput
+		return nil, err
 	}
 
 	res, err := c.doRequest(context, request)
 
 	if err != nil {
-		deployOutput += fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?")
-		deployOutput += fmt.Sprintln(err)
-		return http.StatusInternalServerError, deployOutput
+		errMessage := fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?")
+		errMessage += fmt.Sprintln(err)
+		return res, NewUnknown(errMessage, 0)
 	}
 
-	if res.Body != nil {
-		defer res.Body.Close()
-	}
-
-	switch res.StatusCode {
-	case http.StatusOK, http.StatusCreated, http.StatusAccepted:
-		deployOutput += fmt.Sprintf("Deployed. %s.\n", res.Status)
-
-		deployedURL := fmt.Sprintf("URL: %s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
-		deployOutput += fmt.Sprintln(deployedURL)
-	case http.StatusUnauthorized:
-		deployOutput += fmt.Sprintln("unauthorized access, run \"faas-cli login\" to setup authentication for this server")
-
-	default:
-		bytesOut, err := ioutil.ReadAll(res.Body)
-		if err == nil {
-			deployOutput += fmt.Sprintf("Unexpected status: %d, message: %s\n", res.StatusCode, string(bytesOut))
-		}
+	err = checkForAPIError(res)
+	if err != nil {
+		return res, err
 	}
 
-	return res.StatusCode, deployOutput
+	return res, nil
 }
diff --git a/proxy/deploy_test.go b/proxy/deploy_test.go
@@ -21,7 +21,8 @@ type deployProxyTest struct {
 	mockServerResponses []int
 	replace             bool
 	update              bool
-	expectedOutput      string
+	expectedMessage     string
+	statusCode          int
 }
 
 func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) {
@@ -34,32 +35,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,
-			"",
-			"",
-		})
+	dRes, httpRes, _ := 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,
+		"",
+		"",
 	})
 
-	r := regexp.MustCompile(deployTest.expectedOutput)
-	if !r.MatchString(stdout) {
-		t.Fatalf("Output not matched: %s", stdout)
+	if httpRes.StatusCode != deployTest.statusCode {
+		t.Fatalf("StatuCode did not match. expected: %d, got: %d", deployTest.statusCode, httpRes.StatusCode)
+	}
+
+	r := regexp.MustCompile(deployTest.expectedMessage)
+	if !r.MatchString(dRes.Message) {
+		t.Fatalf("Output not matched: %s", dRes.Message)
 	}
 }
 
@@ -70,21 +73,24 @@ func Test_RunDeployProxyTests(t *testing.T) {
 			mockServerResponses: []int{http.StatusOK, http.StatusOK},
 			replace:             true,
 			update:              false,
-			expectedOutput:      `(?m:Deployed)`,
+			statusCode:          http.StatusOK,
+			expectedMessage:     `(?m:Deployed)`,
 		},
 		{
 			title:               "404_Deploy",
 			mockServerResponses: []int{http.StatusOK, http.StatusNotFound},
 			replace:             true,
 			update:              false,
-			expectedOutput:      `(?m:Unexpected status: 404)`,
+			statusCode:          http.StatusNotFound,
+			expectedMessage:     "",
 		},
 		{
 			title:               "UpdateFailedDeployed",
 			mockServerResponses: []int{http.StatusNotFound, http.StatusOK},
 			replace:             false,
 			update:              true,
-			expectedOutput:      `(?m:Deployed)`,
+			statusCode:          http.StatusOK,
+			expectedMessage:     `(?m:Deployed)`,
 		},
 	}
 	for _, tst := range deployProxyTests {