Fix annotation/label loading in #128

By moving to a reference instead of a value type, this patch
means the annotations are not leaked into other objects in
the result of ListFunctions. Tested on x86 with a Linux host
and I could no longer reproduce the issue in #128

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

pkg/provider/handlers/functions.go

@@ -25,91 +25,96 @@ type Function struct {
 }
 
 // ListFunctions returns a map of all functions with running tasks on namespace
-func ListFunctions(client *containerd.Client) (map[string]Function, error) {
+func ListFunctions(client *containerd.Client) (map[string]*Function, error) {
 	ctx := namespaces.WithNamespace(context.Background(), faasd.FunctionNamespace)
-	functions := make(map[string]Function)
+	functions := make(map[string]*Function)
 
-	containers, _ := client.Containers(ctx)
-	for _, k := range containers {
-		name := k.ID()
+	containers, err := client.Containers(ctx)
+	if err != nil {
+		return functions, err
+	}
+
+	for _, c := range containers {
+		name := c.ID()
 		f, err := GetFunction(client, name)
 		if err != nil {
-			continue
+			log.Printf("error getting function %s: ", name)
+			return functions, err
 		}
-		functions[name] = f
+		functions[name] = &f
 	}
+
 	return functions, nil
 }
 
 // GetFunction returns a function that matches name
 func GetFunction(client *containerd.Client, name string) (Function, error) {
 	ctx := namespaces.WithNamespace(context.Background(), faasd.FunctionNamespace)
+	fn := Function{}
+
 	c, err := client.LoadContainer(ctx, name)
-
-	if err == nil {
-		image, _ := c.Image(ctx)
-
-		containerName := c.ID()
-		allLabels, labelErr := c.Labels(ctx)
-
-		if labelErr != nil {
-			log.Printf("cannot list container %s labels: %s", containerName, labelErr.Error())
-		}
-
-		labels, annotations := buildLabelsAndAnnotations(allLabels)
-
-		f := Function{
-			name:        containerName,
-			namespace:   faasd.FunctionNamespace,
-			image:       image.Name(),
-			labels:      labels,
-			annotations: annotations,
-		}
-
-		replicas := 0
-		task, err := c.Task(ctx, nil)
-		if err == nil {
-			// Task for container exists
-			svc, err := task.Status(ctx)
-			if err != nil {
-				return Function{}, fmt.Errorf("unable to get task status for container: %s %s", name, err)
-			}
-
-			if svc.Status == "running" {
-				replicas = 1
-				f.pid = task.Pid()
-
-				// Get container IP address
-				ip, err := cninetwork.GetIPfromPID(int(task.Pid()))
-				if err != nil {
-					return Function{}, err
-				}
-				f.IP = ip.String()
-			}
-		} else {
-			replicas = 0
-		}
-
-		f.replicas = replicas
-		return f, nil
+	if err != nil {
+		return Function{}, fmt.Errorf("unable to find function: %s, error %s", name, err)
 	}
 
-	return Function{}, fmt.Errorf("unable to find function: %s, error %s", name, err)
+	image, err := c.Image(ctx)
+	if err != nil {
+		return fn, err
+	}
+
+	containerName := c.ID()
+	allLabels, labelErr := c.Labels(ctx)
+
+	if labelErr != nil {
+		log.Printf("cannot list container %s labels: %s", containerName, labelErr.Error())
+	}
+
+	labels, annotations := buildLabelsAndAnnotations(allLabels)
+
+	fn.name = containerName
+	fn.namespace = faasd.FunctionNamespace
+	fn.image = image.Name()
+	fn.labels = labels
+	fn.annotations = annotations
+
+	replicas := 0
+	task, err := c.Task(ctx, nil)
+	if err == nil {
+		// Task for container exists
+		svc, err := task.Status(ctx)
+		if err != nil {
+			return Function{}, fmt.Errorf("unable to get task status for container: %s %s", name, err)
+		}
+
+		if svc.Status == "running" {
+			replicas = 1
+			fn.pid = task.Pid()
+
+			// Get container IP address
+			ip, err := cninetwork.GetIPfromPID(int(task.Pid()))
+			if err != nil {
+				return Function{}, err
+			}
+			fn.IP = ip.String()
+		}
+	} else {
+		replicas = 0
+	}
+
+	fn.replicas = replicas
+	return fn, nil
 }
 
-func buildLabelsAndAnnotations(ctrLabels map[string]string) (labels map[string]string, annotations map[string]string) {
+// buildLabelsAndAnnotations returns a separated list with labels first,
+// followed by annotations by checking each key of ctrLabels for a prefix.
+func buildLabelsAndAnnotations(ctrLabels map[string]string) (map[string]string, map[string]string) {
+	labels := make(map[string]string)
+	annotations := make(map[string]string)
+
 	for k, v := range ctrLabels {
 		if strings.HasPrefix(k, annotationLabelPrefix) {
-			if annotations == nil {
-				annotations = make(map[string]string)
-			}
-
 			annotations[strings.TrimPrefix(k, annotationLabelPrefix)] = v
 		} else {
-			if labels == nil {
-				labels = make(map[string]string)
-			}
-
 			labels[k] = v
 		}
 	}