Merge pull request #44 from cloudfoundry-incubator/reuse-startup-ordi…

…nal-175984792

Reuse startup ordinal 175984792

docs/examples/qstatefulset_active_passive.yaml

@@ -26,6 +26,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

docs/examples/qstatefulset_azs.yaml

@@ -19,6 +19,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

docs/examples/qstatefulset_configs.yaml

@@ -43,6 +43,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

docs/examples/qstatefulset_configs_fail.yaml

@@ -26,6 +26,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - "false"
             volumeMounts:

docs/examples/qstatefulset_configs_updated.yaml

@@ -43,6 +43,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

docs/examples/qstatefulset_pvcs.yaml

@@ -17,6 +17,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

docs/examples/qstatefulset_tolerations.yaml

@@ -17,6 +17,7 @@ spec:
           containers:
           - name: busybox
             image: busybox
+            imagePullPolicy: IfNotPresent
             command:
             - sleep
             - "3600"

go.mod

@@ -1,7 +1,7 @@
 module code.cloudfoundry.org/quarks-statefulset
 
 require (
-	code.cloudfoundry.org/quarks-utils v0.0.2-0.20201027114038-8aab73d224e4
+	code.cloudfoundry.org/quarks-utils v0.0.2-0.20201210092952-19bc6c410169
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/docker/spdystream v0.0.0-20181023171402-6480d4af844c // indirect
 	github.com/elazarl/goproxy v0.0.0-20191011121108-aa519ddbe484 // indirect

go.sum

@@ -11,8 +11,8 @@ cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7
 cloud.google.com/go/firestore v1.1.0/go.mod h1:ulACoGHTpvq5r8rxGJ4ddJZBZqakUQqClKRT5SZwBmk=
 cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I=
 cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw=
-code.cloudfoundry.org/quarks-utils v0.0.2-0.20201027114038-8aab73d224e4 h1:0WGZtcvmHLdWMlzBN9NwP6RrdMmlHwopc8RrkWR8wW4=
-code.cloudfoundry.org/quarks-utils v0.0.2-0.20201027114038-8aab73d224e4/go.mod h1:K8KH67rdNk9+VPOA5QRgrujTFhbmtqxLOuPQ6APL6ks=
+code.cloudfoundry.org/quarks-utils v0.0.2-0.20201210092952-19bc6c410169 h1:C1pBoyhlTLAoISMnoSjkAxNZn8QVWba+Wtd+eSQ2seA=
+code.cloudfoundry.org/quarks-utils v0.0.2-0.20201210092952-19bc6c410169/go.mod h1:js5WFs4G2V0TR/8zJMhDTfMU7j/AjKUNKBvU/SnKiwo=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
 github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI=
@@ -358,8 +358,7 @@ github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
-github.com/onsi/ginkgo v1.13.0 h1:M76yO2HkZASFjXL0HSoZJ1AYEmQxNJmY41Jx1zNUq1Y=
-github.com/onsi/ginkgo v1.13.0/go.mod h1:+REjRxOmWfHCjfv9TTWB1jD1Frx4XydAD3zm1lskyM0=
+github.com/onsi/ginkgo v1.14.2 h1:8mVmC9kjFFmA8H4pKMUhcblgifdkOIXPvbhN1T36q1M=
 github.com/onsi/ginkgo v1.14.2/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY=
 github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
@@ -407,7 +406,6 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
 github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
-github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo=

integration/quarks_statefulset_test.go

@@ -1,20 +1,20 @@
 package integration_test
 
 import (
+	"context"
 	"fmt"
 	"time"
 
 	v1 "k8s.io/api/core/v1"
-
-	"code.cloudfoundry.org/quarks-utils/pkg/pod"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	qstsv1a1 "code.cloudfoundry.org/quarks-statefulset/pkg/kube/apis/quarksstatefulset/v1alpha1"
 	"code.cloudfoundry.org/quarks-statefulset/pkg/kube/controllers/statefulset"
+	"code.cloudfoundry.org/quarks-utils/pkg/pod"
 	"code.cloudfoundry.org/quarks-utils/pkg/pointers"
 	"code.cloudfoundry.org/quarks-utils/testing/machine"
 	helper "code.cloudfoundry.org/quarks-utils/testing/testhelper"
@@ -270,6 +270,46 @@ var _ = Describe("QuarksStatefulSet", func() {
 				Expect(err).NotTo(HaveOccurred())
 			})
 		})
+
+		When("restarting a pod from the quarks statefulset", func() {
+			BeforeEach(func() {
+				quarksStatefulSet.Spec.Template.Spec.Replicas = pointers.Int32(3)
+				quarksStatefulSet.Spec.Template.Spec.Template.Spec.TerminationGracePeriodSeconds = pointers.Int64(10)
+				qSts = &quarksStatefulSet
+			})
+
+			It("keeps the previous startup ordinal", func() {
+				By("Checking for pod")
+				err = env.WaitForPods(env.Namespace, "testpod=yes")
+				Expect(err).NotTo(HaveOccurred())
+
+				pods, err := env.GetPods(env.Namespace, "testpod=yes")
+				Expect(err).NotTo(HaveOccurred())
+
+				By("Checking for the pod-ordinal label")
+				expect := map[string]string{}
+				for _, p := range pods.Items {
+					if ord, ok := p.Labels["quarks.cloudfoundry.org/pod-ordinal"]; ok {
+						expect[ord] = p.Labels[qstsv1a1.LabelStartupOrdinal]
+					}
+				}
+				pod := pods.Items[1]
+				client := env.Clientset.CoreV1().Pods(env.Namespace)
+
+				By("Restarting a pod")
+				err = client.Delete(context.Background(), pod.Name, metav1.DeleteOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				var p *v1.Pod
+				Eventually(func() bool {
+					p, _ = client.Get(context.Background(), pod.Name, metav1.GetOptions{})
+					return p.ObjectMeta.ResourceVersion > pod.ObjectMeta.ResourceVersion
+				}, 60*time.Second).Should(Equal(true))
+
+				ord := p.Labels["quarks.cloudfoundry.org/pod-ordinal"]
+				Expect(p.Labels[qstsv1a1.LabelStartupOrdinal]).To(Equal(expect[ord]))
+			})
+		})
 	})
 
 	Context("Rollout Recovery", func() {

pkg/kube/apis/quarksstatefulset/v1alpha1/types.go

@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	"encoding/json"
 	"fmt"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -20,6 +21,8 @@ const DefaultZoneNodeLabel = "failure-domain.beta.kubernetes.io/zone"
 var (
 	// AnnotationVersion is the annotation key for the StatefulSet version
 	AnnotationVersion = fmt.Sprintf("%s/version", apis.GroupName)
+	// AnnotationRevisions contains the history of revisions and pod ordinals and their startup ordinals
+	AnnotationRevisions = fmt.Sprintf("%s/ordinal-revisions", apis.GroupName)
 	// AnnotationZones is an array of all zones
 	AnnotationZones = fmt.Sprintf("%s/zones", apis.GroupName)
 	// LabelAZIndex is the index of available zone
@@ -33,8 +36,12 @@ var (
 
 	// LabelInstance is the instance.id/spec.id
 	LabelInstance = fmt.Sprintf("%s/spec-index", apis.GroupName)
+
 	// LabelQStsName is the name of the QuarksStatefulSet owns this resource
 	LabelQStsName = fmt.Sprintf("%s/quarks-statefulset-name", apis.GroupName)
+	// LabelStsName is the name of the QuarksStatefulSet owns this resource
+	LabelStsName = fmt.Sprintf("%s/statefulset-name", apis.GroupName)
+
 	// LabelActivePod is the active pod on an active/passive setup
 	LabelActivePod = fmt.Sprintf("%s/pod-active", apis.GroupName)
 )
@@ -108,3 +115,52 @@ func (q *QuarksStatefulSet) GetMaxAvailableVersion(versions map[int]bool) int {
 func (q *QuarksStatefulSet) GetNamespacedName() string {
 	return fmt.Sprintf("%s/%s", q.Namespace, q.Name)
 }
+
+// Revisions maps controller revision hashes to a map of assigned startup ordinals for each pod ordinal
+type Revisions map[string]Ordinals
+
+// Ordinals maps assigned pod ordinals to their corresponding startup ordinals
+type Ordinals map[string]string
+
+// GetRevisions returns the controller-revision-hash lookup table for pod ordinal to startup ordinal assignments
+// returns empty struct if annotation couldn't be parse, since we can't distinguish between 'new' and 'corrupted'
+func (q *QuarksStatefulSet) GetRevisions() Revisions {
+	data := q.Annotations[AnnotationRevisions]
+	revisions := &Revisions{}
+	err := json.Unmarshal([]byte(data), revisions)
+	if err != nil {
+		return Revisions{}
+	}
+	return *revisions
+}
+
+// SetRevisions sets the revisions annotation
+func (q *QuarksStatefulSet) SetRevisions(revisions Revisions) error {
+	data, err := json.Marshal(revisions)
+	if err != nil {
+		return err
+	}
+	metav1.SetMetaDataAnnotation(&q.ObjectMeta, AnnotationRevisions, string(data))
+	return nil
+}
+
+// StartupOrdinal returns the assigned one for a given revision hash and pod ordinal
+func (r Revisions) StartupOrdinal(revision string, podOrdinal string) string {
+	if r, found := r[revision]; found {
+		if startupOrdinal, found := r[podOrdinal]; found {
+			return startupOrdinal
+		}
+	}
+	return ""
+}
+
+// Set the startup ordinal for a given revision and pod ordinal
+func (r Revisions) Set(revision string, podOrdinal string, startupOrdinal string) {
+	o := r[revision]
+	if o == nil {
+		o = Ordinals{}
+	}
+	o[podOrdinal] = startupOrdinal
+
+	r[revision] = o
+}

pkg/kube/controllers/quarksstatefulset/active_passive_reconciler.go

@@ -216,7 +216,7 @@ func (r *ReconcileStatefulSetActivePassive) execContainerCmd(pod *corev1.Pod, co
 
 func (r *ReconcileStatefulSetActivePassive) getStsPodList(ctx context.Context, desiredSts *appsv1.StatefulSet) (*corev1.PodList, error) {
 	podList := &corev1.PodList{}
-	stsSelector := labels.SelectorFromSet(labels.Set(map[string]string{qstsv1a1.LabelQStsName: desiredSts.Name}))
+	stsSelector := labels.SelectorFromSet(labels.Set(map[string]string{qstsv1a1.LabelStsName: desiredSts.Name}))
 	err := r.client.List(ctx,
 		podList,
 		crc.InNamespace(desiredSts.Namespace),

pkg/kube/controllers/quarksstatefulset/pod_mutator.go

@@ -11,6 +11,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -60,7 +61,7 @@ func (m *PodMutator) Handle(ctx context.Context, req admission.Request) admissio
 		}
 		setPodOrdinal(updatedPod, podLabels)
 
-		err := m.setNewOrdinal(ctx, updatedPod, podLabels)
+		err := m.setStartupOrdinal(ctx, updatedPod, podLabels)
 		if err != nil {
 			return admission.Errored(http.StatusInternalServerError, err)
 		}
@@ -91,24 +92,77 @@ func setPodOrdinal(pod *corev1.Pod, podLabels map[string]string) {
 	}
 }
 
-func (m *PodMutator) setNewOrdinal(ctx context.Context, pod *corev1.Pod, podLabels map[string]string) error {
-	labels := map[string]string{"controller-revision-hash": pod.Labels["controller-revision-hash"]}
-	list := &corev1.PodList{}
-	err := m.client.List(ctx, list, client.InNamespace(pod.Namespace), client.MatchingLabels(labels))
+// startupOrdinal values are persisted in an annotation in the owning qsts, so they can be reused in case of a restart
+func (m *PodMutator) setStartupOrdinal(ctx context.Context, pod *corev1.Pod, podLabels map[string]string) error {
+	revision := pod.Labels["controller-revision-hash"]
+
+	// check for revision in qsts annotation
+	qsts := &qstsv1a1.QuarksStatefulSet{}
+	qstsName := types.NamespacedName{Name: pod.Labels[qstsv1a1.LabelQStsName], Namespace: pod.Namespace}
+	err := m.client.Get(ctx, qstsName, qsts)
+	if err != nil {
+		return errors.Wrapf(err, "failed to get qsts owning '%s/%s'", pod.Namespace, pod.Name)
+	}
+
+	revisions := qsts.GetRevisions()
+
+	// cleanup annotation, by removing outdated revisions
+	list := &appsv1.StatefulSetList{}
+	err = m.client.List(ctx, list, client.InNamespace(pod.Namespace))
 	if err != nil {
-		return errors.Wrapf(err, "failed to list pods in namespace: %s", pod.Namespace)
+		return errors.Wrapf(err, "failed to list sts in namespace: '%s'", pod.Namespace)
+	}
+	seen := map[string]bool{}
+	for _, sts := range list.Items {
+		if r := sts.Labels["controller-revision-hash"]; r != "" {
+			seen[r] = true
+		}
+	}
+	for r := range revisions {
+		if !seen[r] {
+			delete(revisions, r)
+		}
 	}
 
-	newOrdinal := 0
-	for _, p := range list.Items {
-		if p.Name != pod.Name {
-			newOrdinal++
+	// use old startup ordinal if stored
+	podOrdinal := pod.Labels[qstsv1a1.LabelPodOrdinal]
+	startupOrdinal := ""
+	if s := revisions.StartupOrdinal(revision, podOrdinal); s != "" {
+		startupOrdinal = s
+
+	} else {
+		// check for other pods in that revision
+		labels := map[string]string{"controller-revision-hash": revision}
+		list := &corev1.PodList{}
+		err = m.client.List(ctx, list, client.InNamespace(pod.Namespace), client.MatchingLabels(labels))
+		if err != nil {
+			return errors.Wrapf(err, "failed to list pods in namespace: '%s'", pod.Namespace)
+		}
+
+		// count existing pods in revision
+		newOrdinal := 0
+		for _, p := range list.Items {
+			if p.Name != pod.Name {
+				newOrdinal++
+			}
 		}
+		startupOrdinal = strconv.Itoa(newOrdinal)
 	}
 
-	// zero based
-	podLabels[qstsv1a1.LabelStartupOrdinal] = strconv.Itoa(newOrdinal)
+	podLabels[qstsv1a1.LabelStartupOrdinal] = startupOrdinal
 	pod.SetLabels(podLabels)
+
+	// store revisions
+	revisions.Set(revision, podOrdinal, startupOrdinal)
+	err = qsts.SetRevisions(revisions)
+	if err != nil {
+		return errors.Wrapf(err, "failed to marshall revisions annotations for '%s'", qstsName)
+	}
+
+	err = m.client.Update(ctx, qsts)
+	if err != nil {
+		return errors.Wrapf(err, "failed to update revisions annotation on qsts '%s'", qstsName)
+	}
 	return nil
 }