From 4cb0fd394987cd167e436a5c1f42c667a336e6c0 Mon Sep 17 00:00:00 2001
From: Ryan Richard <richardry@vmware.com>
Date: Tue, 11 Aug 2020 17:55:34 -0700
Subject: [PATCH] Use a DaemonSet instead of a Deployment to deploy our app

- For high availability reasons, we would like our app to scale linearly
  with the size of the control plane. Using a DaemonSet allows us to run
  one pod on each node-role.kubernetes.io/master node.
- The hope is that the Service that we create should load balance
  between these pods appropriately.
---
 deploy/deployment.yaml                        | 26 +++++++++----------
 ...yment_test.go => app_availability_test.go} | 12 +++------
 test/library/deployment.go                    | 20 --------------
 3 files changed, 16 insertions(+), 42 deletions(-)
 rename test/integration/{deployment_test.go => app_availability_test.go} (51%)
 delete mode 100644 test/library/deployment.go

diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml
index 5b9a23dc..baec010f 100644
--- a/deploy/deployment.yaml
+++ b/deploy/deployment.yaml
@@ -30,18 +30,16 @@ data:
       url: (@= data.values.webhook_url @)
       caBundle: (@= data.values.webhook_ca_bundle @)
 ---
-#! TODO set up healthy, ready, etc. probes correctly for our deployment
-#! TODO set the priority-critical-urgent on our deployment to ask kube to never let it die
-#! TODO set resource minimums (e.g. 512MB RAM) on the deployment to make sure we get scheduled onto a reasonable node
+#! TODO set up healthy, ready, etc. probes correctly?
+#! TODO set resource minimums (e.g. 512MB RAM) to make sure we get scheduled onto a reasonable node?
 apiVersion: apps/v1
-kind: Deployment
+kind: DaemonSet
 metadata:
-  name: #@ data.values.app_name + "-deployment"
+  name: #@ data.values.app_name
   namespace: #@ data.values.namespace
   labels:
     app: #@ data.values.app_name
 spec:
-  replicas: 1 #! TODO more than one replica for high availability, and share the same serving certificate among them (maybe using client-go leader election)
   selector:
     matchLabels:
       app: #@ data.values.app_name
@@ -92,16 +90,16 @@ spec:
           hostPath:
             path: /etc/kubernetes/pki
             type: DirectoryOrCreate
-      #! "system-cluster-critical" cannot be used outside the kube-system namespace until Kubernetes >= 1.17,
-      #! so we skip setting this for now (see https://github.com/kubernetes/kubernetes/issues/60596).
-      #! priorityClassName: system-cluster-critical
-      nodeSelector:
+      nodeSelector: #! Create Pods on all nodes which match this node selector, and not on any other nodes.
         node-role.kubernetes.io/master: ""
       tolerations:
-      - key: CriticalAddonsOnly
-        operator: Exists
-      - effect: NoSchedule
-        key: node-role.kubernetes.io/master
+        - key: CriticalAddonsOnly
+          operator: Exists
+        - key: node-role.kubernetes.io/master #! Allow running on master nodes.
+          effect: NoSchedule
+      #! "system-cluster-critical" cannot be used outside the kube-system namespace until Kubernetes >= 1.17,
+      #! so we skip setting this for now (see https://github.com/kubernetes/kubernetes/issues/60596).
+      #!priorityClassName: system-cluster-critical
 ---
 apiVersion: v1
 kind: Service
diff --git a/test/integration/deployment_test.go b/test/integration/app_availability_test.go
similarity index 51%
rename from test/integration/deployment_test.go
rename to test/integration/app_availability_test.go
index 74c37371..6f830b16 100644
--- a/test/integration/deployment_test.go
+++ b/test/integration/app_availability_test.go
@@ -11,27 +11,23 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/suzerain-io/placeholder-name/test/library"
 )
 
-func TestGetDeployment(t *testing.T) {
+func TestAppAvailability(t *testing.T) {
 	library.SkipUnlessIntegration(t)
 	namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE")
-	deploymentName := library.Getenv(t, "PLACEHOLDER_NAME_DEPLOYMENT")
+	daemonSetName := library.Getenv(t, "PLACEHOLDER_NAME_APP_NAME")
 
 	client := library.NewClientset(t)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	appDeployment, err := client.AppsV1().Deployments(namespaceName).Get(ctx, deploymentName, metav1.GetOptions{})
+	daemonSet, err := client.AppsV1().DaemonSets(namespaceName).Get(ctx, daemonSetName, metav1.GetOptions{})
 	require.NoError(t, err)
 
-	cond := library.GetDeploymentCondition(appDeployment.Status, appsv1.DeploymentAvailable)
-	require.NotNil(t, cond)
-	require.Equalf(t, corev1.ConditionTrue, cond.Status, "app should be available: %s", library.Sdump(appDeployment))
+	require.GreaterOrEqual(t, daemonSet.Status.NumberAvailable, int32(1))
 }
diff --git a/test/library/deployment.go b/test/library/deployment.go
deleted file mode 100644
index 269c8168..00000000
--- a/test/library/deployment.go
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
-Copyright 2020 VMware, Inc.
-SPDX-License-Identifier: Apache-2.0
-*/
-
-package library
-
-import appsv1 "k8s.io/api/apps/v1"
-
-// GetDeploymentCondition returns the condition with the provided type.
-// Copied from k8s.io/kubectl/pkg/util/deployment/deployment.go to prevent us from vendoring the world.
-func GetDeploymentCondition(status appsv1.DeploymentStatus, condType appsv1.DeploymentConditionType) *appsv1.DeploymentCondition {
-	for i := range status.Conditions {
-		c := status.Conditions[i]
-		if c.Type == condType {
-			return &c
-		}
-	}
-	return nil
-}