Skip to content

Commit e5f49f9

Browse files
bugfix: Allow for optional timeslicing configuration
This fixes a vestigial bug from the introduction of MPS. Now that two timeslicing configurations are available, it's possible for MPS to be configured instead of timeslicing. But since the TimeSlicing field was made non-optional - its existence is forced when unmarshalling, and then the parsing fails because no resources are specified under the empty TimeSlicing field. Signed-off-by: Daniel Kleinstein <daniel.kleinstein@gmail.com>
1 parent c09799f commit e5f49f9

File tree

6 files changed

+18
-14
lines changed

6 files changed

+18
-14
lines changed

api/config/v1/config.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,13 @@ func DisableResourceNamingInConfig(logger logger, config *Config) {
9696
config.Resources.MIGs = nil
9797

9898
// Disable renaming / device selection in Sharing.TimeSlicing.Resources
99-
config.Sharing.TimeSlicing.disableResoureRenaming(logger, "timeSlicing")
99+
if (config.sharing.TimeSlicing != nil) {
100+
config.Sharing.TimeSlicing.disableResoureRenaming(logger, "timeSlicing")
101+
}
100102
// Disable renaming / device selection in Sharing.MPS.Resources
101-
config.Sharing.MPS.disableResoureRenaming(logger, "mps")
103+
if (config.Sharing.MPS != nil) {
104+
config.Sharing.MPS.disableResoureRenaming(logger, "mps")
105+
}
102106
}
103107

104108
// parseConfig parses a config file as either YAML of JSON and unmarshals it into a Config struct.

api/config/v1/sharing.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package v1
1919
// Sharing encapsulates the set of sharing strategies that are supported.
2020
type Sharing struct {
2121
// TimeSlicing defines the set of replicas to be made for timeSlicing available resources.
22-
TimeSlicing ReplicatedResources `json:"timeSlicing,omitempty" yaml:"timeSlicing,omitempty"`
22+
TimeSlicing *ReplicatedResources `json:"timeSlicing,omitempty" yaml:"timeSlicing,omitempty"`
2323
// MPS defines the set of replicas to be shared using MPS
2424
MPS *ReplicatedResources `json:"mps,omitempty" yaml:"mps,omitempty"`
2525
}
@@ -38,7 +38,7 @@ func (s *Sharing) SharingStrategy() SharingStrategy {
3838
return SharingStrategyMPS
3939
}
4040

41-
if s.TimeSlicing.isReplicated() {
41+
if s.TimeSlicing != nil && s.TimeSlicing.isReplicated() {
4242
return SharingStrategyTimeSlicing
4343
}
4444
return SharingStrategyNone
@@ -49,5 +49,5 @@ func (s *Sharing) ReplicatedResources() *ReplicatedResources {
4949
if s.MPS != nil {
5050
return s.MPS
5151
}
52-
return &s.TimeSlicing
52+
return s.TimeSlicing
5353
}

internal/lm/mig-strategy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
183183
},
184184
},
185185
Sharing: spec.Sharing{
186-
TimeSlicing: tc.timeSlicing,
186+
TimeSlicing: &tc.timeSlicing,
187187
},
188188
}
189189

internal/lm/nvml_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func TestSharingLabeler(t *testing.T) {
103103
description: "config with timeslicing replicas",
104104
config: &spec.Config{
105105
Sharing: spec.Sharing{
106-
TimeSlicing: spec.ReplicatedResources{
106+
TimeSlicing: &spec.ReplicatedResources{
107107
Resources: []spec.ReplicatedResource{
108108
{
109109
Replicas: 2,

internal/lm/resource_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func TestGPUResourceLabeler(t *testing.T) {
5555
description: "time-slicing ignores non-matching resource",
5656
count: 1,
5757
sharing: spec.Sharing{
58-
TimeSlicing: spec.ReplicatedResources{
58+
TimeSlicing: &spec.ReplicatedResources{
5959
Resources: []spec.ReplicatedResource{
6060
{
6161
Name: "nvidia.com/not-gpu",
@@ -79,7 +79,7 @@ func TestGPUResourceLabeler(t *testing.T) {
7979
description: "time-slicing appends suffix and doubles count",
8080
count: 1,
8181
sharing: spec.Sharing{
82-
TimeSlicing: spec.ReplicatedResources{
82+
TimeSlicing: &spec.ReplicatedResources{
8383
Resources: []spec.ReplicatedResource{
8484
{
8585
Name: "nvidia.com/gpu",
@@ -103,7 +103,7 @@ func TestGPUResourceLabeler(t *testing.T) {
103103
description: "time-slicing renamed does not append suffix and doubles count",
104104
count: 1,
105105
sharing: spec.Sharing{
106-
TimeSlicing: spec.ReplicatedResources{
106+
TimeSlicing: &spec.ReplicatedResources{
107107
Resources: []spec.ReplicatedResource{
108108
{
109109
Name: "nvidia.com/gpu",
@@ -422,7 +422,7 @@ func TestMigResourceLabeler(t *testing.T) {
422422
t.Run(tc.description, func(t *testing.T) {
423423
config := &spec.Config{
424424
Sharing: spec.Sharing{
425-
TimeSlicing: tc.timeSlicing,
425+
TimeSlicing: &tc.timeSlicing,
426426
},
427427
}
428428
l, err := NewMIGResourceLabeler(tc.resourceName, config, device, tc.count)

internal/rm/rm_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestValidateRequest(t *testing.T) {
5353
{
5454
description: "timeslicing with single device",
5555
sharing: spec.Sharing{
56-
TimeSlicing: spec.ReplicatedResources{
56+
TimeSlicing: &spec.ReplicatedResources{
5757
Resources: []spec.ReplicatedResource{
5858
{
5959
Name: "nvidia.com/gpu",
@@ -73,7 +73,7 @@ func TestValidateRequest(t *testing.T) {
7373
{
7474
description: "timeslicing with two devices",
7575
sharing: spec.Sharing{
76-
TimeSlicing: spec.ReplicatedResources{
76+
TimeSlicing: &spec.ReplicatedResources{
7777
Resources: []spec.ReplicatedResource{
7878
{
7979
Name: "nvidia.com/gpu",
@@ -93,7 +93,7 @@ func TestValidateRequest(t *testing.T) {
9393
{
9494
description: "timeslicing with two devices -- failRequestsGreaterThanOne",
9595
sharing: spec.Sharing{
96-
TimeSlicing: spec.ReplicatedResources{
96+
TimeSlicing: &spec.ReplicatedResources{
9797
FailRequestsGreaterThanOne: true,
9898
Resources: []spec.ReplicatedResource{
9999
{

0 commit comments

Comments
 (0)