Add spec.groups field with typed group memberships (#292)

The Cortex Placement Shim replaces OpenStack Placement for kvm and reads scheduling information directly from the Hypervisor CRD. This is an opportunity to rethink how traits and aggregates are modeled on the CRD.

Today, trait and aggregate data is split across spec and status. Controllers reconcile between the two, merging sources and pushing state to OpenStack Placement. With the shim reading the CRD directly, there is no external system to observe or merge from, making this indirection unnecessary. The existing spec fields are also flat string lists tied to OpenStack naming, unable to carry the structured data like UUIDs and metadata that the shim needs to serve a compatible Placement API.

Both traits and aggregates are forms of grouping: traits group hypervisors by capability, aggregates group them by administrative assignment. This proposal unifies them under a new spec.groups field as the single source of truth. Rather than using Kubernetes labels, which are flat key-value strings without schema validation, spec.groups models each group kind as a well-typed substruct that can carry structured data such as UUIDs and metadata.

The field is a list of typed group entries following the field-presence union pattern from core Kubernetes, as used by PodSpec.volumes. Each entry populates exactly one type-specific sub-field:

```yaml
spec:
  groups:
    - trait: {name: COMPUTE_STATUS_DISABLED}
    - trait: {name: CUSTOM_TRAIT_FOO}
    - aggregate:
        name: fast-storage
        uuid: abc-123
        metadata:
          ssd: "true"
```

Each group type has its own struct with only the fields relevant to it. A CEL validation rule enforces that exactly one sub-field is populated per entry. The existing spec.customTraits and spec.aggregates fields remain untouched to not interfere with existing controller logic, separating this api change from the controller refactor. They can be deprecated and removed in a future release once the shim is fully integrated.

Library helper functions like HasTrait, GetTraits, HasAggregate, and GetAggregates follow the meta.IsStatusConditionTrue pattern and are reusable across the shim, operator, and scheduler.

The design is extensible: adding a new grouping concept means adding one optional pointer field to the Group struct and a corresponding type-specific struct. No changes to existing fields or consumers are required.

Author: PhilippMatthes

api/v1/hypervisor_types.go

@@ -132,6 +132,25 @@ type HypervisorSpec struct {
 	// Aggregates are used to apply aggregates to the hypervisor.
 	Aggregates []string `json:"aggregates"`
 
+	// Groups defines typed group memberships for this hypervisor.
+	//
+	// Both traits and aggregates are forms of grouping: traits group
+	// hypervisors by capability, aggregates group them by administrative
+	// assignment. Each entry follows the field-presence union pattern
+	// (as used by PodSpec.volumes in core Kubernetes): exactly one
+	// type-specific sub-field must be populated per entry.
+	//
+	// The Cortex Placement shim and scheduler read group memberships
+	// directly from this field.
+	//
+	// Note: uniqueness of trait names and aggregate UUIDs is not enforced
+	// via CEL because the required O(n^2) comparison exceeds the
+	// Kubernetes CEL cost budget. Enforce uniqueness in the consuming
+	// controller or via a validating webhook if needed.
+	//
+	// +kubebuilder:validation:Optional
+	Groups []Group `json:"groups,omitempty"`
+
 	// +kubebuilder:default:={}
 	// AllowedProjects defines which openstack projects are allowed to schedule
 	// instances on this hypervisor. The values of this list should be project
@@ -212,6 +231,84 @@ type Aggregate struct {
 	Metadata map[string]string `json:"metadata,omitempty"`
 }
 
+// TraitGroup represents a capability trait, such as an OpenStack
+// Placement trait (e.g. HW_CPU_X86_AVX2, COMPUTE_STATUS_DISABLED).
+type TraitGroup struct {
+	// +kubebuilder:validation:MinLength=1
+	Name string `json:"name"`
+}
+
+// AggregateGroup represents an administrative grouping, such as an
+// OpenStack host aggregate.
+type AggregateGroup struct {
+	// +kubebuilder:validation:MinLength=1
+	Name string `json:"name"`
+
+	// +kubebuilder:validation:Pattern=`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$`
+	UUID string `json:"uuid"`
+
+	// +kubebuilder:validation:Optional
+	Metadata map[string]string `json:"metadata,omitempty"`
+}
+
+// Group is a typed group membership entry for a hypervisor.
+//
+// This follows the field-presence union pattern (as used by
+// PodSpec.volumes in core Kubernetes): each entry populates exactly
+// one type-specific sub-field, and the populated field identifies
+// the group type.
+//
+// +kubebuilder:validation:XValidation:rule="(has(self.trait) ? 1 : 0) + (has(self.aggregate) ? 1 : 0) == 1",message="exactly one group type must be set"
+type Group struct {
+	// +kubebuilder:validation:Optional
+	Trait *TraitGroup `json:"trait,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	Aggregate *AggregateGroup `json:"aggregate,omitempty"`
+}
+
+// HasTrait reports whether groups contains a trait entry with the given name.
+func HasTrait(groups []Group, name string) bool {
+	for _, g := range groups {
+		if g.Trait != nil && g.Trait.Name == name {
+			return true
+		}
+	}
+	return false
+}
+
+// GetTraits returns all TraitGroup entries from groups.
+func GetTraits(groups []Group) []TraitGroup {
+	var out []TraitGroup
+	for _, g := range groups {
+		if g.Trait != nil {
+			out = append(out, *g.Trait)
+		}
+	}
+	return out
+}
+
+// HasAggregate reports whether groups contains an aggregate entry with the given UUID.
+func HasAggregate(groups []Group, uuid string) bool {
+	for _, g := range groups {
+		if g.Aggregate != nil && g.Aggregate.UUID == uuid {
+			return true
+		}
+	}
+	return false
+}
+
+// GetAggregates returns all AggregateGroup entries from groups.
+func GetAggregates(groups []Group) []AggregateGroup {
+	var out []AggregateGroup
+	for _, g := range groups {
+		if g.Aggregate != nil {
+			out = append(out, *g.Aggregate)
+		}
+	}
+	return out
+}
+
 type HyperVisorUpdateStatus struct {
 	// +kubebuilder:default:=false
 	// Represents a running Operating System update.

api/v1/hypervisor_validation_test.go

@@ -18,6 +18,8 @@ limitations under the License.
 package v1
 
 import (
+	"fmt"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -396,3 +398,263 @@ var _ = Describe("MaintenanceReason CEL Validation", func() {
 		})
 	})
 })
+
+// TestGroupsCELValidation tests the CEL validation rules for spec.groups:
+// 1. Exactly one group type must be set per entry (union rule on Group)
+// 2. Field-level validation (minLength) on trait name, aggregate name, and aggregate UUID
+var _ = Describe("Groups CEL Validation", func() {
+	var (
+		hypervisor     *Hypervisor
+		hypervisorName types.NamespacedName
+		counter        int
+	)
+
+	BeforeEach(func(ctx SpecContext) {
+		counter++
+		hypervisorName = types.NamespacedName{
+			Name: fmt.Sprintf("test-groups-hv-%d", counter),
+		}
+	})
+
+	AfterEach(func(ctx SpecContext) {
+		if hypervisor != nil {
+			Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, hypervisor))).To(Succeed())
+			hypervisor = nil
+		}
+	})
+
+	Context("Union rule: exactly one group type per entry", func() {
+		It("should accept a group with only trait set", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Trait: &TraitGroup{Name: "HW_CPU_X86_AVX2"}},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should accept a group with only aggregate set", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should accept mixed trait and aggregate entries", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Trait: &TraitGroup{Name: "HW_CPU_X86_AVX2"}},
+						{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
+						{Trait: &TraitGroup{Name: "COMPUTE_STATUS_DISABLED"}},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should reject a group with both trait and aggregate set", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{
+							Trait:     &TraitGroup{Name: "HW_CPU_X86_AVX2"},
+							Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"},
+						},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, hypervisor)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("exactly one group type must be set"))
+		})
+
+		It("should reject a group with neither trait nor aggregate set", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, hypervisor)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("exactly one group type must be set"))
+		})
+	})
+
+	Context("Field validation", func() {
+		It("should reject a trait with empty name", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Trait: &TraitGroup{Name: ""}},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, hypervisor)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("spec.groups[0].trait.name"))
+		})
+
+		It("should reject an aggregate with empty name", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Aggregate: &AggregateGroup{Name: "", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, hypervisor)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("spec.groups[0].aggregate.name"))
+		})
+
+		It("should reject an aggregate with empty UUID", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: ""}},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, hypervisor)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("spec.groups[0].aggregate.uuid"))
+		})
+
+		It("should accept an aggregate without metadata", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should accept an aggregate with metadata", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{
+						{Aggregate: &AggregateGroup{
+							Name:     "fast-storage",
+							UUID:     "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11",
+							Metadata: map[string]string{"ssd": "true"},
+						}},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+
+			created := &Hypervisor{}
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(hypervisor), created)).To(Succeed())
+			Expect(created.Spec.Groups).To(HaveLen(1))
+			Expect(created.Spec.Groups[0].Aggregate).NotTo(BeNil())
+			Expect(created.Spec.Groups[0].Aggregate.Metadata).To(HaveKeyWithValue("ssd", "true"))
+		})
+
+		It("should accept an empty groups list", func(ctx SpecContext) {
+			hypervisor = &Hypervisor{
+				ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
+				Spec: HypervisorSpec{
+					Groups: []Group{},
+				},
+			}
+			Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
+		})
+	})
+})
+
+var _ = Describe("Group Helper Functions", func() {
+	groups := []Group{
+		{Trait: &TraitGroup{Name: "HW_CPU_X86_AVX2"}},
+		{Trait: &TraitGroup{Name: "COMPUTE_STATUS_DISABLED"}},
+		{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", Metadata: map[string]string{"ssd": "true"}}},
+		{Aggregate: &AggregateGroup{Name: "slow-storage", UUID: "b1ffbc99-9c0b-4ef8-bb6d-6bb9bd380a22"}},
+	}
+
+	Context("HasTrait", func() {
+		It("should return true for an existing trait", func() {
+			Expect(HasTrait(groups, "HW_CPU_X86_AVX2")).To(BeTrue())
+		})
+
+		It("should return false for a missing trait", func() {
+			Expect(HasTrait(groups, "NONEXISTENT")).To(BeFalse())
+		})
+
+		It("should return false for an empty list", func() {
+			Expect(HasTrait(nil, "HW_CPU_X86_AVX2")).To(BeFalse())
+		})
+	})
+
+	Context("GetTraits", func() {
+		It("should return all trait entries", func() {
+			traits := GetTraits(groups)
+			Expect(traits).To(HaveLen(2))
+			Expect(traits[0].Name).To(Equal("HW_CPU_X86_AVX2"))
+			Expect(traits[1].Name).To(Equal("COMPUTE_STATUS_DISABLED"))
+		})
+
+		It("should return empty for a list with no traits", func() {
+			aggs := []Group{{Aggregate: &AggregateGroup{Name: "a", UUID: "d0eebc99-9c0b-4ef8-bb6d-6bb9bd380a33"}}}
+			Expect(GetTraits(aggs)).To(BeEmpty())
+		})
+
+		It("should return nil for an empty list", func() {
+			Expect(GetTraits(nil)).To(BeNil())
+		})
+	})
+
+	Context("HasAggregate", func() {
+		It("should return true for an existing aggregate UUID", func() {
+			Expect(HasAggregate(groups, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11")).To(BeTrue())
+		})
+
+		It("should return false for a missing aggregate UUID", func() {
+			Expect(HasAggregate(groups, "c2ffbc99-9c0b-4ef8-bb6d-6bb9bd380a99")).To(BeFalse())
+		})
+
+		It("should return false for an empty list", func() {
+			Expect(HasAggregate(nil, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11")).To(BeFalse())
+		})
+	})
+
+	Context("GetAggregates", func() {
+		It("should return all aggregate entries", func() {
+			aggs := GetAggregates(groups)
+			Expect(aggs).To(HaveLen(2))
+			Expect(aggs[0].Name).To(Equal("fast-storage"))
+			Expect(aggs[0].UUID).To(Equal("a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"))
+			Expect(aggs[0].Metadata).To(HaveKeyWithValue("ssd", "true"))
+			Expect(aggs[1].Name).To(Equal("slow-storage"))
+			Expect(aggs[1].UUID).To(Equal("b1ffbc99-9c0b-4ef8-bb6d-6bb9bd380a22"))
+		})
+
+		It("should return empty for a list with no aggregates", func() {
+			traits := []Group{{Trait: &TraitGroup{Name: "T"}}}
+			Expect(GetAggregates(traits)).To(BeEmpty())
+		})
+
+		It("should return nil for an empty list", func() {
+			Expect(GetAggregates(nil)).To(BeNil())
+		})
+	})
+})