Revert "[core] INVARIANT status introduced"

core/task/status.go

@@ -32,59 +32,47 @@ const (
 	PARTIAL
 	ACTIVE
 	UNDEPLOYABLE
-	INVARIANT // overwritten by product with any other state. It is used only when merging non-critical states. If you merge aggregateState with only non-critical statuses you will propagate INVARIANT further
 )
 
-var STATUS_PRODUCT = map[Status]map[Status]Status{
-	UNDEFINED: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     UNDEFINED,
-		PARTIAL:      UNDEFINED,
-		ACTIVE:       UNDEFINED,
-		UNDEPLOYABLE: UNDEFINED,
-		INVARIANT:    UNDEFINED,
-	},
-	INACTIVE: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     INACTIVE,
-		PARTIAL:      PARTIAL,
-		ACTIVE:       PARTIAL,
-		UNDEPLOYABLE: UNDEPLOYABLE,
-		INVARIANT:    INACTIVE,
-	},
-	PARTIAL: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     PARTIAL,
-		PARTIAL:      PARTIAL,
-		ACTIVE:       PARTIAL,
-		UNDEPLOYABLE: UNDEPLOYABLE,
-		INVARIANT:    PARTIAL,
-	},
-	ACTIVE: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     PARTIAL,
-		PARTIAL:      PARTIAL,
-		ACTIVE:       ACTIVE,
-		UNDEPLOYABLE: UNDEPLOYABLE,
-		INVARIANT:    ACTIVE,
-	},
-	UNDEPLOYABLE: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     UNDEPLOYABLE,
-		PARTIAL:      UNDEPLOYABLE,
-		ACTIVE:       UNDEPLOYABLE,
-		UNDEPLOYABLE: UNDEPLOYABLE,
-		INVARIANT:    UNDEPLOYABLE,
-	},
-	INVARIANT: {
-		UNDEFINED:    UNDEFINED,
-		INACTIVE:     INACTIVE,
-		PARTIAL:      PARTIAL,
-		ACTIVE:       ACTIVE,
-		UNDEPLOYABLE: UNDEPLOYABLE,
-		INVARIANT:    INVARIANT,
-	},
-}
+var (
+	STATUS_PRODUCT = map[Status]map[Status]Status{
+		UNDEFINED: {
+			UNDEFINED:    UNDEFINED,
+			INACTIVE:     UNDEFINED,
+			PARTIAL:      UNDEFINED,
+			ACTIVE:       UNDEFINED,
+			UNDEPLOYABLE: UNDEFINED,
+		},
+		INACTIVE: {
+			UNDEFINED:    UNDEFINED,
+			INACTIVE:     INACTIVE,
+			PARTIAL:      PARTIAL,
+			ACTIVE:       PARTIAL,
+			UNDEPLOYABLE: UNDEPLOYABLE,
+		},
+		PARTIAL: {
+			UNDEFINED:    UNDEFINED,
+			INACTIVE:     PARTIAL,
+			PARTIAL:      PARTIAL,
+			ACTIVE:       PARTIAL,
+			UNDEPLOYABLE: UNDEPLOYABLE,
+		},
+		ACTIVE: {
+			UNDEFINED:    UNDEFINED,
+			INACTIVE:     PARTIAL,
+			PARTIAL:      PARTIAL,
+			ACTIVE:       ACTIVE,
+			UNDEPLOYABLE: UNDEPLOYABLE,
+		},
+		UNDEPLOYABLE: {
+			UNDEFINED:    UNDEFINED,
+			INACTIVE:     UNDEPLOYABLE,
+			PARTIAL:      UNDEPLOYABLE,
+			ACTIVE:       UNDEPLOYABLE,
+			UNDEPLOYABLE: UNDEPLOYABLE,
+		},
+	}
+)
 
 func (s Status) String() string {
 	names := []string{
@@ -93,9 +81,8 @@ func (s Status) String() string {
 		"PARTIAL",
 		"ACTIVE",
 		"UNDEPLOYABLE",
-		"INVARIANT",
 	}
-	if s > INVARIANT {
+	if s > UNDEPLOYABLE {
 		return "UNDEFINED"
 	}
 	return names[s]

core/workflow/safestatus.go

@@ -25,78 +25,52 @@
 package workflow
 
 import (
-	"strconv"
 	"strings"
 	"sync"
 
 	"github.com/AliceO2Group/Control/core/task"
 	"github.com/sirupsen/logrus"
-	"github.com/spf13/viper"
 )
 
 type SafeStatus struct {
 	mu     sync.RWMutex
 	status task.Status
 }
 
-func reportTraceRoles(roles []Role, status task.Status) {
-	if viper.GetBool("veryVerbose") {
-		stati := make([]string, len(roles))
-		critical := make([]string, len(roles))
-		names := make([]string, len(roles))
-		for i, role := range roles {
-			stati[i] = role.GetStatus().String()
-			names[i] = role.GetName()
-			if taskR, isTaskRole := role.(*taskRole); isTaskRole {
-				critical[i] = strconv.FormatBool(taskR.IsCritical())
-			} else if callR, isCallRole := role.(*callRole); isCallRole {
-				critical[i] = strconv.FormatBool(callR.IsCritical())
-			} else {
-				critical[i] = strconv.FormatBool(true)
-			}
-		}
-		log.WithFields(logrus.Fields{
-			"statuses":   strings.Join(stati, ", "),
-			"critical":   strings.Join(critical, ", "),
-			"names":      strings.Join(names, ", "),
-			"aggregated": status.String(),
-		}).
-			Trace("aggregating statuses")
-	}
-}
-
-// role that are not taskRole or callRole are critical by default
 func aggregateStatus(roles []Role) (status task.Status) {
 	if len(roles) == 0 {
 		status = task.UNDEFINED
 		return
 	}
+	stati := make([]string, len(roles))
+	for i, role := range roles {
+		stati[i] = role.GetStatus().String()
+	}
 
-	status = task.INVARIANT
-	for _, role := range roles {
-		if status == task.UNDEFINED {
-			break
-		}
+	status = roles[0].GetStatus()
+	if len(roles) > 1 {
+		for _, c := range roles[1:] {
+			if status == task.UNDEFINED {
+				log.WithFields(logrus.Fields{
+					"statuses":   strings.Join(stati, ", "),
+					"aggregated": status.String(),
+				}).
+					Trace("aggregating statuses")
 
-		if taskR, isTaskRole := role.(*taskRole); isTaskRole {
-			if !taskR.IsCritical() {
-				continue
-			}
-		} else if callR, isCallRole := role.(*callRole); isCallRole {
-			if !callR.IsCritical() {
-				continue
+				return
 			}
+			status = status.X(c.GetStatus())
 		}
-		status = status.X(role.GetStatus())
 	}
-
-	reportTraceRoles(roles, status)
+	log.WithFields(logrus.Fields{
+		"statuses":   strings.Join(stati, ", "),
+		"aggregated": status.String(),
+	}).
+		Trace("aggregating statuses")
 
 	return
 }
 
-// TODO: this function is prime candidate for refactoring. The reason being that it mostly ignores status argument
-// for merging, moreover it also does not use status of role from argument. Both of these behaivour are counter-intuitive.
 func (t *SafeStatus) merge(s task.Status, r Role) {
 	t.mu.Lock()
 	defer t.mu.Unlock()