Lazy upgrade of the Spilo image (#859)

* initial implementation

* describe forcing the rolling upgrade

* make parameter name more descriptive

* add missing pieces

* address review

* address review

* fix bug in e2e tests

* fix cluster name label in e2e test

* raise test timeout

* load spilo test image

* use available spilo image

* delete replica pod for lazy update test

* fix e2e

* fix e2e with a vengeance

* lets wait for another 30m

* print pod name in error msg

* print pod name in error msg 2

* raise timeout, comment other tests

* subsequent updates of config

* add comma

* fix e2e test

* run unit tests before e2e

* remove conflicting dependency

* Revert "remove conflicting dependency"

This reverts commit 65fc09054b.

* improve cdp build

* dont run unit before e2e tests

* Revert "improve cdp build"

This reverts commit e2a8fa12aa.

Co-authored-by: Sergey Dudoladov <sergey.dudoladov@zalando.de>
Co-authored-by: Felix Kunde <felix-kunde@gmx.de>

pkg/apis/acid.zalan.do/v1/crds.go

@@ -761,6 +761,9 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation
 					"enable_crd_validation": {
 						Type: "boolean",
 					},
+					"enable_lazy_spilo_upgrade": {
+						Type: "boolean",
+					},
 					"enable_shm_volume": {
 						Type: "boolean",
 					},

pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

@@ -183,6 +183,7 @@ type OperatorLogicalBackupConfiguration struct {
 // OperatorConfigurationData defines the operation config
 type OperatorConfigurationData struct {
 	EnableCRDValidation     *bool    `json:"enable_crd_validation,omitempty"`
+	EnableLazySpiloUpgrade  bool     `json:"enable_lazy_spilo_upgrade,omitempty"`
 	EtcdHost                string   `json:"etcd_host,omitempty"`
 	KubernetesUseConfigMaps bool     `json:"kubernetes_use_configmaps,omitempty"`
 	DockerImage             string   `json:"docker_image,omitempty"`

pkg/cluster/cluster.go

@@ -470,6 +470,14 @@ func (c *Cluster) compareStatefulSetWith(statefulSet *appsv1.StatefulSet) *compa
 		}
 	}
 
+	// lazy Spilo update: modify the image in the statefulset itself but let its pods run with the old image
+	// until they are re-created for other reasons, for example node rotation
+	if c.OpConfig.EnableLazySpiloUpgrade && !reflect.DeepEqual(c.Statefulset.Spec.Template.Spec.Containers[0].Image, statefulSet.Spec.Template.Spec.Containers[0].Image) {
+		needsReplace = true
+		needsRollUpdate = false
+		reasons = append(reasons, "lazy Spilo update: new statefulset's pod image doesn't match the current one")
+	}
+
 	if needsRollUpdate || needsReplace {
 		match = false
 	}
@@ -501,8 +509,6 @@ func (c *Cluster) compareContainers(description string, setA, setB []v1.Containe
 	checks := []containerCheck{
 		newCheck("new statefulset %s's %s (index %d) name doesn't match the current one",
 			func(a, b v1.Container) bool { return a.Name != b.Name }),
-		newCheck("new statefulset %s's %s (index %d) image doesn't match the current one",
-			func(a, b v1.Container) bool { return a.Image != b.Image }),
 		newCheck("new statefulset %s's %s (index %d) ports don't match the current one",
 			func(a, b v1.Container) bool { return !reflect.DeepEqual(a.Ports, b.Ports) }),
 		newCheck("new statefulset %s's %s (index %d) resources don't match the current ones",
@@ -513,6 +519,11 @@ func (c *Cluster) compareContainers(description string, setA, setB []v1.Containe
 			func(a, b v1.Container) bool { return !reflect.DeepEqual(a.EnvFrom, b.EnvFrom) }),
 	}
 
+	if !c.OpConfig.EnableLazySpiloUpgrade {
+		checks = append(checks, newCheck("new statefulset %s's %s (index %d) image doesn't match the current one",
+			func(a, b v1.Container) bool { return a.Image != b.Image }))
+	}
+
 	for index, containerA := range setA {
 		containerB := setB[index]
 		for _, check := range checks {

pkg/cluster/sync.go

@@ -4,17 +4,17 @@ import (
 	"context"
 	"fmt"
 
-	batchv1beta1 "k8s.io/api/batch/v1beta1"
-	v1 "k8s.io/api/core/v1"
-	policybeta1 "k8s.io/api/policy/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
 	"github.com/zalando/postgres-operator/pkg/spec"
 	"github.com/zalando/postgres-operator/pkg/util"
 	"github.com/zalando/postgres-operator/pkg/util/constants"
 	"github.com/zalando/postgres-operator/pkg/util/k8sutil"
 	"github.com/zalando/postgres-operator/pkg/util/volumes"
+	appsv1 "k8s.io/api/apps/v1"
+	batchv1beta1 "k8s.io/api/batch/v1beta1"
+	v1 "k8s.io/api/core/v1"
+	policybeta1 "k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // Sync syncs the cluster, making sure the actual Kubernetes objects correspond to what is defined in the manifest.
@@ -252,6 +252,28 @@ func (c *Cluster) syncPodDisruptionBudget(isUpdate bool) error {
 	return nil
 }
 
+func (c *Cluster) mustUpdatePodsAfterLazyUpdate(desiredSset *appsv1.StatefulSet) (bool, error) {
+
+	pods, err := c.listPods()
+	if err != nil {
+		return false, fmt.Errorf("could not list pods of the statefulset: %v", err)
+	}
+
+	for _, pod := range pods {
+
+		effectivePodImage := pod.Spec.Containers[0].Image
+		ssImage := desiredSset.Spec.Template.Spec.Containers[0].Image
+
+		if ssImage != effectivePodImage {
+			c.logger.Infof("not all pods were re-started when the lazy upgrade was enabled; forcing the rolling upgrade now")
+			return true, nil
+		}
+
+	}
+
+	return false, nil
+}
+
 func (c *Cluster) syncStatefulSet() error {
 	var (
 		podsRollingUpdateRequired bool
@@ -330,6 +352,19 @@ func (c *Cluster) syncStatefulSet() error {
 				}
 			}
 		}
+
+		if !podsRollingUpdateRequired && !c.OpConfig.EnableLazySpiloUpgrade {
+			// even if desired and actual statefulsets match
+			// there still may be not up-to-date pods on condition
+			//  (a) the lazy update was just disabled
+			// and
+			//  (b) some of the pods were not restarted when the lazy update was still in place
+			podsRollingUpdateRequired, err = c.mustUpdatePodsAfterLazyUpdate(desiredSS)
+			if err != nil {
+				return fmt.Errorf("could not list pods of the statefulset: %v", err)
+			}
+		}
+
 	}
 
 	// Apply special PostgreSQL parameters that can only be set via the Patroni API.

pkg/controller/operator_config.go

@@ -34,6 +34,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 
 	// general config
 	result.EnableCRDValidation = fromCRD.EnableCRDValidation
+	result.EnableLazySpiloUpgrade = fromCRD.EnableLazySpiloUpgrade
 	result.EtcdHost = fromCRD.EtcdHost
 	result.KubernetesUseConfigMaps = fromCRD.KubernetesUseConfigMaps
 	result.DockerImage = fromCRD.DockerImage

pkg/util/config/config.go

@@ -157,6 +157,7 @@ type Config struct {
 	ProtectedRoles            []string          `name:"protected_role_names" default:"admin"`
 	PostgresSuperuserTeams    []string          `name:"postgres_superuser_teams" default:""`
 	SetMemoryRequestToLimit   bool              `name:"set_memory_request_to_limit" default:"false"`
+	EnableLazySpiloUpgrade    bool              `name:"enable_lazy_spilo_upgrade" default:"false"`
 }
 
 // MustMarshal marshals the config or panics