Skip to content

Commit 2b2ba5d

Browse files
pohlyoxxenixbart0sh
committed
adapt implementation to new API
The structured parameter allocation logic was written from scratch in staging/src/k8s.io/dynamic-resource-allocation/structured. Besides the new features (amount, admin access) and API it now supports backtracking when the initial device selection doesn't lead to a complete allocation of all claims. DRA: use VAP to control "admin access" permissions The advantages of using a validation admission policy (VAP) are that no changes are needed in Kubernetes and that admins have full flexibility if and how they want to control which users are allowed to use "admin access" in their requests. The downside is that without admins taking actions, the feature is enabled out-of-the-box in a cluster. Documentation for DRA will have to make it very clear that something needs to be done in multi-tenant clusters. The test/e2e/testing-manifests/dra/admin-access-policy.yaml shows how to do this. The corresponding E2E tests ensures that it actually works as intended. For some reason, adding the namespace to the message expression leads to a type check errors, so it's currently commented out. DRA kubelet: always call NodePrepareResources, even if not used This could be useful for drivers where that call has some effect other than injecting CDI device IDs into containers. DRA: remove obsolete support for deterministic claim name Now only the claim name as recorded in the pod status matters, which can be looked up without listing claims. DRA kubelet: extend and clean up logging Because of a faulty E2E test, kubelet was told to contact the wrong driver for a claim. This was not visible in the kubelet log output. Now changes to the claim info cache are getting logged. While at it, naming of variables and some existing log output gets harmonized. Co-authored-by: Oksana Baranova <oksana.baranova@intel.com> Co-authored-by: Ed Bartosh <eduard.bartosh@intel.com>
1 parent dbe9a14 commit 2b2ba5d

File tree

79 files changed

+5054
-6872
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+5054
-6872
lines changed

‎pkg/apis/resource/validation/validation.go‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ func validatePoolName(name string, fldPath *field.Path) field.ErrorList {
5555
}
5656
parts := strings.Split(name, "/")
5757
for i, part := range parts {
58-
allErrs = append(allErrs, corevalidation.ValidateDNS1123Subdomain(part, fldPath.Index(i))...)
58+
idxPath := fldPath
59+
if len(parts) > 1 {
60+
idxPath = idxPath.Index(i)
61+
}
62+
allErrs = append(allErrs, corevalidation.ValidateDNS1123Subdomain(part, idxPath)...)
5963
}
6064
}
6165
return allErrs
@@ -72,7 +76,9 @@ func ValidateResourceClaim(resourceClaim *resource.ResourceClaim) field.ErrorLis
7276
func ValidateResourceClaimUpdate(resourceClaim, oldClaim *resource.ResourceClaim) field.ErrorList {
7377
allErrs := corevalidation.ValidateObjectMetaUpdate(&resourceClaim.ObjectMeta, &oldClaim.ObjectMeta, field.NewPath("metadata"))
7478
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(resourceClaim.Spec, oldClaim.Spec, field.NewPath("spec"))...)
75-
// Not really necessary as the spec must have been valid before and is immutable, but let's check anyway.
79+
// Because the spec is immutable, all CEL expressions in it must have been stored.
80+
// If the user tries an update, this is not true and checking is less strict, but
81+
// as there are errors, it doesn't matter.
7682
allErrs = append(allErrs, validateResourceClaimSpec(&resourceClaim.Spec, field.NewPath("spec"), true)...)
7783
return allErrs
7884
}
@@ -352,7 +358,7 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati
352358
var allErrs field.ErrorList
353359
allErrs = append(allErrs, validateRequestNameRef(result.Request, fldPath.Child("request"), requestNames)...)
354360
allErrs = append(allErrs, validateDriverName(result.Driver, fldPath.Child("driver"))...)
355-
allErrs = append(allErrs, validatePoolName(result.Driver, fldPath.Child("pool"))...)
361+
allErrs = append(allErrs, validatePoolName(result.Pool, fldPath.Child("pool"))...)
356362
allErrs = append(allErrs, validateDeviceName(result.Device, fldPath.Child("device"))...)
357363
return allErrs
358364
}
@@ -468,15 +474,10 @@ func validatePodSchedulingClaim(status resource.ResourceClaimSchedulingStatus, f
468474
return allErrs
469475
}
470476

471-
// ValidateClaimTemplace validates a ResourceClaimTemplate.
477+
// ValidateResourceClaimTemplate validates a ResourceClaimTemplate.
472478
func ValidateResourceClaimTemplate(template *resource.ResourceClaimTemplate) field.ErrorList {
473-
var allErrs field.ErrorList
474-
return allErrs
475-
}
476-
477-
func validateResourceClaimTemplate(template *resource.ResourceClaimTemplate, stored bool) field.ErrorList {
478479
allErrs := corevalidation.ValidateObjectMeta(&template.ObjectMeta, true, corevalidation.ValidateResourceClaimTemplateName, field.NewPath("metadata"))
479-
allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, field.NewPath("spec"), stored)...)
480+
allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, field.NewPath("spec"), false)...)
480481
return allErrs
481482
}
482483

@@ -490,7 +491,10 @@ func validateResourceClaimTemplateSpec(spec *resource.ResourceClaimTemplateSpec,
490491
func ValidateResourceClaimTemplateUpdate(template, oldTemplate *resource.ResourceClaimTemplate) field.ErrorList {
491492
allErrs := corevalidation.ValidateObjectMetaUpdate(&template.ObjectMeta, &oldTemplate.ObjectMeta, field.NewPath("metadata"))
492493
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(template.Spec, oldTemplate.Spec, field.NewPath("spec"))...)
493-
allErrs = append(allErrs, ValidateResourceClaimTemplate(template)...)
494+
// Because the spec is immutable, all CEL expressions in it must have been stored.
495+
// If the user tries an update, this is not true and checking is less strict, but
496+
// as there are errors, it doesn't matter.
497+
allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, field.NewPath("spec"), true)...)
494498
return allErrs
495499
}
496500

@@ -518,9 +522,6 @@ func ValidateResourceSliceUpdate(resourceSlice, oldResourceSlice *resource.Resou
518522

519523
func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPath *field.Path) field.ErrorList {
520524
var allErrs field.ErrorList
521-
if spec.NodeName != "" {
522-
allErrs = append(allErrs, validateNodeName(spec.NodeName, fldPath.Child("nodeName"))...)
523-
}
524525
allErrs = append(allErrs, validateDriverName(spec.Driver, fldPath.Child("driver"))...)
525526
allErrs = append(allErrs, validateResourcePool(spec.Pool, fldPath.Child("pool"))...)
526527
nodeSelectionFields := sets.New[string]()
@@ -556,7 +557,7 @@ func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPat
556557

557558
func validateResourcePool(pool resource.ResourcePool, fldPath *field.Path) field.ErrorList {
558559
var allErrs field.ErrorList
559-
allErrs = append(allErrs, validatePoolName(pool.Name, fldPath.Child("pool"))...)
560+
allErrs = append(allErrs, validatePoolName(pool.Name, fldPath.Child("name"))...)
560561
if pool.ResourceSliceCount <= 0 {
561562
allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceSliceCount"), pool.ResourceSliceCount, "must be greater than zero"))
562563
}
@@ -605,7 +606,7 @@ var (
605606
`$`)
606607
)
607608

608-
func validateDeviceAttributes(attribute resource.DeviceAttribute, fldPath *field.Path) field.ErrorList {
609+
func validateDeviceAttribute(attribute resource.DeviceAttribute, fldPath *field.Path) field.ErrorList {
609610
var allErrs field.ErrorList
610611
numFields := 0
611612
if attribute.BoolValue != nil {
@@ -635,11 +636,6 @@ func validateDeviceAttributes(attribute resource.DeviceAttribute, fldPath *field
635636
return allErrs
636637
}
637638

638-
func validateDeviceAttribute(attribute resource.DeviceAttribute, fldPath *field.Path) field.ErrorList {
639-
var allErrs field.ErrorList
640-
return allErrs
641-
}
642-
643639
func validateQuantity(quantity apiresource.Quantity, fldPath *field.Path) field.ErrorList {
644640
// Any parsed quantity is valid.
645641
return nil
@@ -722,6 +718,8 @@ func validateSet[T any, K comparable](slice []T, maxSize int, validateItem func(
722718
}
723719
if allItems.Has(key) {
724720
allErrs = append(allErrs, field.Duplicate(childPath, key))
721+
} else {
722+
allItems.Insert(key)
725723
}
726724
}
727725
return allErrs

‎pkg/apis/resource/validation/validation_deviceclass_test.go‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package validation
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
"k8s.io/apimachinery/pkg/util/validation/field"
26+
"k8s.io/kubernetes/pkg/apis/core"
27+
"k8s.io/kubernetes/pkg/apis/resource"
28+
"k8s.io/utils/pointer"
29+
)
30+
31+
func testClass(name string) *resource.DeviceClass {
32+
return &resource.DeviceClass{
33+
ObjectMeta: metav1.ObjectMeta{
34+
Name: name,
35+
},
36+
}
37+
}
38+
39+
func TestValidateClass(t *testing.T) {
40+
goodName := "foo"
41+
now := metav1.Now()
42+
badName := "!@#$%^"
43+
badValue := "spaces not allowed"
44+
45+
scenarios := map[string]struct {
46+
class *resource.DeviceClass
47+
wantFailures field.ErrorList
48+
}{
49+
"good-class": {
50+
class: testClass(goodName),
51+
},
52+
"missing-name": {
53+
wantFailures: field.ErrorList{field.Required(field.NewPath("metadata", "name"), "name or generateName is required")},
54+
class: testClass(""),
55+
},
56+
"bad-name": {
57+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("metadata", "name"), badName, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")},
58+
class: testClass(badName),
59+
},
60+
"generate-name": {
61+
class: func() *resource.DeviceClass {
62+
class := testClass(goodName)
63+
class.GenerateName = "pvc-"
64+
return class
65+
}(),
66+
},
67+
"uid": {
68+
class: func() *resource.DeviceClass {
69+
class := testClass(goodName)
70+
class.UID = "ac051fac-2ead-46d9-b8b4-4e0fbeb7455d"
71+
return class
72+
}(),
73+
},
74+
"resource-version": {
75+
class: func() *resource.DeviceClass {
76+
class := testClass(goodName)
77+
class.ResourceVersion = "1"
78+
return class
79+
}(),
80+
},
81+
"generation": {
82+
class: func() *resource.DeviceClass {
83+
class := testClass(goodName)
84+
class.Generation = 100
85+
return class
86+
}(),
87+
},
88+
"creation-timestamp": {
89+
class: func() *resource.DeviceClass {
90+
class := testClass(goodName)
91+
class.CreationTimestamp = now
92+
return class
93+
}(),
94+
},
95+
"deletion-grace-period-seconds": {
96+
class: func() *resource.DeviceClass {
97+
class := testClass(goodName)
98+
class.DeletionGracePeriodSeconds = pointer.Int64(10)
99+
return class
100+
}(),
101+
},
102+
"owner-references": {
103+
class: func() *resource.DeviceClass {
104+
class := testClass(goodName)
105+
class.OwnerReferences = []metav1.OwnerReference{
106+
{
107+
APIVersion: "v1",
108+
Kind: "pod",
109+
Name: "foo",
110+
UID: "ac051fac-2ead-46d9-b8b4-4e0fbeb7455d",
111+
},
112+
}
113+
return class
114+
}(),
115+
},
116+
"finalizers": {
117+
class: func() *resource.DeviceClass {
118+
class := testClass(goodName)
119+
class.Finalizers = []string{
120+
"example.com/foo",
121+
}
122+
return class
123+
}(),
124+
},
125+
"managed-fields": {
126+
class: func() *resource.DeviceClass {
127+
class := testClass(goodName)
128+
class.ManagedFields = []metav1.ManagedFieldsEntry{
129+
{
130+
FieldsType: "FieldsV1",
131+
Operation: "Apply",
132+
APIVersion: "apps/v1",
133+
Manager: "foo",
134+
},
135+
}
136+
return class
137+
}(),
138+
},
139+
"good-labels": {
140+
class: func() *resource.DeviceClass {
141+
class := testClass(goodName)
142+
class.Labels = map[string]string{
143+
"apps.kubernetes.io/name": "test",
144+
}
145+
return class
146+
}(),
147+
},
148+
"bad-labels": {
149+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("metadata", "labels"), badValue, "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')")},
150+
class: func() *resource.DeviceClass {
151+
class := testClass(goodName)
152+
class.Labels = map[string]string{
153+
"hello-world": badValue,
154+
}
155+
return class
156+
}(),
157+
},
158+
"good-annotations": {
159+
class: func() *resource.DeviceClass {
160+
class := testClass(goodName)
161+
class.Annotations = map[string]string{
162+
"foo": "bar",
163+
}
164+
return class
165+
}(),
166+
},
167+
"bad-annotations": {
168+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations"), badName, "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')")},
169+
class: func() *resource.DeviceClass {
170+
class := testClass(goodName)
171+
class.Annotations = map[string]string{
172+
badName: "hello world",
173+
}
174+
return class
175+
}(),
176+
},
177+
"invalid-node-selector": {
178+
wantFailures: field.ErrorList{field.Required(field.NewPath("suitableNodes", "nodeSelectorTerms"), "must have at least one node selector term")},
179+
class: func() *resource.DeviceClass {
180+
class := testClass(goodName)
181+
class.Spec.SuitableNodes = &core.NodeSelector{
182+
// Must not be empty.
183+
}
184+
return class
185+
}(),
186+
},
187+
"valid-node-selector": {
188+
class: func() *resource.DeviceClass {
189+
class := testClass(goodName)
190+
class.Spec.SuitableNodes = &core.NodeSelector{
191+
NodeSelectorTerms: []core.NodeSelectorTerm{{
192+
MatchExpressions: []core.NodeSelectorRequirement{{
193+
Key: "foo",
194+
Operator: core.NodeSelectorOpDoesNotExist,
195+
}},
196+
}},
197+
}
198+
return class
199+
}(),
200+
},
201+
}
202+
203+
for name, scenario := range scenarios {
204+
t.Run(name, func(t *testing.T) {
205+
errs := ValidateDeviceClass(scenario.class)
206+
assert.Equal(t, scenario.wantFailures, errs)
207+
})
208+
}
209+
}
210+
211+
func TestValidateClassUpdate(t *testing.T) {
212+
validClass := testClass(goodName)
213+
214+
scenarios := map[string]struct {
215+
oldClass *resource.DeviceClass
216+
update func(class *resource.DeviceClass) *resource.DeviceClass
217+
wantFailures field.ErrorList
218+
}{
219+
"valid-no-op-update": {
220+
oldClass: validClass,
221+
update: func(class *resource.DeviceClass) *resource.DeviceClass { return class },
222+
},
223+
"update-node-selector": {
224+
oldClass: validClass,
225+
update: func(class *resource.DeviceClass) *resource.DeviceClass {
226+
class = class.DeepCopy()
227+
class.Spec.SuitableNodes = &core.NodeSelector{
228+
NodeSelectorTerms: []core.NodeSelectorTerm{{
229+
MatchExpressions: []core.NodeSelectorRequirement{{
230+
Key: "foo",
231+
Operator: core.NodeSelectorOpDoesNotExist,
232+
}},
233+
}},
234+
}
235+
return class
236+
},
237+
},
238+
}
239+
240+
for name, scenario := range scenarios {
241+
t.Run(name, func(t *testing.T) {
242+
scenario.oldClass.ResourceVersion = "1"
243+
errs := ValidateDeviceClassUpdate(scenario.update(scenario.oldClass.DeepCopy()), scenario.oldClass)
244+
assert.Equal(t, scenario.wantFailures, errs)
245+
})
246+
}
247+
}

0 commit comments

Comments
 (0)