Only schedule switchover for pod migration, consider mainWindow for PGVERSION env change

Author: hughcapet

docs/administrator.md

@@ -208,9 +208,6 @@ Note that, changes in `SPILO_CONFIGURATION` env variable under `bootstrap.dcs`
 path are ignored for the diff. They will be applied through Patroni's rest api
 interface, following a restart of all instances.
 
-Rolling update is postponed until the next maintenance window if any is defined
-under the `maintenanceWindows` cluster manifest parameter.
-
 The operator also support lazy updates of the Spilo image. In this case the
 StatefulSet is only updated, but no rolling update follows. This feature saves
 you a switchover - and hence downtime - when you know pods are re-started later

docs/reference/cluster_manifest.md

@@ -116,7 +116,7 @@ These parameters are grouped directly under  the `spec` key in the manifest.
 
 * **maintenanceWindows**
   a list which defines specific time frames when certain maintenance operations
-  such as automatic major upgrades or rolling updates are allowed. Accepted formats
+  such as automatic major upgrades or master pod migration. Accepted formats
   are "01:00-06:00" for daily maintenance windows or "Sat:00:00-04:00" for specific
   days, with all times in UTC.
 

e2e/tests/test_e2e.py

@@ -1187,7 +1187,7 @@ def test_major_version_upgrade(self):
         Test major version upgrade: with full upgrade, maintenance window, and annotation
         """
         def check_version():
-            p = k8s.patroni_rest("acid-upgrade-test-0", "")
+            p = k8s.patroni_rest("acid-upgrade-test-0", "") or {}
             version = p.get("server_version", 0) // 10000
             return version
 

pkg/cluster/cluster.go

@@ -957,6 +957,11 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 	defer c.mu.Unlock()
 
 	c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdating)
+
+	if !isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) {
+		// do not apply any major version related changes yet
+		newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion
+	}
 	c.setSpec(newSpec)
 
 	defer func() {
@@ -1761,35 +1766,39 @@ func (c *Cluster) GetSwitchoverSchedule() string {
 }
 
 // Switchover does a switchover (via Patroni) to a candidate pod
-func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) error {
+func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName, inMaintWindow bool) error {
 	var err error
-	c.logger.Debugf("switching over from %q to %q", curMaster.Name, candidate)
-
-	if !isInMaintenanceWindow(c.Spec.MaintenanceWindows) {
-		c.logger.Infof("postponing switchover, not in maintenance window")
-		schedule := c.GetSwitchoverSchedule()
-
-		if err := c.patroni.Switchover(curMaster, candidate.Name, schedule); err != nil {
-			return fmt.Errorf("could not schedule switchover: %v", err)
-		}
-		c.logger.Infof("switchover is scheduled at %s", schedule)
-		return nil
-	}
 
-	c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeNormal, "Switchover", "Switching over from %q to %q", curMaster.Name, candidate)
 	stopCh := make(chan struct{})
 	ch := c.registerPodSubscriber(candidate)
 	defer c.unregisterPodSubscriber(candidate)
 	defer close(stopCh)
 
-	if err = c.patroni.Switchover(curMaster, candidate.Name, ""); err == nil {
+	var scheduled_at string
+	if inMaintWindow {
+		c.logger.Debugf("switching over from %q to %q", curMaster.Name, candidate)
+		c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeNormal, "Switchover", "Switching over from %q to %q", curMaster.Name, candidate)
+
+		scheduled_at = c.GetSwitchoverSchedule()
+	} else {
+		scheduled_at = ""
+	}
+
+	if err = c.patroni.Switchover(curMaster, candidate.Name, scheduled_at); err == nil {
+		if inMaintWindow {
+			c.logger.Infof("switchover is scheduled at %s", scheduled_at)
+			return nil
+		}
 		c.logger.Debugf("successfully switched over from %q to %q", curMaster.Name, candidate)
 		c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeNormal, "Switchover", "Successfully switched over from %q to %q", curMaster.Name, candidate)
 		_, err = c.waitForPodLabel(ch, stopCh, nil)
 		if err != nil {
 			err = fmt.Errorf("could not get master pod label: %v", err)
 		}
 	} else {
+		if inMaintWindow {
+			return fmt.Errorf("could not schedule switchover: %v", err)
+		}
 		err = fmt.Errorf("could not switch over from %q to %q: %v", curMaster.Name, candidate, err)
 		c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeNormal, "Switchover", "Switchover from %q to %q FAILED: %v", curMaster.Name, candidate, err)
 	}

pkg/cluster/pod.go

@@ -280,11 +280,16 @@ func (c *Cluster) MigrateMasterPod(podName spec.NamespacedName) error {
 		return fmt.Errorf("could not move pod: %v", err)
 	}
 
+	scheduleSwitchover := false
+	if !isInMaintenanceWindow(c.Spec.MaintenanceWindows) {
+		c.logger.Infof("postponing switchover, not in maintenance window")
+		scheduleSwitchover = true
+	}
 	err = retryutil.Retry(1*time.Minute, 5*time.Minute,
 		func() (bool, error) {
-			err := c.Switchover(oldMaster, masterCandidateName)
+			err := c.Switchover(oldMaster, masterCandidateName, scheduleSwitchover)
 			if err != nil {
-				c.logger.Errorf("could not failover to pod %q: %v", masterCandidateName, err)
+				c.logger.Errorf("could not switchover to pod %q: %v", masterCandidateName, err)
 				return false, nil
 			}
 			return true, nil
@@ -445,7 +450,7 @@ func (c *Cluster) recreatePods(pods []v1.Pod, switchoverCandidates []spec.Namesp
 				// do not recreate master now so it will keep the update flag and switchover will be retried on next sync
 				return fmt.Errorf("skipping switchover: %v", err)
 			}
-			if err := c.Switchover(masterPod, masterCandidate); err != nil {
+			if err := c.Switchover(masterPod, masterCandidate, false); err != nil {
 				return fmt.Errorf("could not perform switch over: %v", err)
 			}
 		} else if newMasterPod == nil && len(replicas) == 0 {

pkg/cluster/sync.go

@@ -97,6 +97,11 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		}
 	}
 
+	if !isInMaintenanceWindow(newSpec.Spec.MaintenanceWindows) {
+		// do not apply any major version related changes yet
+		newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion
+	}
+
 	if err = c.syncStatefulSet(); err != nil {
 		if !k8sutil.ResourceAlreadyExists(err) {
 			err = fmt.Errorf("could not sync statefulsets: %v", err)
@@ -658,11 +663,6 @@ func (c *Cluster) syncStatefulSet() error {
 		isSafeToRecreatePods = false
 	}
 
-	if !isInMaintenanceWindow(c.Spec.MaintenanceWindows) {
-		postponeReasons = append(postponeReasons, "not in maintenance window")
-		isSafeToRecreatePods = false
-	}
-
 	// if we get here we also need to re-create the pods (either leftovers from the old
 	// statefulset or those that got their configuration from the outdated statefulset)
 	if len(podsToRecreate) > 0 {