Add comment to explain how method works

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>

cmd/provider.go

@@ -98,7 +98,7 @@ func makeProviderCmd() *cobra.Command {
 			DeployHandler:        handlers.MakeDeployHandler(client, cni, baseUserSecretsPath, alwaysPull),
 			FunctionReader:       handlers.MakeReadHandler(client),
 			ReplicaReader:        handlers.MakeReplicaReaderHandler(client),
-			ReplicaUpdater:       handlers.MakeReplicaUpdateHandler(client, cni),
+			ReplicaUpdater:       handlers.MakeReplicaUpdateHandler(client, cni, invokeResolver),
 			UpdateHandler:        handlers.MakeUpdateHandler(client, cni, baseUserSecretsPath, alwaysPull),
 			HealthHandler:        func(w http.ResponseWriter, r *http.Request) {},
 			InfoHandler:          handlers.MakeInfoHandler(Version, GitCommit),

pkg/provider/handlers/functions.go

@@ -29,6 +29,7 @@ type Function struct {
 	secrets     []string
 	envVars     map[string]string
 	envProcess  string
+	memoryLimit int64
 	createdAt   time.Time
 }
 
@@ -92,7 +93,7 @@ func GetFunction(client *containerd.Client, name string, namespace string) (Func
 
 	spec, err := c.Spec(ctx)
 	if err != nil {
-		return Function{}, fmt.Errorf("unable to load function spec for reading secrets: %s, error %w", name, err)
+		return Function{}, fmt.Errorf("unable to load function %s error: %w", name, err)
 	}
 
 	info, err := c.Info(ctx)
@@ -112,6 +113,7 @@ func GetFunction(client *containerd.Client, name string, namespace string) (Func
 	fn.envVars = envVars
 	fn.envProcess = envProcess
 	fn.createdAt = info.CreatedAt
+	fn.memoryLimit = readMemoryLimitFromSpec(spec)
 
 	replicas := 0
 	task, err := c.Task(ctx, nil)
@@ -231,3 +233,10 @@ func findNamespace(target string, items []string) bool {
 	}
 	return false
 }
+
+func readMemoryLimitFromSpec(spec *specs.Spec) int64 {
+	if spec.Linux == nil || spec.Linux.Resources == nil || spec.Linux.Resources.Memory == nil || spec.Linux.Resources.Memory.Limit == nil {
+		return 0
+	}
+	return *spec.Linux.Resources.Memory.Limit
+}

pkg/provider/handlers/functions_test.go

@@ -32,54 +32,54 @@ func Test_BuildLabelsAndAnnotationsFromServiceSpec_Annotations(t *testing.T) {
 }
 
 func Test_SplitMountToSecrets(t *testing.T) {
-	type test struct {
-		Name     string
-		Input    []specs.Mount
-		Expected []string
+	type testCase struct {
+		Name  string
+		Input []specs.Mount
+		Want  []string
 	}
-	tests := []test{
-		{Name: "No matching openfaas secrets", Input: []specs.Mount{{Destination: "/foo/"}}, Expected: []string{}},
-		{Name: "Nil mounts", Input: nil, Expected: []string{}},
-		{Name: "No Mounts", Input: []specs.Mount{{Destination: "/foo/"}}, Expected: []string{}},
-		{Name: "One Mounts IS secret", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}}, Expected: []string{"secret1"}},
-		{Name: "Multiple Mounts 1 secret", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}, {Destination: "/some/other/path"}}, Expected: []string{"secret1"}},
-		{Name: "Multiple Mounts all secrets", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}, {Destination: "/var/openfaas/secrets/secret2"}}, Expected: []string{"secret1", "secret2"}},
+	tests := []testCase{
+		{Name: "No matching openfaas secrets", Input: []specs.Mount{{Destination: "/foo/"}}, Want: []string{}},
+		{Name: "Nil mounts", Input: nil, Want: []string{}},
+		{Name: "No Mounts", Input: []specs.Mount{{Destination: "/foo/"}}, Want: []string{}},
+		{Name: "One Mounts IS secret", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}}, Want: []string{"secret1"}},
+		{Name: "Multiple Mounts 1 secret", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}, {Destination: "/some/other/path"}}, Want: []string{"secret1"}},
+		{Name: "Multiple Mounts all secrets", Input: []specs.Mount{{Destination: "/var/openfaas/secrets/secret1"}, {Destination: "/var/openfaas/secrets/secret2"}}, Want: []string{"secret1", "secret2"}},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.Name, func(t *testing.T) {
 			got := readSecretsFromMounts(tc.Input)
-			if !reflect.DeepEqual(got, tc.Expected) {
-				t.Fatalf("expected %s, got %s", tc.Expected, got)
+			if !reflect.DeepEqual(got, tc.Want) {
+				t.Fatalf("Want %s, got %s", tc.Want, got)
 			}
 		})
 	}
 }
 
 func Test_ProcessEnvToEnvVars(t *testing.T) {
-	type test struct {
+	type testCase struct {
 		Name     string
 		Input    []string
-		Expected map[string]string
+		Want     map[string]string
 		fprocess string
 	}
-	tests := []test{
-		{Name: "No matching EnvVars", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "fprocess=python index.py"}, Expected: make(map[string]string), fprocess: "python index.py"},
-		{Name: "No EnvVars", Input: []string{}, Expected: make(map[string]string), fprocess: ""},
-		{Name: "One EnvVar", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "fprocess=python index.py", "env=this"}, Expected: map[string]string{"env": "this"}, fprocess: "python index.py"},
-		{Name: "Multiple EnvVars", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "this=that", "env=var", "fprocess=python index.py"}, Expected: map[string]string{"this": "that", "env": "var"}, fprocess: "python index.py"},
-		{Name: "Nil EnvVars", Input: nil, Expected: make(map[string]string)},
+	tests := []testCase{
+		{Name: "No matching EnvVars", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "fprocess=python index.py"}, Want: make(map[string]string), fprocess: "python index.py"},
+		{Name: "No EnvVars", Input: []string{}, Want: make(map[string]string), fprocess: ""},
+		{Name: "One EnvVar", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "fprocess=python index.py", "env=this"}, Want: map[string]string{"env": "this"}, fprocess: "python index.py"},
+		{Name: "Multiple EnvVars", Input: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "this=that", "env=var", "fprocess=python index.py"}, Want: map[string]string{"this": "that", "env": "var"}, fprocess: "python index.py"},
+		{Name: "Nil EnvVars", Input: nil, Want: make(map[string]string)},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.Name, func(t *testing.T) {
 			got, fprocess := readEnvFromProcessEnv(tc.Input)
-			if !reflect.DeepEqual(got, tc.Expected) {
-				t.Fatalf("expected: %s, got: %s", tc.Expected, got)
+			if !reflect.DeepEqual(got, tc.Want) {
+				t.Fatalf("Want: %s, got: %s", tc.Want, got)
 			}
 
 			if fprocess != tc.fprocess {
-				t.Fatalf("expected fprocess: %s, got: %s", tc.fprocess, got)
+				t.Fatalf("Want fprocess: %s, got: %s", tc.fprocess, got)
 
 			}
 		})
@@ -87,22 +87,46 @@ func Test_ProcessEnvToEnvVars(t *testing.T) {
 }
 
 func Test_findNamespace(t *testing.T) {
-	type test struct {
+	type testCase struct {
 		Name            string
 		foundNamespaces []string
 		namespace       string
-		Expected        bool
+		Want            bool
 	}
-	tests := []test{
-		{Name: "Namespace Found", namespace: "fn", foundNamespaces: []string{"fn", "openfaas-fn"}, Expected: true},
-		{Name: "namespace Not Found", namespace: "fn", foundNamespaces: []string{"openfaas-fn"}, Expected: false},
+	tests := []testCase{
+		{Name: "Namespace Found", namespace: "fn", foundNamespaces: []string{"fn", "openfaas-fn"}, Want: true},
+		{Name: "namespace Not Found", namespace: "fn", foundNamespaces: []string{"openfaas-fn"}, Want: false},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.Name, func(t *testing.T) {
 			got := findNamespace(tc.namespace, tc.foundNamespaces)
-			if got != tc.Expected {
-				t.Fatalf("expected %t, got %t", tc.Expected, got)
+			if got != tc.Want {
+				t.Fatalf("Want %t, got %t", tc.Want, got)
+			}
+		})
+	}
+}
+
+func Test_readMemoryLimitFromSpec(t *testing.T) {
+	type testCase struct {
+		Name string
+		Spec *specs.Spec
+		Want int64
+	}
+	testLimit := int64(64)
+	tests := []testCase{
+		{Name: "specs.Linux not found", Spec: &specs.Spec{Linux: nil}, Want: int64(0)},
+		{Name: "specs.LinuxResource not found", Spec: &specs.Spec{Linux: &specs.Linux{Resources: nil}}, Want: int64(0)},
+		{Name: "specs.LinuxMemory not found", Spec: &specs.Spec{Linux: &specs.Linux{Resources: &specs.LinuxResources{Memory: nil}}}, Want: int64(0)},
+		{Name: "specs.LinuxMemory.Limit not found", Spec: &specs.Spec{Linux: &specs.Linux{Resources: &specs.LinuxResources{Memory: &specs.LinuxMemory{Limit: nil}}}}, Want: int64(0)},
+		{Name: "Memory limit set as Want", Spec: &specs.Spec{Linux: &specs.Linux{Resources: &specs.LinuxResources{Memory: &specs.LinuxMemory{Limit: &testLimit}}}}, Want: int64(64)},
+	}
+	for _, tc := range tests {
+		t.Run(tc.Name, func(t *testing.T) {
+			got := readMemoryLimitFromSpec(tc.Spec)
+			if got != tc.Want {
+				t.Fatalf("Want %d, got %d", tc.Want, got)
 			}
 		})
 	}

pkg/provider/handlers/read.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"encoding/json"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"log"
 	"net/http"
 
@@ -37,6 +38,7 @@ func MakeReadHandler(client *containerd.Client) func(w http.ResponseWriter, r *h
 		for _, fn := range fns {
 			annotations := &fn.annotations
 			labels := &fn.labels
+			memory := resource.NewQuantity(fn.memoryLimit, resource.BinarySI)
 			res = append(res, types.FunctionStatus{
 				Name:        fn.name,
 				Image:       fn.image,
@@ -47,6 +49,7 @@ func MakeReadHandler(client *containerd.Client) func(w http.ResponseWriter, r *h
 				Secrets:     fn.secrets,
 				EnvVars:     fn.envVars,
 				EnvProcess:  fn.envProcess,
+				Limits:      &types.FunctionResources{Memory: memory.String()},
 				CreatedAt:   fn.createdAt,
 			})
 		}

pkg/provider/handlers/scale.go

@@ -6,16 +6,20 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
+	"net"
 	"net/http"
+	"net/url"
+	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/namespaces"
 	gocni "github.com/containerd/go-cni"
 
+	"github.com/openfaas/faas-provider/proxy"
 	"github.com/openfaas/faas-provider/types"
 )
 
-func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w http.ResponseWriter, r *http.Request) {
+func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI, resolver proxy.BaseURLResolver) func(w http.ResponseWriter, r *http.Request) {
 
 	return func(w http.ResponseWriter, r *http.Request) {
 
@@ -30,12 +34,9 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 		log.Printf("[Scale] request: %s\n", string(body))
 
 		req := types.ScaleServiceRequest{}
-		err := json.Unmarshal(body, &req)
-
-		if err != nil {
+		if err := json.Unmarshal(body, &req); err != nil {
 			log.Printf("[Scale] error parsing input: %s\n", err)
 			http.Error(w, err.Error(), http.StatusBadRequest)
-
 			return
 		}
 
@@ -55,18 +56,23 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 
 		name := req.ServiceName
 
-		if _, err := GetFunction(client, name, namespace); err != nil {
+		fn, err := GetFunction(client, name, namespace)
+		if err != nil {
 			msg := fmt.Sprintf("service %s not found", name)
 			log.Printf("[Scale] %s\n", msg)
 			http.Error(w, msg, http.StatusNotFound)
 			return
 		}
 
-		ctx := namespaces.WithNamespace(context.Background(), namespace)
+		healthPath := "/_/healthz"
+		if v := fn.annotations["com.openfaas.health.http.path"]; len(v) > 0 {
+			healthPath = v
+		}
 
-		ctr, ctrErr := client.LoadContainer(ctx, name)
-		if ctrErr != nil {
-			msg := fmt.Sprintf("cannot load service %s, error: %s", name, ctrErr)
+		ctx := namespaces.WithNamespace(context.Background(), namespace)
+		ctr, err := client.LoadContainer(ctx, name)
+		if err != nil {
+			msg := fmt.Sprintf("cannot load service %s, error: %s", name, err)
 			log.Printf("[Scale] %s\n", msg)
 			http.Error(w, msg, http.StatusNotFound)
 			return
@@ -75,16 +81,16 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 		var taskExists bool
 		var taskStatus *containerd.Status
 
-		task, taskErr := ctr.Task(ctx, nil)
-		if taskErr != nil {
-			msg := fmt.Sprintf("cannot load task for service %s, error: %s", name, taskErr)
+		task, err := ctr.Task(ctx, nil)
+		if err != nil {
+			msg := fmt.Sprintf("cannot load task for service %s, error: %s", name, err)
 			log.Printf("[Scale] %s\n", msg)
 			taskExists = false
 		} else {
 			taskExists = true
-			status, statusErr := task.Status(ctx)
-			if statusErr != nil {
-				msg := fmt.Sprintf("cannot load task status for %s, error: %s", name, statusErr)
+			status, err := task.Status(ctx)
+			if err != nil {
+				msg := fmt.Sprintf("cannot load task status for %s, error: %s", name, err)
 				log.Printf("[Scale] %s\n", msg)
 				http.Error(w, msg, http.StatusInternalServerError)
 				return
@@ -99,28 +105,31 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 		if req.Replicas == 0 {
 			// If a task is running, pause it
 			if taskExists && taskStatus.Status == containerd.Running {
-				if pauseErr := task.Pause(ctx); pauseErr != nil {
-					wrappedPauseErr := fmt.Errorf("error pausing task %s, error: %s", name, pauseErr)
-					log.Printf("[Scale] %s\n", wrappedPauseErr.Error())
-					http.Error(w, wrappedPauseErr.Error(), http.StatusNotFound)
+				if err := task.Pause(ctx); err != nil {
+					werr := fmt.Errorf("error pausing task %s, error: %s", name, err)
+					log.Printf("[Scale] %s\n", werr.Error())
+					http.Error(w, werr.Error(), http.StatusNotFound)
 					return
 				}
 			}
+
+			// Otherwise, no action is required
+			return
 		}
 
 		if taskExists {
 			if taskStatus != nil {
 				if taskStatus.Status == containerd.Paused {
-					if resumeErr := task.Resume(ctx); resumeErr != nil {
-						log.Printf("[Scale] error resuming task %s, error: %s\n", name, resumeErr)
-						http.Error(w, resumeErr.Error(), http.StatusBadRequest)
+					if err := task.Resume(ctx); err != nil {
+						log.Printf("[Scale] error resuming task %s, error: %s\n", name, err)
+						http.Error(w, err.Error(), http.StatusBadRequest)
 						return
 					}
 				} else if taskStatus.Status == containerd.Stopped {
 					// Stopped tasks cannot be restarted, must be removed, and created again
-					if _, delErr := task.Delete(ctx); delErr != nil {
-						log.Printf("[Scale] error deleting stopped task %s, error: %s\n", name, delErr)
-						http.Error(w, delErr.Error(), http.StatusBadRequest)
+					if _, err := task.Delete(ctx); err != nil {
+						log.Printf("[Scale] error deleting stopped task %s, error: %s\n", name, err)
+						http.Error(w, err.Error(), http.StatusBadRequest)
 						return
 					}
 					createNewTask = true
@@ -131,12 +140,70 @@ func MakeReplicaUpdateHandler(client *containerd.Client, cni gocni.CNI) func(w h
 		}
 
 		if createNewTask {
-			deployErr := createTask(ctx, client, ctr, cni)
-			if deployErr != nil {
-				log.Printf("[Scale] error deploying %s, error: %s\n", name, deployErr)
-				http.Error(w, deployErr.Error(), http.StatusBadRequest)
+			err := createTask(ctx, client, ctr, cni)
+			if err != nil {
+				log.Printf("[Scale] error deploying %s, error: %s\n", name, err)
+				http.Error(w, err.Error(), http.StatusBadRequest)
 				return
 			}
 		}
+
+		if err := waitUntilHealthy(name, resolver, healthPath); err != nil {
+			log.Printf("[Scale] error waiting for function %s to become ready, error: %s\n", name, err)
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
 	}
 }
+
+// waitUntilHealthy blocks until the healthPath returns a HTTP 200 for the
+// IP address resolved for the given function.
+// Maximum retries: 100
+// Delay between each attempt: 20ms
+// A custom path can be set via an annotation in the function's spec:
+//  com.openfaas.health.http.path: /handlers/ready
+//
+func waitUntilHealthy(name string, resolver proxy.BaseURLResolver, healthPath string) error {
+	endpoint, err := resolver.Resolve(name)
+	if err != nil {
+		return err
+	}
+
+	host, port, _ := net.SplitHostPort(endpoint.Host)
+	u, err := url.Parse(fmt.Sprintf("http://%s:%s%s", host, port, healthPath))
+	if err != nil {
+		return err
+	}
+
+	// Try to hit the health endpoint and block until
+	// ready.
+	attempts := 100
+	pause := time.Millisecond * 20
+	for i := 0; i < attempts; i++ {
+		req, err := http.NewRequest(http.MethodGet, u.String(), nil)
+		if err != nil {
+			return err
+		}
+
+		res, err := http.DefaultClient.Do(req)
+		if err != nil {
+			return err
+		}
+
+		if res.Body != nil {
+			res.Body.Close()
+		}
+
+		if res.StatusCode != http.StatusOK {
+			return fmt.Errorf("unexpected health status: %d", res.StatusCode)
+		}
+
+		if err == nil {
+			break
+		}
+
+		time.Sleep(pause)
+	}
+
+	return nil
+}