diff --git a/pkg/cdi/cache.go b/pkg/cdi/cache.go index d653ac38..60e4d83d 100644 --- a/pkg/cdi/cache.go +++ b/pkg/cdi/cache.go @@ -17,11 +17,15 @@ package cdi import ( + "io/fs" + "os" "path/filepath" "sort" "strings" "sync" + stderr "errors" + cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go" "github.com/fsnotify/fsnotify" "github.com/hashicorp/go-multierror" @@ -255,11 +259,22 @@ func (c *Cache) InjectDevices(ociSpec *oci.Spec, devices ...string) ([]string, e return nil, nil } -// WriteSpec writes a Spec file with the given content. Priority is used -// as an index into the list of Spec directories to pick a directory for -// the file, adjusting for any under- or overflows. If name has a "json" -// or "yaml" extension it choses the encoding. Otherwise JSON encoding -// is used with a "json" extension. +// highestPrioritySpecDir returns the Spec directory with highest priority +// and its priority. +func (c *Cache) highestPrioritySpecDir() (string, int) { + if len(c.specDirs) == 0 { + return "", -1 + } + + prio := len(c.specDirs) - 1 + dir := c.specDirs[prio] + + return dir, prio +} + +// WriteSpec writes a Spec file with the given content into the highest +// priority Spec directory. If name has a "json" or "yaml" extension it +// choses the encoding. Otherwise the default YAML encoding is used. func (c *Cache) WriteSpec(raw *cdi.Spec, name string) error { var ( specDir string @@ -269,23 +284,51 @@ func (c *Cache) WriteSpec(raw *cdi.Spec, name string) error { err error ) - if len(c.specDirs) == 0 { + specDir, prio = c.highestPrioritySpecDir() + if specDir == "" { return errors.New("no Spec directories to write to") } - prio = len(c.specDirs) - 1 - specDir = c.specDirs[prio] path = filepath.Join(specDir, name) if ext := filepath.Ext(path); ext != ".json" && ext != ".yaml" { - path += ".json" + path += defaultSpecExt } - spec, err = NewSpec(raw, path, prio) + spec, err = newSpec(raw, path, prio) if err != nil { return err } - return spec.Write(true) + return spec.write(true) +} + +// RemoveSpec removes a Spec with the given name from the highest +// priority Spec directory. This function can be used to remove a +// Spec previously written by WriteSpec(). If the file exists and +// its removal fails RemoveSpec returns an error. +func (c *Cache) RemoveSpec(name string) error { + var ( + specDir string + path string + err error + ) + + specDir, _ = c.highestPrioritySpecDir() + if specDir == "" { + return errors.New("no Spec directories to remove from") + } + + path = filepath.Join(specDir, name) + if ext := filepath.Ext(path); ext != ".json" && ext != ".yaml" { + path += defaultSpecExt + } + + err = os.Remove(path) + if err != nil && stderr.Is(err, fs.ErrNotExist) { + err = nil + } + + return err } // GetDevice returns the cached device for the given qualified name. diff --git a/pkg/cdi/cache_test.go b/pkg/cdi/cache_test.go index ff5b8a5d..810fea53 100644 --- a/pkg/cdi/cache_test.go +++ b/pkg/cdi/cache_test.go @@ -17,8 +17,10 @@ package cdi import ( + "fmt" "os" "path/filepath" + "strconv" "strings" "sync" "testing" @@ -29,6 +31,7 @@ import ( oci "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" ) func TestNewCache(t *testing.T) { @@ -1633,6 +1636,215 @@ containerEdits: } } +func TestCacheTransientSpecs(t *testing.T) { + type testCase struct { + name string + specs []string + invalid map[int]bool + expected [][]string + numSpecFiles []int + } + for _, tc := range []*testCase{ + { + name: "invalid spec", + specs: []string{ + ` +cdiVersion: "` + cdi.CurrentVersion + `" +kind: "vendor.comdevice" +devices: + - name: "dev1" + containerEdits: + deviceNodes: + - path: "/dev/vendor1-dev1" + type: b + major: 10 + minor: 1`, + }, + invalid: map[int]bool{ + 0: true, + }, + }, + { + name: "add/remove one valid spec", + specs: []string{ + ` +cdiVersion: "` + cdi.CurrentVersion + `" +kind: "vendor.com/device" +devices: + - name: "dev1" + containerEdits: + deviceNodes: + - path: "/dev/vendor-dev1" + type: b + major: 10 + minor: 1 +`, + "-0", + }, + expected: [][]string{ + []string{ + "vendor.com/device=dev1", + }, + nil, + }, + numSpecFiles: []int{ + 1, + 0, + }, + }, + { + name: "add/remove multiple valid specs", + specs: []string{ + ` +cdiVersion: "` + cdi.CurrentVersion + `" +kind: "vendor.com/device" +devices: + - name: "dev1" + containerEdits: + deviceNodes: + - path: "/dev/vendor-dev1" + type: b + major: 10 + minor: 1 +`, + ` +cdiVersion: "` + cdi.CurrentVersion + `" +kind: "vendor.com/device" +devices: + - name: "dev2" + containerEdits: + deviceNodes: + - path: "/dev/vendor-dev2" + type: b + major: 10 + minor: 2 +`, + ` +cdiVersion: "` + cdi.CurrentVersion + `" +kind: "vendor.com/device" +devices: + - name: "dev3" + containerEdits: + deviceNodes: + - path: "/dev/vendor-dev3" + type: b + major: 10 + minor: 3 + - name: "dev4" + containerEdits: + deviceNodes: + - path: "/dev/vendor-dev4" + type: b + major: 10 + minor: 4 +`, + "-0", + "-1", + "-2", + }, + expected: [][]string{ + []string{ + "vendor.com/device=dev1", + }, + []string{ + "vendor.com/device=dev1", + "vendor.com/device=dev2", + }, + []string{ + "vendor.com/device=dev1", + "vendor.com/device=dev2", + "vendor.com/device=dev3", + "vendor.com/device=dev4", + }, + []string{ + "vendor.com/device=dev2", + "vendor.com/device=dev3", + "vendor.com/device=dev4", + }, + []string{ + "vendor.com/device=dev3", + "vendor.com/device=dev4", + }, + nil, + }, + numSpecFiles: []int{ + 1, + 2, + 3, + 2, + 1, + 0, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var ( + dir string + err error + cache *Cache + specFiles []os.DirEntry + specs = map[int]string{} + ) + + dir, err = createSpecDirs(t, nil, nil) + require.NoError(t, err) + cache, err = NewCache( + WithSpecDirs( + filepath.Join(dir, "etc"), + filepath.Join(dir, "run"), + ), + WithAutoRefresh(false), + ) + + require.NoError(t, err) + require.NotNil(t, cache) + + for idx, data := range tc.specs { + var ( + transientID string + raw *cdi.Spec + delIdx int + err error + ) + + if data[0] == '-' { + delIdx, err = strconv.Atoi(string(data[1:])) + require.NoError(t, err) + + err = cache.RemoveSpec(specs[delIdx]) + require.NoError(t, err) + } else { + err = yaml.Unmarshal([]byte(data), &raw) + require.NoError(t, err) + + transientID = fmt.Sprintf("id%d", idx) + specs[idx], err = GenerateNameForTransientSpec(raw, transientID) + if tc.invalid[idx] { + require.NotNil(t, err) + continue + } + require.NoError(t, err) + + err = cache.WriteSpec(raw, specs[idx]) + require.NoError(t, err) + } + + err = cache.Refresh() + require.NoError(t, err) + + devices := cache.ListDevices() + require.Equal(t, tc.expected[idx], devices) + + specFiles, err = os.ReadDir( + filepath.Join(dir, "run"), + ) + require.NoError(t, err) + require.Equal(t, tc.numSpecFiles[idx], len(specFiles)) + } + }) + } +} + // Create and populate automatically cleaned up spec directories. func createSpecDirs(t *testing.T, etc, run map[string]string) (string, error) { return mkTestDir(t, map[string]map[string]string{ diff --git a/pkg/cdi/doc.go b/pkg/cdi/doc.go index 847e5125..96461229 100644 --- a/pkg/cdi/doc.go +++ b/pkg/cdi/doc.go @@ -124,10 +124,15 @@ // // Generated Spec Files, Multiple Directories, Device Precedence // -// There are systems where the set of available or usable CDI devices -// changes dynamically and this needs to be reflected in the CDI Specs. -// This is done by dynamically regenerating CDI Spec files which are -// affected by these changes. +// It is often necessary to generate Spec files dynamically. On some +// systems the available or usable set of CDI devices might change +// dynamically which then needs to be reflected in CDI Specs. For +// some device classes it makes sense to enumerate the available +// devices at every boot and generate Spec file entries for each +// device found. Some CDI devices might need special client- or +// request-specific configuration which can only be fulfilled by +// dynamically generated client-specific entries in transient Spec +// files. // // CDI can collect Spec files from multiple directories. Spec files are // automatically assigned priorities according to which directory they @@ -141,7 +146,111 @@ // separating dynamically generated CDI Spec files from static ones. // The default directories are '/etc/cdi' and '/var/run/cdi'. By putting // dynamically generated Spec files under '/var/run/cdi', those take -// precedence over static ones in '/etc/cdi'. +// precedence over static ones in '/etc/cdi'. With this scheme, static +// Spec files, typically installed by distro-specific packages, go into +// '/etc/cdi' while all the dynamically generated Spec files, transient +// or other, go into '/var/run/cdi'. +// +// Spec File Generation +// +// CDI offers two functions for writing and removing dynamically generated +// Specs from CDI Spec directories. These functions, WriteSpec() and +// RemoveSpec() implicitly follow the principle of separating dynamic Specs +// from the rest and therefore always write to and remove Specs from the +// last configured directory. +// +// Corresponding functions are also provided for generating names for Spec +// files. These functions follow a simple naming convention to ensure that +// multiple entities generating Spec files simultaneously on the same host +// do not end up using conflicting Spec file names. GenerateSpecName(), +// GenerateNameForSpec(), GenerateTransientSpecName(), and +// GenerateTransientNameForSpec() all generate names which can be passed +// as such to WriteSpec() and subsequently to RemoveSpec(). +// +// Generating a Spec file for a vendor/device class can be done with a +// code snippet similar to the following: +// +// import ( +// "fmt" +// ... +// "github.com/container-orchestrated-devices/container-device-interface/specs-go" +// "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" +// ) +// +// func generateDeviceSpecs() error { +// registry := cdi.GetRegistry() +// spec := &specs.Spec{ +// Version: specs.CurrentVersion, +// Kind: vendor+"/"+class, +// } +// +// for _, dev := range enumerateDevices() { +// spec.Devices = append(spec.Devices, specs.Device{ +// Name: dev.Name, +// ContainerEdits: getContainerEditsForDevice(dev), +// }) +// } +// +// specName, err := cdi.GenerateNameForSpec(spec) +// if err != nil { +// return fmt.Errorf("failed to generate Spec name: %w", err) +// } +// +// return registry.WriteSpec(spec, specName) +// } +// +// Similary, generating and later cleaning up transient Spec files can be +// done with code fragments similar to the following. These transient Spec +// files are temporary Spec files with container-specific parametrization. +// They are typically created before the associated container is created +// and removed once that container is removed. +// +// import ( +// "fmt" +// ... +// "github.com/container-orchestrated-devices/container-device-interface/specs-go" +// "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" +// ) +// +// func generateTransientSpec(ctr Container) error { +// registry := cdi.GetRegistry() +// devices := getContainerDevs(ctr, vendor, class) +// spec := &specs.Spec{ +// Version: specs.CurrentVersion, +// Kind: vendor+"/"+class, +// } +// +// for _, dev := range devices { +// spec.Devices = append(spec.Devices, specs.Device{ +// // the generated name needs to be unique within the +// // vendor/class domain on the host/node. +// Name: generateUniqueDevName(dev, ctr), +// ContainerEdits: getEditsForContainer(dev), +// }) +// } +// +// // transientID is expected to guarantee that the Spec file name +// // generated using is unique within +// // the host/node. If more than one device is allocated with the +// // same vendor/class domain, either all generated Spec entries +// // should go to a single Spec file (like in this sample snippet), +// // or transientID should be unique for each generated Spec file. +// transientID := getSomeSufficientlyUniqueIDForContainer(ctr) +// specName, err := cdi.GenerateNameForTransientSpec(vendor, class, transientID) +// if err != nil { +// return fmt.Errorf("failed to generate Spec name: %w", err) +// } +// +// return registry.WriteSpec(spec, specName) +// } +// +// func removeTransientSpec(ctr Container) error { +// registry := cdi.GetRegistry() +// transientID := getSomeSufficientlyUniqueIDForContainer(ctr) +// specName := cdi.GenerateNameForTransientSpec(vendor, class, transientID) +// +// return registry.RemoveSpec(specName) +// } // // CDI Spec Validation // diff --git a/pkg/cdi/registry.go b/pkg/cdi/registry.go index d5bd54b0..10fab899 100644 --- a/pkg/cdi/registry.go +++ b/pkg/cdi/registry.go @@ -107,6 +107,7 @@ type RegistrySpecDB interface { GetVendorSpecs(vendor string) []*Spec GetSpecErrors(*Spec) []error WriteSpec(raw *cdi.Spec, name string) error + RemoveSpec(name string) error } type registry struct { diff --git a/pkg/cdi/spec.go b/pkg/cdi/spec.go index 3dfbab2f..c558b8ef 100644 --- a/pkg/cdi/spec.go +++ b/pkg/cdi/spec.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "sync" oci "github.com/opencontainers/runtime-spec/specs-go" @@ -30,6 +31,14 @@ import ( cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go" ) +const ( + // CurrentVersion is the current vesion of the CDI Spec. + CurrentVersion = cdi.CurrentVersion + + // defaultSpecExt is the file extension for the default encoding. + defaultSpecExt = ".yaml" +) + var ( // Valid CDI Spec versions. validSpecVersions = map[string]struct{}{ @@ -80,7 +89,7 @@ func ReadSpec(path string, priority int) (*Spec, error) { return nil, errors.Errorf("failed to parse CDI Spec %q, no Spec data", path) } - spec, err := NewSpec(raw, path, priority) + spec, err := newSpec(raw, path, priority) if err != nil { return nil, err } @@ -88,11 +97,11 @@ func ReadSpec(path string, priority int) (*Spec, error) { return spec, nil } -// NewSpec creates a new Spec from the given CDI Spec data. The +// newSpec creates a new Spec from the given CDI Spec data. The // Spec is marked as loaded from the given path with the given -// priority. If Spec data validation fails NewSpec returns a nil +// priority. If Spec data validation fails newSpec returns a nil // Spec and an error. -func NewSpec(raw *cdi.Spec, path string, priority int) (*Spec, error) { +func newSpec(raw *cdi.Spec, path string, priority int) (*Spec, error) { err := validateSpec(raw) if err != nil { return nil, err @@ -104,6 +113,10 @@ func NewSpec(raw *cdi.Spec, path string, priority int) (*Spec, error) { priority: priority, } + if ext := filepath.Ext(spec.path); ext != ".yaml" && ext != ".json" { + spec.path += defaultSpecExt + } + spec.vendor, spec.class = ParseQualifier(spec.Kind) if spec.devices, err = spec.validate(); err != nil { @@ -114,8 +127,8 @@ func NewSpec(raw *cdi.Spec, path string, priority int) (*Spec, error) { } // Write the CDI Spec to the file associated with it during instantiation -// by NewSpec() or ReadSpec(). -func (s *Spec) Write(overwrite bool) error { +// by newSpec() or ReadSpec(). +func (s *Spec) write(overwrite bool) error { var ( data []byte dir string @@ -249,7 +262,7 @@ func ParseSpec(data []byte) (*cdi.Spec, error) { // SetSpecValidator sets a CDI Spec validator function. This function // is used for extra CDI Spec content validation whenever a Spec file -// loaded (using ReadSpec() or NewSpec()) or written (Spec.Write()). +// loaded (using ReadSpec() or written (using WriteSpec()). func SetSpecValidator(fn func(*cdi.Spec) error) { validatorLock.Lock() defer validatorLock.Unlock() @@ -270,3 +283,68 @@ func validateSpec(raw *cdi.Spec) error { } return nil } + +// GenerateSpecName generates a vendor+class scoped Spec file name. The +// name can be passed to WriteSpec() to write a Spec file to the file +// system. +// +// vendor and class should match the vendor and class of the CDI Spec. +// The file name is generated without a ".json" or ".yaml" extension. +// The caller can append the desired extension to choose a particular +// encoding. Otherwise WriteSpec() will use its default encoding. +// +// This function always returns the same name for the same vendor/class +// combination. Therefore it cannot be used as such to generate multiple +// Spec file names for a single vendor and class. +func GenerateSpecName(vendor, class string) string { + return vendor + "-" + class +} + +// GenerateTransientSpecName generates a vendor+class scoped transient +// Spec file name. The name can be passed to WriteSpec() to write a Spec +// file to the file system. +// +// Transient Specs are those whose lifecycle is tied to that of some +// external entity, for instance a container. vendor and class should +// match the vendor and class of the CDI Spec. transientID should be +// unique among all CDI users on the same host that might generate +// transient Spec files using the same vendor/class combination. If +// the external entity to which the lifecycle of the tranient Spec +// is tied to has a unique ID of its own, then this is usually a +// good choice for transientID. +// +// The file name is generated without a ".json" or ".yaml" extension. +// The caller can append the desired extension to choose a particular +// encoding. Otherwise WriteSpec() will use its default encoding. +func GenerateTransientSpecName(vendor, class, transientID string) string { + transientID = strings.ReplaceAll(transientID, "/", "_") + return GenerateSpecName(vendor, class) + "_" + transientID +} + +// GenerateNameForSpec generates a name for the given Spec using +// GenerateSpecName with the vendor and class taken from the Spec. +// On success it returns the generated name and a nil error. If +// the Spec does not contain a valid vendor or class, it returns +// an empty name and a non-nil error. +func GenerateNameForSpec(raw *cdi.Spec) (string, error) { + vendor, class := ParseQualifier(raw.Kind) + if vendor == "" { + return "", errors.Errorf("invalid vendor/class %q in Spec", raw.Kind) + } + + return GenerateSpecName(vendor, class), nil +} + +// GenerateNameForTransientSpec generates a name for the given transient +// Spec using GenerateTransientSpecName with the vendor and class taken +// from the Spec. On success it returns the generated name and a nil error. +// If the Spec does not contain a valid vendor or class, it returns an +// an empty name and a non-nil error. +func GenerateNameForTransientSpec(raw *cdi.Spec, transientID string) (string, error) { + vendor, class := ParseQualifier(raw.Kind) + if vendor == "" { + return "", errors.Errorf("invalid vendor/class %q in Spec", raw.Kind) + } + + return GenerateTransientSpecName(vendor, class, transientID), nil +} diff --git a/pkg/cdi/spec_test.go b/pkg/cdi/spec_test.go index 15afff77..c20b3209 100644 --- a/pkg/cdi/spec_test.go +++ b/pkg/cdi/spec_test.go @@ -20,6 +20,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/pkg/errors" @@ -77,7 +78,7 @@ devices: }, } { t.Run(tc.name, func(t *testing.T) { - file, err := mkTestFile(t, []byte(tc.data)) + file, err := mkTestSpec(t, []byte(tc.data)) if err != nil { t.Errorf("failed to create CDI Spec test file: %v", err) return @@ -245,7 +246,7 @@ devices: } require.NoError(t, err) - spec, err = NewSpec(raw, tc.name, 0) + spec, err = newSpec(raw, tc.name, 0) if tc.invalid || tc.schemaFail { require.Error(t, err) require.Nil(t, spec) @@ -366,22 +367,22 @@ devices: err = yaml.Unmarshal([]byte(tc.data), raw) require.NoError(t, err) - spec, err = NewSpec(raw, filepath.Join(dir, tc.name), 0) + spec, err = newSpec(raw, filepath.Join(dir, tc.name), 0) if tc.invalid { - require.Error(t, err, "NewSpec with invalid data") - require.Nil(t, spec, "NewSpec with invalid data") + require.Error(t, err, "newSpec with invalid data") + require.Nil(t, spec, "newSpec with invalid data") return } require.NoError(t, err) require.NotNil(t, spec) - err = spec.Write(true) + err = spec.write(true) require.NoError(t, err) _, err = os.Stat(spec.GetPath()) require.NoError(t, err, "spec.Write destination file") - err = spec.Write(false) + err = spec.write(false) require.Error(t, err) chk, err = ReadSpec(spec.GetPath(), spec.GetPriority()) @@ -461,7 +462,7 @@ devices: }, } { t.Run(tc.name, func(t *testing.T) { - file, err := mkTestFile(t, []byte(tc.data)) + file, err := mkTestSpec(t, []byte(tc.data)) if err != nil { t.Errorf("failed to create CDI Spec test file: %v", err) return @@ -493,8 +494,8 @@ devices: } // Create an automatically cleaned up temporary file for a test. -func mkTestFile(t *testing.T, data []byte) (string, error) { - tmp, err := ioutil.TempFile("", ".cdi-test.*") +func mkTestSpec(t *testing.T, data []byte) (string, error) { + tmp, err := ioutil.TempFile("", ".cdi-test.*."+specType(data)) if err != nil { return "", errors.Wrapf(err, "failed to create test file") } @@ -515,6 +516,14 @@ func mkTestFile(t *testing.T, data []byte) (string, error) { return file, nil } +func specType(content []byte) string { + spec := strings.TrimSpace(string(content)) + if spec != "" && spec[0] == '{' { + return "json" + } + return "yaml" +} + func TestCurrentVersionIsValid(t *testing.T) { require.NoError(t, validateVersion(cdi.CurrentVersion)) }