mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 02:57:27 +09:00
fix(gpu): keep template device names and marks within API limits
A VMI name can be a 253-character DNS subdomain, and worker VMIs
named after Cluster API machines do grow long. Carried as a label
value it fails the 63-character limit, and with a suffix appended it
fails the name limit, so the webhook could not create the template
device at all. The mark is now a plain "true", and a name too long
for the suffix is shortened with a hash of the full VMI name.
The device-reference parser also refused nothing after the JSON list,
so a malformed annotation could pass as valid.
Fixes: a11737a2dd ("feat(gpu): fix the shared GPU keys, settings and binding contract")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
7 files changed
+55
-8
No files matched your search
@@ -31,6 +31,8 @@ type Config struct {
|
||||
// set adds class and agent-reported names (model.ResourceNames).
|
||||
ResourceNames []string
|
||||
|
||||
// RawResourceNames is the hoster's gpu.rawResourceNames; empty means
|
||||
// RawNamesAllowed.
|
||||
RawResourceNames RawNamesPolicy
|
||||
|
||||
// LostCapacityGrace is how long a device stays missing or unhealthy, or
|
||||
|
||||
@@ -31,8 +31,10 @@ const (
|
||||
// to a VMI, so a reinvocation recognises its own entries.
|
||||
VMIGPUNamePrefix = "gpudev-"
|
||||
|
||||
// LabelTemplateDevice marks a GPUDevice the webhook created from a
|
||||
// deviceTemplate entry. Its value is the VMI name. Tenants cannot set it.
|
||||
// LabelTemplateDevice, valued LabelValueTrue, marks a GPUDevice the
|
||||
// webhook created from a deviceTemplate entry. Tenants cannot set it. The
|
||||
// VMI is found through model.TemplateDeviceName, not the label, since a
|
||||
// VMI name can exceed a label value's 63 characters.
|
||||
LabelTemplateDevice = Group + "/template-device"
|
||||
|
||||
// LabelUsageMonitor is the reserved selector key of the controller-owned
|
||||
|
||||
@@ -22,8 +22,8 @@ import (
|
||||
"github.com/cozystack/cozystack/internal/gpu"
|
||||
)
|
||||
|
||||
// Setup is wired into cozystack-controller before any reconciler exists:
|
||||
// it must succeed and touch nothing, so it accepts a nil manager.
|
||||
// Setup may be called before any reconciler exists: it must succeed and
|
||||
// touch nothing, so it accepts a nil manager.
|
||||
func TestSetupWithoutReconcilersIsANoOp(t *testing.T) {
|
||||
if err := Setup(nil, gpu.Config{Enabled: true}); err != nil {
|
||||
t.Fatalf("Setup: %v", err)
|
||||
|
||||
@@ -38,6 +38,10 @@ limitations under the License.
|
||||
// with a Result, and admits the VMI only if all of them are Succeeded
|
||||
// and share one node.
|
||||
//
|
||||
// model.Snapshot.CheckOrder judges one order. To judge a set, such as a
|
||||
// VMI's template devices, add each admitted device to the snapshot as Bound
|
||||
// before checking the next.
|
||||
//
|
||||
// The binder, for a request whose devices all carry the same ID, either
|
||||
// places the whole set on one eligible node with room for all of it, moving
|
||||
// or binding devices as needed, and writes Result Succeeded on each; or
|
||||
|
||||
@@ -18,10 +18,15 @@ package model
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"k8s.io/apimachinery/pkg/util/validation"
|
||||
)
|
||||
|
||||
// DeviceRef is one entry of the gpu.AnnotationDevices list on a VMI:
|
||||
@@ -47,6 +52,9 @@ func ParseDeviceRefs(s string) ([]DeviceRef, error) {
|
||||
if err := dec.Decode(&refs); err != nil {
|
||||
return nil, fmt.Errorf("parse device references: %w", err)
|
||||
}
|
||||
if dec.More() {
|
||||
return nil, errors.New("parse device references: data after the list")
|
||||
}
|
||||
if len(refs) == 0 {
|
||||
return nil, errors.New("device references: empty list")
|
||||
}
|
||||
@@ -80,7 +88,15 @@ func FormatDeviceRefs(refs []DeviceRef) (string, error) {
|
||||
|
||||
// TemplateDeviceName names the GPUDevice created for the deviceTemplate
|
||||
// entry at index of a VMI's references, so a repeated admission or a
|
||||
// recreated VMI finds the same device.
|
||||
// recreated VMI finds the same device. A VMI name too long to take the
|
||||
// suffix is shortened and made unique by a hash of the full name.
|
||||
func TemplateDeviceName(vmi string, index int) string {
|
||||
return vmi + "-gpu-" + strconv.Itoa(index)
|
||||
suffix := "-gpu-" + strconv.Itoa(index)
|
||||
if len(vmi)+len(suffix) <= validation.DNS1123SubdomainMaxLength {
|
||||
return vmi + suffix
|
||||
}
|
||||
sum := sha256.Sum256([]byte(vmi))
|
||||
hash := hex.EncodeToString(sum[:])[:10]
|
||||
keep := validation.DNS1123SubdomainMaxLength - len(suffix) - len(hash) - 1
|
||||
return strings.TrimRight(vmi[:keep], "-.") + "-" + hash + suffix
|
||||
}
|
||||
@@ -18,7 +18,10 @@ package model
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"k8s.io/apimachinery/pkg/util/validation"
|
||||
)
|
||||
|
||||
func TestParseDeviceRefs(t *testing.T) {
|
||||
@@ -41,6 +44,7 @@ func TestParseDeviceRefs(t *testing.T) {
|
||||
{name: "template without class", in: `[{"deviceTemplate":{"count":1}}]`, wantErr: true},
|
||||
{name: "unknown field", in: `[{"name":"nvidia.com/gpu"}]`, wantErr: true},
|
||||
{name: "same device twice", in: `[{"device":"a"},{"device":"a"}]`, wantErr: true},
|
||||
{name: "data after the list", in: `[{"device":"a"}] x`, wantErr: true},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
@@ -71,4 +75,23 @@ func TestTemplateDeviceName(t *testing.T) {
|
||||
if got := TemplateDeviceName("vm-instance-trainer", 1); got != "vm-instance-trainer-gpu-1" {
|
||||
t.Errorf("TemplateDeviceName = %q", got)
|
||||
}
|
||||
// Worker VMIs are named after Cluster API machines and can reach the
|
||||
// 253 characters of a DNS subdomain.
|
||||
long := strings.Repeat("a", 120) + "." + strings.Repeat("b", 132)
|
||||
other := strings.Repeat("a", 120) + "." + strings.Repeat("b", 131) + "c"
|
||||
for _, vmi := range []string{long, other, strings.Repeat("x", 245) + "-" + "yyyyyyy"} {
|
||||
got := TemplateDeviceName(vmi, 12)
|
||||
if errs := validation.IsDNS1123Subdomain(got); len(errs) > 0 {
|
||||
t.Errorf("TemplateDeviceName(%d chars) = %q: %v", len(vmi), got, errs)
|
||||
}
|
||||
if !strings.HasSuffix(got, "-gpu-12") {
|
||||
t.Errorf("TemplateDeviceName lost the index: %q", got)
|
||||
}
|
||||
}
|
||||
if TemplateDeviceName(long, 0) == TemplateDeviceName(other, 0) {
|
||||
t.Error("two long VMI names sharing a prefix map to one device")
|
||||
}
|
||||
if TemplateDeviceName(long, 0) != TemplateDeviceName(long, 0) {
|
||||
t.Error("TemplateDeviceName is not deterministic")
|
||||
}
|
||||
}
|
||||
@@ -22,8 +22,8 @@ import (
|
||||
"github.com/cozystack/cozystack/internal/gpu"
|
||||
)
|
||||
|
||||
// Register is wired into cozystack-controller before any webhook exists:
|
||||
// it must succeed and touch nothing, so it accepts a nil manager.
|
||||
// Register may be called before any webhook exists: it must succeed and
|
||||
// touch nothing, so it accepts a nil manager.
|
||||
func TestRegisterWithoutWebhooksIsANoOp(t *testing.T) {
|
||||
if err := Register(nil, gpu.Config{Enabled: true}); err != nil {
|
||||
t.Fatalf("Register: %v", err)
|
||||
|
||||
Reference in new issue
Block a user