Reapply "[core] INVARIANT status introduced"

Michal Tichák · knopers8 · commit ef77c960a2ef · 2025-05-14T08:30:17.000+02:00
This reverts commit 0a7036b.
diff --git a/core/task/status.go b/core/task/status.go
@@ -32,47 +32,59 @@ 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,
-		},
-		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,
-		},
-	}
-)
+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,
+	},
+}
 
 func (s Status) String() string {
 	names := []string{
@@ -81,8 +93,9 @@ func (s Status) String() string {
 		"PARTIAL",
 		"ACTIVE",
 		"UNDEPLOYABLE",
+		"INVARIANT",
 	}
-	if s > UNDEPLOYABLE {
+	if s > INVARIANT {
 		return "UNDEFINED"
 	}
 	return names[s]
diff --git a/core/task/status_test.go b/core/task/status_test.go
@@ -0,0 +1,110 @@
+/*
+ * === This file is part of ALICE O² ===
+ *
+ * Copyright 2025 CERN and copyright holders of ALICE O².
+ * Author: Teo Mrnjavac <teo.mrnjavac@cern.ch>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * In applying this license CERN does not waive the privileges and
+ * immunities granted to it by virtue of its status as an
+ * Intergovernmental Organization or submit itself to any jurisdiction.
+ */
+
+package task
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("task status", func() {
+	When("product of all statuses with UNDEFINED is done", func() {
+		undefined := Status(UNDEFINED)
+		It("should be UNDEFINED for all", func() {
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(UNDEFINED)))
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(INACTIVE)))
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(PARTIAL)))
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(ACTIVE)))
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(UNDEPLOYABLE)))
+			Expect(Status(UNDEFINED)).To(Equal(undefined.X(INVARIANT)))
+		})
+	})
+	When("product of all statuses with INACTIVE is done", func() {
+		inactive := Status(INACTIVE)
+		It("should have different results", func() {
+			Expect(Status(UNDEFINED)).To(Equal(inactive.X(UNDEFINED)))
+			Expect(Status(INACTIVE)).To(Equal(inactive.X(INACTIVE)))
+			Expect(Status(PARTIAL)).To(Equal(inactive.X(PARTIAL)))
+			Expect(Status(PARTIAL)).To(Equal(inactive.X(ACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(inactive.X(UNDEPLOYABLE)))
+			Expect(Status(INACTIVE)).To(Equal(inactive.X(INVARIANT)))
+		})
+	})
+	When("product of all statuses with PARTIAL is done", func() {
+		partial := Status(PARTIAL)
+		It("should have different results", func() {
+			Expect(Status(UNDEFINED)).To(Equal(partial.X(UNDEFINED)))
+			Expect(Status(PARTIAL)).To(Equal(partial.X(INACTIVE)))
+			Expect(Status(PARTIAL)).To(Equal(partial.X(PARTIAL)))
+			Expect(Status(PARTIAL)).To(Equal(partial.X(ACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(partial.X(UNDEPLOYABLE)))
+			Expect(Status(PARTIAL)).To(Equal(partial.X(INVARIANT)))
+		})
+	})
+	When("product of all statuses with ACTIVE is done", func() {
+		active := Status(ACTIVE)
+		It("should have different results", func() {
+			Expect(Status(UNDEFINED)).To(Equal(active.X(UNDEFINED)))
+			Expect(Status(PARTIAL)).To(Equal(active.X(INACTIVE)))
+			Expect(Status(PARTIAL)).To(Equal(active.X(PARTIAL)))
+			Expect(Status(ACTIVE)).To(Equal(active.X(ACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(active.X(UNDEPLOYABLE)))
+			Expect(Status(ACTIVE)).To(Equal(active.X(INVARIANT)))
+		})
+	})
+	When("product of all statuses with UNDEPLOYABLE is done", func() {
+		undeployable := Status(UNDEPLOYABLE)
+		It("should be UNDEPLOYABLE unless UNDEFINED", func() {
+			Expect(Status(UNDEFINED)).To(Equal(undeployable.X(UNDEFINED)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(undeployable.X(INACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(undeployable.X(PARTIAL)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(undeployable.X(ACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(undeployable.X(UNDEPLOYABLE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(undeployable.X(INVARIANT)))
+		})
+	})
+	When("product of all statuses with INVARIANT is done", func() {
+		invariant := Status(INVARIANT)
+		It("it should be the status the product was done with", func() {
+			Expect(Status(UNDEFINED)).To(Equal(invariant.X(UNDEFINED)))
+			Expect(Status(INACTIVE)).To(Equal(invariant.X(INACTIVE)))
+			Expect(Status(PARTIAL)).To(Equal(invariant.X(PARTIAL)))
+			Expect(Status(ACTIVE)).To(Equal(invariant.X(ACTIVE)))
+			Expect(Status(UNDEPLOYABLE)).To(Equal(invariant.X(UNDEPLOYABLE)))
+			Expect(Status(INVARIANT)).To(Equal(invariant.X(INVARIANT)))
+		})
+	})
+	When("String() of Status is called", func() {
+		It("should return string representation of a status", func() {
+			Expect("UNDEFINED").To(Equal(Status(UNDEFINED).String()))
+			Expect("INACTIVE").To(Equal(Status(INACTIVE).String()))
+			Expect("PARTIAL").To(Equal(Status(PARTIAL).String()))
+			Expect("ACTIVE").To(Equal(Status(ACTIVE).String()))
+			Expect("UNDEPLOYABLE").To(Equal(Status(UNDEPLOYABLE).String()))
+			Expect("INVARIANT").To(Equal(Status(INVARIANT).String()))
+			Expect("UNDEFINED").To(Equal(Status(255).String()))
+		})
+	})
+})
diff --git a/core/task/task_suite_test.go b/core/task/task_suite_test.go
@@ -0,0 +1,37 @@
+/*
+ * === This file is part of ALICE O² ===
+ *
+ * Copyright 2025 CERN and copyright holders of ALICE O².
+ * Author: Teo Mrnjavac <teo.mrnjavac@cern.ch>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * In applying this license CERN does not waive the privileges and
+ * immunities granted to it by virtue of its status as an
+ * Intergovernmental Organization or submit itself to any jurisdiction.
+ */
+
+package task
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestTask(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Task Test Suite")
+}
diff --git a/core/workflow/safestatus.go b/core/workflow/safestatus.go
@@ -25,52 +25,78 @@
 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 = 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")
+	status = task.INVARIANT
+	for _, role := range roles {
+		if status == task.UNDEFINED {
+			break
+		}
 
-				return
+		if taskR, isTaskRole := role.(*taskRole); isTaskRole {
+			if !taskR.IsCritical() {
+				continue
+			}
+		} else if callR, isCallRole := role.(*callRole); isCallRole {
+			if !callR.IsCritical() {
+				continue
 			}
-			status = status.X(c.GetStatus())
 		}
+		status = status.X(role.GetStatus())
 	}
-	log.WithFields(logrus.Fields{
-		"statuses":   strings.Join(stati, ", "),
-		"aggregated": status.String(),
-	}).
-		Trace("aggregating statuses")
+
+	reportTraceRoles(roles, status)
 
 	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()
diff --git a/core/workflow/safestatus_test.go b/core/workflow/safestatus_test.go
