Skip to content

Commit 6a5e54c

Browse files
bart0shbart0sh
committed
Get CDI devices from CRI Config.CDIDevices field
Signed-off-by: Ed Bartosh <[email protected]> (cherry picked from commit cd16b31) Signed-off-by: Ed Bartosh <[email protected]>
1 parent 8f682ed commit 6a5e54c

5 files changed

Lines changed: 439 additions & 19 deletions

File tree

‎pkg/cri/opts/spec_linux.go‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ import (
3030
"github.com/containerd/cgroups/v3"
3131
"github.com/sirupsen/logrus"
3232
"golang.org/x/sys/unix"
33+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
3334

3435
"github.com/containerd/containerd/containers"
3536
"github.com/containerd/containerd/log"
3637
"github.com/containerd/containerd/oci"
38+
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
3739
)
3840

3941
// Linux dependent OCI spec opts.
@@ -139,18 +141,40 @@ func IsCgroup2UnifiedMode() bool {
139141
}
140142

141143
// WithCDI updates OCI spec with CDI content
142-
func WithCDI(annotations map[string]string) oci.SpecOpts {
144+
func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) oci.SpecOpts {
143145
return func(ctx context.Context, _ oci.Client, c *containers.Container, s *oci.Spec) error {
144-
// TODO: Once CRI is extended with native CDI support this will need to be updated...
145-
_, cdiDevices, err := cdi.ParseAnnotations(annotations)
146+
// Add devices from CDIDevices CRI field
147+
var devices []string
148+
var err error
149+
for _, device := range CDIDevices {
150+
devices = append(devices, device.Name)
151+
}
152+
log.G(ctx).Infof("Container %v: CDI devices from CRI Config.CDIDevices: %v", c.ID, devices)
153+
154+
// Add devices from CDI annotations
155+
_, devsFromAnnotations, err := cdi.ParseAnnotations(annotations)
146156
if err != nil {
147157
return fmt.Errorf("failed to parse CDI device annotations: %w", err)
148158
}
149-
if cdiDevices == nil {
150-
return nil
159+
160+
if devsFromAnnotations != nil {
161+
log.G(ctx).Infof("Container %v: CDI devices from annotations: %v", c.ID, devsFromAnnotations)
162+
for _, deviceName := range devsFromAnnotations {
163+
if ctrdutil.InStringSlice(devices, deviceName) {
164+
// TODO: change to Warning when passing CDI devices as annotations is deprecated
165+
log.G(ctx).Debugf("Skipping duplicated CDI device %s", deviceName)
166+
continue
167+
}
168+
devices = append(devices, deviceName)
169+
}
170+
// TODO: change to Warning when passing CDI devices as annotations is deprecated
171+
log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead")
151172
}
152173

153-
log.G(ctx).Infof("container %v: CDI devices: %v", c.ID, cdiDevices)
174+
if len(devices) == 0 {
175+
// No devices found, skip device injection
176+
return nil
177+
}
154178

155179
registry := cdi.GetRegistry()
156180
if err = registry.Refresh(); err != nil {
@@ -162,7 +186,7 @@ func WithCDI(annotations map[string]string) oci.SpecOpts {
162186
log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
163187
}
164188

165-
if _, err := registry.InjectDevices(s, cdiDevices...); err != nil {
189+
if _, err := registry.InjectDevices(s, devices...); err != nil {
166190
return fmt.Errorf("CDI device injection failed: %w", err)
167191
}
168192

‎pkg/cri/sbserver/container_create_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
121121
specOpts = append(specOpts, seccompSpecOpts)
122122
}
123123
if c.config.EnableCDI {
124-
specOpts = append(specOpts, customopts.WithCDI(config.Annotations))
124+
specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices))
125125
}
126126
return specOpts, nil
127127
}

‎pkg/cri/sbserver/container_create_linux_test.go‎

Lines changed: 203 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,13 +1553,20 @@ func TestCDIInjections(t *testing.T) {
15531553
for _, test := range []struct {
15541554
description string
15551555
cdiSpecFiles []string
1556+
cdiDevices []*runtime.CDIDevice
15561557
annotations map[string]string
15571558
expectError bool
15581559
expectDevices []runtimespec.LinuxDevice
15591560
expectEnv []string
15601561
}{
1561-
{description: "expect no CDI error for nil annotations"},
1562-
{description: "expect no CDI error for empty annotations",
1562+
{description: "expect no CDI error for nil annotations",
1563+
cdiDevices: []*runtime.CDIDevice{},
1564+
},
1565+
{description: "expect no CDI error for nil CDIDevices",
1566+
annotations: map[string]string{},
1567+
},
1568+
{description: "expect no CDI error for empty CDI devices and annotations",
1569+
cdiDevices: []*runtime.CDIDevice{},
15631570
annotations: map[string]string{},
15641571
},
15651572
{description: "expect CDI error for invalid CDI device reference in annotations",
@@ -1568,13 +1575,25 @@ func TestCDIInjections(t *testing.T) {
15681575
},
15691576
expectError: true,
15701577
},
1571-
{description: "expect CDI error for unresolvable devices",
1578+
{description: "expect CDI error for invalid CDI device reference in CDIDevices",
1579+
cdiDevices: []*runtime.CDIDevice{
1580+
{Name: "foobar"},
1581+
},
1582+
expectError: true,
1583+
},
1584+
{description: "expect CDI error for unresolvable devices in annotations",
15721585
annotations: map[string]string{
15731586
cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev",
15741587
},
15751588
expectError: true,
15761589
},
1577-
{description: "expect properly injected resolvable CDI devices",
1590+
{description: "expect CDI error for unresolvable devices in CDIDevices",
1591+
cdiDevices: []*runtime.CDIDevice{
1592+
{Name: "vendor1.com/device=no-such-dev"},
1593+
},
1594+
expectError: true,
1595+
},
1596+
{description: "expect properly injected resolvable CDI devices from annotations",
15781597
cdiSpecFiles: []string{
15791598
`
15801599
cdiVersion: "0.3.0"
@@ -1636,6 +1655,185 @@ containerEdits:
16361655
"VENDOR2=present",
16371656
},
16381657
},
1658+
{description: "expect properly injected resolvable CDI devices from CDIDevices",
1659+
cdiSpecFiles: []string{
1660+
`
1661+
cdiVersion: "0.3.0"
1662+
kind: "vendor1.com/device"
1663+
devices:
1664+
- name: foo
1665+
containerEdits:
1666+
deviceNodes:
1667+
- path: /dev/loop8
1668+
type: b
1669+
major: 7
1670+
minor: 8
1671+
env:
1672+
- FOO=injected
1673+
containerEdits:
1674+
env:
1675+
- "VENDOR1=present"
1676+
`,
1677+
`
1678+
cdiVersion: "0.3.0"
1679+
kind: "vendor2.com/device"
1680+
devices:
1681+
- name: bar
1682+
containerEdits:
1683+
deviceNodes:
1684+
- path: /dev/loop9
1685+
type: b
1686+
major: 7
1687+
minor: 9
1688+
env:
1689+
- BAR=injected
1690+
containerEdits:
1691+
env:
1692+
- "VENDOR2=present"
1693+
`,
1694+
},
1695+
cdiDevices: []*runtime.CDIDevice{
1696+
{Name: "vendor1.com/device=foo"},
1697+
{Name: "vendor2.com/device=bar"},
1698+
},
1699+
expectDevices: []runtimespec.LinuxDevice{
1700+
{
1701+
Path: "/dev/loop8",
1702+
Type: "b",
1703+
Major: 7,
1704+
Minor: 8,
1705+
},
1706+
{
1707+
Path: "/dev/loop9",
1708+
Type: "b",
1709+
Major: 7,
1710+
Minor: 9,
1711+
},
1712+
},
1713+
expectEnv: []string{
1714+
"FOO=injected",
1715+
"VENDOR1=present",
1716+
"BAR=injected",
1717+
"VENDOR2=present",
1718+
},
1719+
},
1720+
{description: "expect CDI devices from CDIDevices and annotations",
1721+
cdiSpecFiles: []string{
1722+
`
1723+
cdiVersion: "0.3.0"
1724+
kind: "vendor1.com/device"
1725+
devices:
1726+
- name: foo
1727+
containerEdits:
1728+
deviceNodes:
1729+
- path: /dev/loop8
1730+
type: b
1731+
major: 7
1732+
minor: 8
1733+
env:
1734+
- FOO=injected
1735+
containerEdits:
1736+
env:
1737+
- "VENDOR1=present"
1738+
`,
1739+
`
1740+
cdiVersion: "0.3.0"
1741+
kind: "vendor2.com/device"
1742+
devices:
1743+
- name: bar
1744+
containerEdits:
1745+
deviceNodes:
1746+
- path: /dev/loop9
1747+
type: b
1748+
major: 7
1749+
minor: 9
1750+
env:
1751+
- BAR=injected
1752+
containerEdits:
1753+
env:
1754+
- "VENDOR2=present"
1755+
`,
1756+
`
1757+
cdiVersion: "0.3.0"
1758+
kind: "vendor3.com/device"
1759+
devices:
1760+
- name: foo3
1761+
containerEdits:
1762+
deviceNodes:
1763+
- path: /dev/loop10
1764+
type: b
1765+
major: 7
1766+
minor: 10
1767+
env:
1768+
- FOO3=injected
1769+
containerEdits:
1770+
env:
1771+
- "VENDOR3=present"
1772+
`,
1773+
`
1774+
cdiVersion: "0.3.0"
1775+
kind: "vendor4.com/device"
1776+
devices:
1777+
- name: bar4
1778+
containerEdits:
1779+
deviceNodes:
1780+
- path: /dev/loop11
1781+
type: b
1782+
major: 7
1783+
minor: 11
1784+
env:
1785+
- BAR4=injected
1786+
containerEdits:
1787+
env:
1788+
- "VENDOR4=present"
1789+
`,
1790+
},
1791+
cdiDevices: []*runtime.CDIDevice{
1792+
{Name: "vendor1.com/device=foo"},
1793+
{Name: "vendor2.com/device=bar"},
1794+
{Name: "vendor3.com/device=foo3"},
1795+
},
1796+
annotations: map[string]string{
1797+
cdi.AnnotationPrefix + "vendor3_devices": "vendor3.com/device=foo3", // Duplicated device, should be ignored
1798+
cdi.AnnotationPrefix + "vendor4_devices": "vendor4.com/device=bar4",
1799+
},
1800+
expectDevices: []runtimespec.LinuxDevice{
1801+
{
1802+
Path: "/dev/loop8",
1803+
Type: "b",
1804+
Major: 7,
1805+
Minor: 8,
1806+
},
1807+
{
1808+
Path: "/dev/loop9",
1809+
Type: "b",
1810+
Major: 7,
1811+
Minor: 9,
1812+
},
1813+
{
1814+
Path: "/dev/loop10",
1815+
Type: "b",
1816+
Major: 7,
1817+
Minor: 10,
1818+
},
1819+
{
1820+
Path: "/dev/loop11",
1821+
Type: "b",
1822+
Major: 7,
1823+
Minor: 11,
1824+
},
1825+
},
1826+
expectEnv: []string{
1827+
"FOO=injected",
1828+
"VENDOR1=present",
1829+
"BAR=injected",
1830+
"VENDOR2=present",
1831+
"FOO3=injected",
1832+
"VENDOR3=present",
1833+
"BAR4=injected",
1834+
"VENDOR4=present",
1835+
},
1836+
},
16391837
} {
16401838
t.Run(test.description, func(t *testing.T) {
16411839
spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
@@ -1653,7 +1851,7 @@ containerEdits:
16531851
err = reg.Configure(cdi.WithSpecDirs(cdiDir))
16541852
require.NoError(t, err)
16551853

1656-
injectFun := customopts.WithCDI(test.annotations)
1854+
injectFun := customopts.WithCDI(test.annotations, test.cdiDevices)
16571855
err = injectFun(ctx, nil, testContainer, spec)
16581856
assert.Equal(t, test.expectError, err != nil)
16591857

‎pkg/cri/server/container_create_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
414414
specOpts = append(specOpts, seccompSpecOpts)
415415
}
416416
if c.config.EnableCDI {
417-
specOpts = append(specOpts, customopts.WithCDI(config.Annotations))
417+
specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices))
418418
}
419419
return specOpts, nil
420420
}

0 commit comments

Comments
 (0)