[core] INVARIANT status introduced

Michal Tichák · justonedev1 · commit 77bdd3ab2ebf · 2025-04-02T17:00:50.000+02:00
INVARIANT status is introduced to fix OCTRL-948. INVARIANT is
overwritten by any other status in status product (X() function). We are
taking account the criticality of the role when merging. If all roles
are noncritical we are returning INVARIANT. If there is any critical
role(s) we are merging and returning their status, which shouldn't be
INVARIANT. All roles except taskRole and callrole are taken as critical
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
