Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review processing of DeviceRequest due to Docker engine changes #689

Merged
merged 1 commit into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2123,7 +2123,7 @@ services:
}

func TestServiceDeviceRequestCountStringType(t *testing.T) {
_, err := loadYAML(`
project, err := loadYAML(`
name: service-device-request-count
services:
hello-world:
Expand All @@ -2137,6 +2137,7 @@ services:
count: all
`)
assert.NilError(t, err)
assert.Equal(t, project.Services["hello-world"].Deploy.Resources.Reservations.Devices[0].Count, types.DeviceCount(-1), err)
}

func TestServiceDeviceRequestCountIntegerAsStringType(t *testing.T) {
Expand All @@ -2155,6 +2156,22 @@ services:
`)
assert.NilError(t, err)
}
func TestServiceDeviceRequestWithoutCountAndDeviceIdsType(t *testing.T) {
project, err := loadYAML(`
name: service-device-request-count-type
services:
hello-world:
image: redis:alpine
deploy:
resources:
reservations:
devices:
- driver: nvidia
capabilities: [gpu]
`)
assert.NilError(t, err)
assert.Equal(t, project.Services["hello-world"].Deploy.Resources.Reservations.Devices[0].Count, types.DeviceCount(-1), err)
}

func TestServiceDeviceRequestCountInvalidStringType(t *testing.T) {
_, err := loadYAML(`
Expand All @@ -2173,6 +2190,40 @@ services:
assert.ErrorContains(t, err, `invalid value "some_string", the only value allowed is 'all' or a number`)
}

func TestServiceDeviceRequestCountAndDeviceIdsExclusive(t *testing.T) {
_, err := loadYAML(`
name: service-device-request-count-type
services:
hello-world:
image: redis:alpine
deploy:
resources:
reservations:
devices:
- driver: nvidia
capabilities: [gpu]
count: 2
device_ids: ["my-device-id"]
`)
assert.ErrorContains(t, err, `invalid "count" and "device_ids" are attributes are exclusive`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.ErrorContains(t, err, `invalid "count" and "device_ids" are attributes are exclusive`)
assert.ErrorContains(t, err, `invalid "count" and "device_ids", these attributes are exclusive`)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: why can't capabilities and device_ids be defined at the same time?

}

func TestServiceDeviceRequestCapabilitiesMandatory(t *testing.T) {
_, err := loadYAML(`
name: service-device-request-count-type
services:
hello-world:
image: redis:alpine
deploy:
resources:
reservations:
devices:
- driver: nvidia
count: 2
`)
assert.ErrorContains(t, err, `"capabilities" attribute is mandatory for device request definition`)
}

func TestServicePullPolicy(t *testing.T) {
actual, err := loadYAML(`
name: service-pull-policy
Expand Down
45 changes: 45 additions & 0 deletions types/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,48 @@ func (c *DeviceCount) DecodeMapstructure(value interface{}) error {
}
return nil
}

func (d *DeviceRequest) DecodeMapstructure(value interface{}) error {
v, ok := value.(map[string]any)
if !ok {
return fmt.Errorf("invalid device request type %T", value)
}
if _, okCaps := v["capabilities"]; !okCaps {
return fmt.Errorf(`"capabilities" attribute is mandatory for device request definition`)
}
if _, okCount := v["count"]; okCount {
if _, okDeviceIds := v["device_ids"]; okDeviceIds {
return fmt.Errorf(`invalid "count" and "device_ids" are attributes are exclusive`)
}
}
d.Count = DeviceCount(-1)

capabilities := v["capabilities"]
caps := StringList{}
if err := caps.DecodeMapstructure(capabilities); err != nil {
return err
}
d.Capabilities = caps
if driver, ok := v["driver"]; ok {
if val, ok := driver.(string); ok {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't JSON schema handle this?

d.Driver = val
} else {
return fmt.Errorf("invalid type for driver value: %T", driver)
}
}
if count, ok := v["count"]; ok {
if err := d.Count.DecodeMapstructure(count); err != nil {
return err
}
}
if deviceIDs, ok := v["device_ids"]; ok {
ids := StringList{}
if err := ids.DecodeMapstructure(deviceIDs); err != nil {
return err
}
d.IDs = ids
d.Count = DeviceCount(len(ids))
}
return nil

}