feat: add strict ordering

Author: ayuskauskas

operator/api/v1alpha1/skyhook_types.go

@@ -71,6 +71,11 @@ type SkyhookSpec struct {
 	// This skyhook is required to have been completed before any workloads can start
 	//+kubebuilder:default=false
 	RuntimeRequired bool `json:"runtimeRequired,omitempty"`
+
+	// Priority determines the order in which skyhooks are applied. Lower values are applied first.
+	//+kubebuilder:validation:Minimum=0
+	//+kubebuilder:default=0
+	Priority int `json:"priority,omitempty"`
 }
 
 // BuildGraph turns packages in the a graph of dependencies
@@ -659,3 +664,23 @@ func init() {
 func (s *Skyhook) WasUpdated() bool {
 	return s.Generation > 1 && s.Generation > s.Status.ObservedGeneration
 }
+
+func (s *Skyhook) IsPaused() bool {
+	if s.Annotations == nil {
+		return false
+	}
+	if val, ok := s.Annotations[fmt.Sprintf("%s/pause", METADATA_PREFIX)]; ok {
+		return val == "true"
+	}
+	return false
+}
+
+func (s *Skyhook) IsDisabled() bool {
+	if s.Annotations == nil {
+		return false
+	}
+	if val, ok := s.Annotations[fmt.Sprintf("%s/disable", METADATA_PREFIX)]; ok {
+		return val == "true"
+	}
+	return false
+}

operator/api/v1alpha1/skyhook_types_test.go

@@ -23,6 +23,7 @@ package v1alpha1
 import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var _ = Describe("Skyhook Types", func() {
@@ -552,4 +553,52 @@ var _ = Describe("Skyhook Types", func() {
 		Expect(nodeState.IsPackageComplete(packages["bar"], interrupts, configUpdates)).To(BeEquivalentTo(false))
 	})
 
+	It("Should detect IsPaused correctly", func() {
+		s := &Skyhook{
+			ObjectMeta: metav1.ObjectMeta{},
+		}
+		// Case 1: Annotations is nil
+		Expect(s.IsPaused()).To(BeFalse())
+
+		// Case 2: Annotations is empty map
+		s.Annotations = map[string]string{}
+		Expect(s.IsPaused()).To(BeFalse())
+
+		// Case 3: Key not present
+		s.Annotations = map[string]string{"other": "true"}
+		Expect(s.IsPaused()).To(BeFalse())
+
+		// Case 4: Key present with value "true"
+		s.Annotations = map[string]string{METADATA_PREFIX + "/pause": "true"}
+		Expect(s.IsPaused()).To(BeTrue())
+
+		// Case 5: Key present with value "false"
+		s.Annotations = map[string]string{METADATA_PREFIX + "/pause": "false"}
+		Expect(s.IsPaused()).To(BeFalse())
+	})
+
+	It("Should detect IsDisabled correctly", func() {
+		s := &Skyhook{
+			ObjectMeta: metav1.ObjectMeta{},
+		}
+		// Case 1: Annotations is nil
+		Expect(s.IsDisabled()).To(BeFalse())
+
+		// Case 2: Annotations is empty map
+		s.Annotations = map[string]string{}
+		Expect(s.IsDisabled()).To(BeFalse())
+
+		// Case 3: Key not present
+		s.Annotations = map[string]string{"other": "true"}
+		Expect(s.IsDisabled()).To(BeFalse())
+
+		// Case 4: Key present with value "true"
+		s.Annotations = map[string]string{METADATA_PREFIX + "/disable": "true"}
+		Expect(s.IsDisabled()).To(BeTrue())
+
+		// Case 5: Key present with value "false"
+		s.Annotations = map[string]string{METADATA_PREFIX + "/disable": "false"}
+		Expect(s.IsDisabled()).To(BeFalse())
+	})
+
 })

operator/config/crd/bases/skyhook.nvidia.com_skyhooks.yaml

@@ -488,6 +488,12 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
+              priority:
+                default: 0
+                description: Priority determines the order in which skyhooks are applied.
+                  Lower values are applied first.
+                minimum: 0
+                type: integer
               runtimeRequired:
                 default: false
                 description: This skyhook is required to have been completed before

operator/internal/controller/cluster_state_v2.go

@@ -104,7 +104,13 @@ func BuildState(skyhooks *v1alpha1.SkyhookList, nodes *corev1.NodeList) (*cluste
 		}
 	}
 
+	// Sort by priority (ascending), then by name (ascending) if priorities are equal
 	sort.Slice(ret.skyhooks, func(i, j int) bool {
+		pi := ret.skyhooks[i].skyhook.Spec.Priority
+		pj := ret.skyhooks[j].skyhook.Spec.Priority
+		if pi != pj {
+			return pi < pj
+		}
 		return ret.skyhooks[i].skyhook.Name < ret.skyhooks[j].skyhook.Name
 	})
 
@@ -117,13 +123,26 @@ func BuildState(skyhooks *v1alpha1.SkyhookList, nodes *corev1.NodeList) (*cluste
 	return ret, nil
 }
 
+func GetNextSkyhook(skyhooks []*skyhookNodes) *skyhookNodes {
+	for _, skyhook := range skyhooks {
+		// Move skip and pause to be annotations
+		if skyhook.IsComplete() || skyhook.skyhook.IsDisabled() {
+			continue
+		}
+		return skyhook
+	}
+	return nil
+}
+
 // SkyhookNodes wraps the skyhook and nodes that it pertains too
 type SkyhookNodes interface {
 	CollectNodeStatus() v1alpha1.Status
 	GetSkyhook() *wrapper.Skyhook
 	GetNodes() []wrapper.SkyhookNode
 	GetNode(name string) (v1alpha1.Status, wrapper.SkyhookNode)
 	IsComplete() bool
+	IsDisabled() bool
+	IsPaused() bool
 	NodeCount() int
 	SetStatus(status v1alpha1.Status)
 	Status() v1alpha1.Status
@@ -168,6 +187,14 @@ func (s *skyhookNodes) IsComplete() bool {
 	return true
 }
 
+func (s *skyhookNodes) IsDisabled() bool {
+	return s.skyhook.IsDisabled()
+}
+
+func (s *skyhookNodes) IsPaused() bool {
+	return s.skyhook.IsPaused()
+}
+
 func (s *skyhookNodes) Status() v1alpha1.Status {
 	return s.skyhook.Status.Status
 }

operator/internal/controller/cluster_state_v2_test.go

@@ -24,7 +24,11 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	"github.com/NVIDIA/skyhook/api/v1alpha1"
+	skyhookNodesMock "github.com/NVIDIA/skyhook/internal/controller/mock"
+	"github.com/NVIDIA/skyhook/internal/wrapper"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var _ = Describe("cluster state v2 tests", func() {
@@ -106,3 +110,82 @@ var _ = Describe("cluster state v2 tests", func() {
 		Expect(CheckTaintToleration(tolerations, taints)).To(BeTrue())
 	})
 })
+
+// --- Add GetNextSkyhook tests ---
+
+var _ = Describe("GetNextSkyhook", func() {
+	It("returns the first not-complete, not-disabled skyhook", func() {
+		// Helper to make a skyhookNodes with given complete/disabled
+		makeSkyhookNodes := func(name string, complete bool, disabled bool, paused bool) SkyhookNodes {
+			sn_mock := skyhookNodesMock.MockSkyhookNodes{}
+			sn_mock.EXPECT().IsComplete().Return(complete)
+			sn_mock.EXPECT().IsDisabled().Return(disabled)
+			sn_mock.EXPECT().IsPaused().Return(paused)
+			return &sn_mock
+		}
+
+		// Not complete, not disabled
+		n1 := makeSkyhookNodes("n1", false, false, false)
+		// Complete
+		n2 := makeSkyhookNodes("n2", true, false, false)
+		// Disabled
+		n3 := makeSkyhookNodes("n3", false, true, false)
+
+		// Should return n1
+		result := GetNextSkyhook([]SkyhookNodes{n1, n2, n3})
+		Expect(result).To(Equal(n1))
+
+		// Should skip n1 if complete, return nil if all complete/disabled
+		n1.skyhook.Status.Status = v1alpha1.StatusComplete
+		result = GetNextSkyhook([]*skyhookNodes{n1, n2, n3})
+		Expect(result).To(BeNil())
+
+		// Should skip disabled, return next not-complete, not-disabled
+		n1.skyhook.Status.Status = v1alpha1.StatusUnknown
+		n1.skyhook.Annotations[v1alpha1.METADATA_PREFIX+"/disable"] = "true"
+		n2.skyhook.Status.Status = v1alpha1.StatusUnknown
+		n2.skyhook.Annotations[v1alpha1.METADATA_PREFIX+"/disable"] = "false"
+		result = GetNextSkyhook([]*skyhookNodes{n1, n2, n3})
+		Expect(result).To(Equal(n2))
+	})
+
+	It("returns nil if all skyhooks are complete or disabled", func() {
+		n1 := &skyhookNodes{skyhook: wrapper.NewSkyhookWrapper(&v1alpha1.Skyhook{
+			ObjectMeta: metav1.ObjectMeta{Name: "n1", Annotations: map[string]string{v1alpha1.METADATA_PREFIX + "/disable": "true"}},
+			Status:     v1alpha1.SkyhookStatus{Status: v1alpha1.StatusComplete},
+		})}
+		n2 := &skyhookNodes{skyhook: wrapper.NewSkyhookWrapper(&v1alpha1.Skyhook{
+			ObjectMeta: metav1.ObjectMeta{Name: "n2"},
+			Status:     v1alpha1.SkyhookStatus{Status: v1alpha1.StatusComplete},
+		})}
+		result := GetNextSkyhook([]*skyhookNodes{n1, n2})
+		Expect(result).To(BeNil())
+	})
+})
+
+var _ = Describe("BuildState ordering", func() {
+	It("orders skyhooks by priority and name", func() {
+		priorityKey := v1alpha1.METADATA_PREFIX + "/priority"
+		skyhooks := &v1alpha1.SkyhookList{
+			Items: []v1alpha1.Skyhook{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "b", Annotations: map[string]string{priorityKey: "2"}},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "a", Annotations: map[string]string{priorityKey: "1"}},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "c", Annotations: map[string]string{priorityKey: "2"}},
+				},
+			},
+		}
+		nodes := &corev1.NodeList{Items: []corev1.Node{}}
+		clusterState, err := BuildState(skyhooks, nodes)
+		Expect(err).ToNot(HaveOccurred())
+		ordered := clusterState.skyhooks
+		// Should be: a (priority 1), b (priority 2, name b), c (priority 2, name c)
+		Expect(ordered[0].skyhook.Name).To(Equal("a"))
+		Expect(ordered[1].skyhook.Name).To(Equal("b"))
+		Expect(ordered[2].skyhook.Name).To(Equal("c"))
+	})
+})