From d56c7cff7e54df8d3c0ebd48d6e3c78397a56ee2 Mon Sep 17 00:00:00 2001 From: Mykhailo Bobrovskyi Date: Mon, 2 Dec 2024 17:01:48 +0200 Subject: [PATCH] TAS: validate that kubernetes.io/hostname can only be at the lowest level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michał Woźniak --- apis/kueue/v1alpha1/tas_types.go | 4 +++- .../crd/kueue.x-k8s.io_topologies.yaml | 9 ++++++++- .../crd/bases/kueue.x-k8s.io_topologies.yaml | 9 ++++++++- test/integration/tas/tas_test.go | 18 ++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/apis/kueue/v1alpha1/tas_types.go b/apis/kueue/v1alpha1/tas_types.go index 1d9d138f3c..d881487990 100644 --- a/apis/kueue/v1alpha1/tas_types.go +++ b/apis/kueue/v1alpha1/tas_types.go @@ -73,7 +73,9 @@ type TopologySpec struct { // +listType=atomic // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=8 - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="the levels field is immutable" + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="field is immutable" + // +kubebuilder:validation:XValidation:rule="size(self.filter(i, size(self.filter(j, j == i)) > 1)) == 0",message="must be unique" + // +kubebuilder:validation:XValidation:rule="size(self.filter(i, i.nodeLabel == 'kubernetes.io/hostname')) == 0 || self[size(self) - 1].nodeLabel == 'kubernetes.io/hostname'",message="the kubernetes.io/hostname label can only be used at the lowest level of topology" Levels []TopologyLevel `json:"levels,omitempty"` } diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_topologies.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_topologies.yaml index 648416a014..d9b150168c 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_topologies.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_topologies.yaml @@ -79,8 +79,15 @@ spec: type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: the levels field is immutable + - message: field is immutable rule: self == oldSelf + - message: must be unique + rule: size(self.filter(i, size(self.filter(j, j == i)) > 1)) == + 0 + - message: the kubernetes.io/hostname label can only be used at the + lowest level of topology + rule: size(self.filter(i, i.nodeLabel == 'kubernetes.io/hostname')) + == 0 || self[size(self) - 1].nodeLabel == 'kubernetes.io/hostname' required: - levels type: object diff --git a/config/components/crd/bases/kueue.x-k8s.io_topologies.yaml b/config/components/crd/bases/kueue.x-k8s.io_topologies.yaml index 4438f4cef9..b770fcd8fc 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_topologies.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_topologies.yaml @@ -64,8 +64,15 @@ spec: type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: the levels field is immutable + - message: field is immutable rule: self == oldSelf + - message: must be unique + rule: size(self.filter(i, size(self.filter(j, j == i)) > 1)) == + 0 + - message: the kubernetes.io/hostname label can only be used at the + lowest level of topology + rule: size(self.filter(i, i.nodeLabel == 'kubernetes.io/hostname')) + == 0 || self[size(self) - 1].nodeLabel == 'kubernetes.io/hostname' required: - levels type: object diff --git a/test/integration/tas/tas_test.go b/test/integration/tas/tas_test.go index 5441ea2e2b..0010a85629 100644 --- a/test/integration/tas/tas_test.go +++ b/test/integration/tas/tas_test.go @@ -979,6 +979,18 @@ var _ = ginkgo.Describe("Topology validations", func() { ginkgo.Entry("invalid levels", testing.MakeTopology("invalid-level").Levels([]string{"@invalid"}).Obj(), testing.BeInvalidError()), + ginkgo.Entry("non-unique levels", + testing.MakeTopology("default").Levels([]string{tasBlockLabel, tasBlockLabel}).Obj(), + testing.BeInvalidError()), + ginkgo.Entry("kubernetes.io/hostname first", + testing.MakeTopology("default").Levels([]string{corev1.LabelHostname, tasBlockLabel, tasRackLabel}).Obj(), + testing.BeInvalidError()), + ginkgo.Entry("kubernetes.io/hostname middle", + testing.MakeTopology("default").Levels([]string{tasBlockLabel, corev1.LabelHostname, tasRackLabel}).Obj(), + testing.BeInvalidError()), + ginkgo.Entry("kubernetes.io/hostname last", + testing.MakeTopology("default").Levels([]string{tasBlockLabel, tasRackLabel, corev1.LabelHostname}).Obj(), + gomega.Succeed()), ) }) @@ -1011,6 +1023,12 @@ var _ = ginkgo.Describe("Topology validations", func() { }) }, testing.BeInvalidError()), + ginkgo.Entry("updating levels order is prohibited", + testing.MakeTopology("default").Levels([]string{tasRackLabel, tasBlockLabel, corev1.LabelHostname}).Obj(), + func(topology *kueuealpha.Topology) { + topology.Spec.Levels[0], topology.Spec.Levels[1] = topology.Spec.Levels[1], topology.Spec.Levels[0] + }, + testing.BeInvalidError()), ) }) })