From c4936133f67b8e6b22720bfc4537088aa203bd59 Mon Sep 17 00:00:00 2001
From: "Alex Ellis (OpenFaaS Ltd)" <alexellis2@gmail.com>
Date: Thu, 31 Dec 2020 18:37:31 +0000
Subject: [PATCH] Pre-pull images for updates

The update flow used to delete the active function before
synchronously pulling the next and starting it. That meant
functions would always face downtime during the pull.

This changes the order to pre-pull and reduce any down time.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
---
 pkg/provider/handlers/deploy.go | 35 +++++++++++++++++++++++++++++++--
 pkg/provider/handlers/update.go | 24 +++++++++++++---------
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/pkg/provider/handlers/deploy.go b/pkg/provider/handlers/deploy.go
index 63de3a5..1067f77 100644
--- a/pkg/provider/handlers/deploy.go
+++ b/pkg/provider/handlers/deploy.go
@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/cio"
@@ -68,7 +69,11 @@ func MakeDeployHandler(client *containerd.Client, cni gocni.CNI, secretMountPath
 	}
 }
 
-func deploy(ctx context.Context, req types.FunctionDeployment, client *containerd.Client, cni gocni.CNI, secretMountPath string, alwaysPull bool) error {
+// prepull is an optimization which means an image can be pulled before a deployment
+// request, since a deployment request first deletes the active function before
+// trying to deploy a new one.
+func prepull(ctx context.Context, req types.FunctionDeployment, client *containerd.Client, alwaysPull bool) error {
+	start := time.Now()
 	r, err := reference.ParseNormalizedNamed(req.Image)
 	if err != nil {
 		return err
@@ -87,7 +92,33 @@ func deploy(ctx context.Context, req types.FunctionDeployment, client *container
 	}
 
 	size, _ := image.Size(ctx)
-	log.Printf("Deploy %s size: %d\n", image.Name(), size)
+	log.Printf("[Prepull] Deploy %s size: %d, took: %fs\n", image.Name(), size, time.Since(start).Seconds())
+
+	return nil
+}
+
+func deploy(ctx context.Context, req types.FunctionDeployment, client *containerd.Client, cni gocni.CNI, secretMountPath string, alwaysPull bool) error {
+	start := time.Now()
+
+	r, err := reference.ParseNormalizedNamed(req.Image)
+	if err != nil {
+		return err
+	}
+
+	imgRef := reference.TagNameOnly(r).String()
+
+	snapshotter := ""
+	if val, ok := os.LookupEnv("snapshotter"); ok {
+		snapshotter = val
+	}
+
+	image, err := service.PrepareImage(ctx, client, imgRef, snapshotter, alwaysPull)
+	if err != nil {
+		return errors.Wrapf(err, "unable to pull image %s", imgRef)
+	}
+
+	size, _ := image.Size(ctx)
+	log.Printf("[deploy] Deploy %s size: %d, took: %fs\n", image.Name(), size, time.Since(start).Seconds())
 
 	envs := prepareEnv(req.EnvProcess, req.EnvVars)
 	mounts := getMounts()
diff --git a/pkg/provider/handlers/update.go b/pkg/provider/handlers/update.go
index cff0156..1d7a5f3 100644
--- a/pkg/provider/handlers/update.go
+++ b/pkg/provider/handlers/update.go
@@ -56,6 +56,12 @@ func MakeUpdateHandler(client *containerd.Client, cni gocni.CNI, secretMountPath
 		}
 
 		ctx := namespaces.WithNamespace(context.Background(), faasd.FunctionNamespace)
+
+		if err := prepull(ctx, req, client, alwaysPull); err != nil {
+			log.Printf("[Update] error with pre-pull: %s, %s\n", name, err)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+		}
+
 		if function.replicas != 0 {
 			err = cninetwork.DeleteCNINetwork(ctx, cni, client, name)
 			if err != nil {
@@ -63,19 +69,19 @@ func MakeUpdateHandler(client *containerd.Client, cni gocni.CNI, secretMountPath
 			}
 		}
 
-		containerErr := service.Remove(ctx, client, name)
-		if containerErr != nil {
-			log.Printf("[Update] error removing %s, %s\n", name, containerErr)
-			http.Error(w, containerErr.Error(), http.StatusInternalServerError)
+		if err := service.Remove(ctx, client, name); err != nil {
+			log.Printf("[Update] error removing %s, %s\n", name, err)
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
 
-		deployErr := deploy(ctx, req, client, cni, secretMountPath, alwaysPull)
-		if deployErr != nil {
-			log.Printf("[Update] error deploying %s, error: %s\n", name, deployErr)
-			http.Error(w, deployErr.Error(), http.StatusBadRequest)
+		// The pull has already been done in prepull, so we can force this pull to "false"
+		pull := false
+
+		if err := deploy(ctx, req, client, cni, secretMountPath, pull); err != nil {
+			log.Printf("[Update] error deploying %s, error: %s\n", name, err)
+			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 	}
-
 }