operator: prevent upgrades on degraded pools

pkg/operator/status.go

@@ -247,6 +247,12 @@ func (optr *Operator) syncUpgradeableStatus() error {
 	if co == nil {
 		return nil
 	}
+
+	pools, err := optr.mcpLister.List(labels.Everything())
+	if err != nil {
+		return err
+	}
+
 	// Report default "Upgradeable=True" status. When known hazardous states for upgrades are
 	// determined, specific "Upgradeable=False" status can be added with messages for how admins
 	// can resolve it.
@@ -256,6 +262,14 @@ func (optr *Operator) syncUpgradeableStatus() error {
 		Status: configv1.ConditionTrue,
 		Reason: asExpectedReason,
 	}
+	for _, pool := range pools {
+		degraded := isPoolStatusConditionTrue(pool, mcfgv1.MachineConfigPoolDegraded)
+		if degraded {
+			coStatus.Status = configv1.ConditionFalse
+			coStatus.Reason = "One or more machine config pool is degraded, please see `oc get mcp` for further details and resolve before upgrading"
+		}
+	}
+
 	return optr.updateStatus(co, coStatus)
 }
 

pkg/operator/sync.go

@@ -19,6 +19,7 @@ import (
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/tools/cache"
 
@@ -555,18 +556,7 @@ func (optr *Operator) syncMachineConfigServer(config *renderConfig) error {
 // syncRequiredMachineConfigPools ensures that all the nodes in machineconfigpools labeled with requiredForUpgradeMachineConfigPoolLabelKey
 // have updated to the latest configuration.
 func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error {
-	sel, err := metav1.LabelSelectorAsSelector(metav1.AddLabelToSelector(&metav1.LabelSelector{}, requiredForUpgradeMachineConfigPoolLabelKey, ""))
-	if err != nil {
-		return err
-	}
-	isPoolStatusConditionTrue := func(pool *mcfgv1.MachineConfigPool, conditionType mcfgv1.MachineConfigPoolConditionType) bool {
-		for _, condition := range pool.Status.Conditions {
-			if condition.Type == conditionType {
-				return condition.Status == corev1.ConditionTrue
-			}
-		}
-		return false
-	}
+	glog.Infof("syncing Required MachineConfigPools")
 
 	var lastErr error
 	if err := wait.Poll(time.Second, 10*time.Minute, func() (bool, error) {
@@ -587,28 +577,46 @@ func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error {
 				return false, nil
 			}
 		}
-		pools, err := optr.mcpLister.List(sel)
+
+		pools, err := optr.mcpLister.List(labels.Everything())
 		if err != nil {
 			lastErr = err
 			return false, nil
 		}
 		for _, pool := range pools {
-			if err := isMachineConfigPoolConfigurationValid(pool, version.Hash, optr.mcLister.Get); err != nil {
-				lastErr = fmt.Errorf("pool %s has not progressed to latest configuration: %v, retrying", pool.Name, err)
+			degraded := isPoolStatusConditionTrue(pool, mcfgv1.MachineConfigPoolDegraded)
+			if degraded {
+				lastErr = fmt.Errorf("error pool %s is not ready, retrying. Status: (pool degraded: %v total: %d, ready %d, updated: %d, unavailable: %d)", pool.Name, degraded, pool.Status.MachineCount, pool.Status.ReadyMachineCount, pool.Status.UpdatedMachineCount, pool.Status.UnavailableMachineCount)
+				glog.Errorf("Error syncing Required MachineConfigPools: %q", lastErr)
+				syncerr := optr.syncUpgradeableStatus()
+				if syncerr != nil {
+					glog.Errorf("Error syncingUpgradeableStatus: %q", syncerr)
+				}
 				return false, nil
 			}
-			degraded := isPoolStatusConditionTrue(pool, mcfgv1.MachineConfigPoolDegraded)
-			if pool.Generation <= pool.Status.ObservedGeneration &&
-				isPoolStatusConditionTrue(pool, mcfgv1.MachineConfigPoolUpdated) &&
-				!degraded {
-				continue
+
+			_, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey]
+
+			if hasRequiredPoolLabel {
+				if err := isMachineConfigPoolConfigurationValid(pool, version.Hash, optr.mcLister.Get); err != nil {
+					lastErr = fmt.Errorf("pool %s has not progressed to latest configuration: %v, retrying", pool.Name, err)
+					return false, nil
+				}
+
+				if pool.Generation <= pool.Status.ObservedGeneration &&
+					isPoolStatusConditionTrue(pool, mcfgv1.MachineConfigPoolUpdated) {
+					continue
+				}
 			}
-			lastErr = fmt.Errorf("error pool %s is not ready, retrying. Status: (pool degraded: %v total: %d, ready %d, updated: %d, unavailable: %d)", pool.Name, degraded, pool.Status.MachineCount, pool.Status.ReadyMachineCount, pool.Status.UpdatedMachineCount, pool.Status.UnavailableMachineCount)
-			return false, nil
+		}
+		syncstatuserr := optr.syncUpgradeableStatus()
+		if syncstatuserr != nil {
+			glog.Errorf("Error syncingUpgradeableStatus: %q", syncstatuserr)
 		}
 		return true, nil
 	}); err != nil {
 		if err == wait.ErrWaitTimeout {
+			glog.Errorf("Error syncing Required MachineConfigPools: %q", lastErr)
 			return fmt.Errorf("%v during syncRequiredMachineConfigPools: %v", err, lastErr)
 		}
 		return err
@@ -840,3 +848,12 @@ func mergeCertWithCABundle(initialBundle, newBundle []byte, subject string) []by
 	}
 	return mergedBytes
 }
+
+func isPoolStatusConditionTrue(pool *mcfgv1.MachineConfigPool, conditionType mcfgv1.MachineConfigPoolConditionType) bool {
+	for _, condition := range pool.Status.Conditions {
+		if condition.Type == conditionType {
+			return condition.Status == corev1.ConditionTrue
+		}
+	}
+	return false
+}