no longer gate removed processing when imagestreams in progress

pkg/stub/config.go

@@ -247,20 +247,14 @@ func (h *Handler) processError(opcfg *v1.Config, ctype v1.ConfigConditionType, c
 func (h *Handler) ProcessManagementField(cfg *v1.Config) (bool, bool, error) {
 	switch cfg.Spec.ManagementState {
 	case operatorsv1api.Removed:
-		// first, we will not process a Removed setting if a prior create/update cycle is still in progress;
-		// if still creating/updating, set the remove on hold condition and we'll try the remove once that
-		// is false
-		if util.ConditionTrue(cfg, v1.ImageChangesInProgress) && util.ConditionTrue(cfg, v1.RemovePending) {
-			return false, false, nil
-		}
-
 		if cfg.Status.ManagementState != operatorsv1api.Removed && !util.ConditionTrue(cfg, v1.RemovePending) {
 			now := kapis.Now()
 			condition := util.Condition(cfg, v1.RemovePending)
 			condition.LastTransitionTime = now
 			condition.LastUpdateTime = now
 			condition.Status = corev1.ConditionTrue
 			util.ConditionUpdate(cfg, condition)
+			logrus.Printf("Attempting stage 1 Removed management state: RemovePending == true")
 			return false, true, nil
 		}
 
@@ -272,6 +266,7 @@ func (h *Handler) ProcessManagementField(cfg *v1.Config) (bool, bool, error) {
 			condition.LastUpdateTime = now
 			condition.Status = corev1.ConditionFalse
 			util.ConditionUpdate(cfg, condition)
+			logrus.Printf("Attempting stage 3 Removed management state: RemovePending == false")
 			return false, true, nil
 		}
 
@@ -286,19 +281,24 @@ func (h *Handler) ProcessManagementField(cfg *v1.Config) (bool, bool, error) {
 			if err != nil {
 				return false, true, h.processError(cfg, v1.SamplesExist, corev1.ConditionUnknown, err, "The error %v during openshift namespace cleanup has left the samples in an unknown state")
 			}
-			// explicitly reset samples exist and import cred to false since the Config has not
-			// actually been deleted; secret watch ignores events when samples resource is in removed state
+			// explicitly reset exist/inprogress/error to false
 			now := kapis.Now()
-			condition := util.Condition(cfg, v1.SamplesExist)
-			condition.LastTransitionTime = now
-			condition.LastUpdateTime = now
-			condition.Status = corev1.ConditionFalse
-			util.ConditionUpdate(cfg, condition)
+			conditionsToSet := []v1.ConfigConditionType{v1.SamplesExist, v1.ImageChangesInProgress, v1.ImportImageErrorsExist}
+			for _, c := range conditionsToSet {
+				condition := util.Condition(cfg, c)
+				condition.LastTransitionTime = now
+				condition.LastUpdateTime = now
+				condition.Message = ""
+				condition.Reason = ""
+				condition.Status = corev1.ConditionFalse
+				util.ConditionUpdate(cfg, condition)
+			}
 			cfg.Status.ManagementState = operatorsv1api.Removed
 			// after online starter upgrade attempts while this operator was not set to managed,
 			// group arch discussion has decided that we report the latest version
 			cfg.Status.Version = h.version
 			h.ClearStatusConfigForRemoved(cfg)
+			logrus.Printf("Attempting stage 2 Removed management state: Status == Removed")
 			return false, true, nil
 		}
 

pkg/stub/handler.go

@@ -174,16 +174,14 @@ func (h *Handler) prepSamplesWatchEvent(kind, name string, annotations map[strin
 		return nil, "", false, nil
 	}
 
-	if util.ConditionFalse(cfg, v1.ImageChangesInProgress) {
-		// we do no return the cfg in these cases because we do not want to bother with any progress tracking
-		switch cfg.Spec.ManagementState {
-		case operatorsv1api.Removed:
-			logrus.Debugf("Not upserting %s/%s event because operator is in removed state and image changes are not in progress", kind, name)
-			return nil, "", false, nil
-		case operatorsv1api.Unmanaged:
-			logrus.Debugf("Not upserting %s/%s event because operator is in unmanaged state and image changes are not in progress", kind, name)
-			return nil, "", false, nil
-		}
+	// we do not return the cfg in these cases because we do not want to bother with any progress tracking
+	switch cfg.Spec.ManagementState {
+	case operatorsv1api.Removed:
+		logrus.Debugf("Not upserting %s/%s event because operator is in removed state and image changes are not in progress", kind, name)
+		return nil, "", false, nil
+	case operatorsv1api.Unmanaged:
+		logrus.Debugf("Not upserting %s/%s event because operator is in unmanaged state and image changes are not in progress", kind, name)
+		return nil, "", false, nil
 	}
 
 	filePath := ""
@@ -644,7 +642,7 @@ func (h *Handler) Handle(event util.Event) error {
 		if !doit || err != nil {
 			if err != nil || cfgUpdate {
 				// flush status update
-				dbg := "process mgmt update"
+				dbg := fmt.Sprintf("process mgmt update spec %s status %s", string(cfg.Spec.ManagementState), string(cfg.Status.ManagementState))
 				logrus.Printf("CRDUPDATE %s", dbg)
 				return h.crdwrapper.UpdateStatus(cfg, dbg)
 			}

pkg/stub/handler_test.go

@@ -187,38 +187,27 @@ func TestManagementState(t *testing.T) {
 	cfg.ResourceVersion = "3"
 	cfg.Spec.ManagementState = operatorsv1api.Removed
 	err = h.Handle(event)
+	// RemovePending now true
 	statuses[4] = corev1.ConditionTrue
 	validate(true, err, "", cfg, conditions, statuses, t)
 	if cfg.Status.ManagementState == operatorsv1api.Removed {
 		t.Fatalf("cfg status set to removed too early %#v", cfg)
 	}
 
-	// verify while we are image in progress and the remove on hold setting is still set to true
-	// if another event comes in
+	// verify while we are image in progress no false and the remove on hold setting is still set to true
 	cfg.ResourceVersion = "4"
 	err = h.Handle(event)
-	validate(true, err, "", cfg, conditions, statuses, t)
-	if cfg.Status.ManagementState == operatorsv1api.Removed {
-		t.Fatalf("cfg status set to removed too early %#v", cfg)
-	}
-
-	// mimic when in progress set to false by imagestream watch
-	// then analyze resulting Config event
-	progressing := util.Condition(cfg, v1.ImageChangesInProgress)
-	progressing.Status = corev1.ConditionFalse
-	progressing.Reason = ""
-	util.ConditionUpdate(cfg, progressing)
-	cfg.ResourceVersion = "5"
-	err = h.Handle(event)
-	// index 0 samples exist should be false
+	// SamplesExists, ImageChangesInProgress, ImportImageErrorsExists all false
 	statuses[0] = corev1.ConditionFalse
-	// in progress should be false
 	statuses[3] = corev1.ConditionFalse
+	statuses[6] = corev1.ConditionFalse
 	validate(true, err, "", cfg, conditions, statuses, t)
 	if cfg.Status.ManagementState != operatorsv1api.Removed {
-		t.Fatalf("cfg status not set to removed %#v", cfg)
+		t.Fatalf("cfg status should have been set to removed %#v", cfg)
 	}
-	cfg.ResourceVersion = "6"
+
+	cfg.ResourceVersion = "5"
+	err = h.Handle(event)
 	// remove pending should be false
 	statuses[4] = corev1.ConditionFalse
 	err = h.Handle(event)

pkg/util/util.go

@@ -277,7 +277,7 @@ func ClusterOperatorStatusProgressingCondition(s *samplev1.Config, degradedState
 		return configv1.ConditionTrue, "", fmt.Sprintf(moving, os.Getenv("RELEASE_VERSION"))
 	}
 	if ConditionTrue(s, samplev1.RemovePending) {
-		return configv1.ConditionTrue, "", fmt.Sprintf(removing, s.Status.Version)
+		return configv1.ConditionTrue, "", fmt.Sprintf(removing, os.Getenv("RELEASE_VERSION"))
 	}
 	if available == configv1.ConditionTrue {
 		msg := fmt.Sprintf(installed, s.Status.Version)