Skip to content

Commit 1c6e3fe

Browse files
danielkleinsteindanielkleinstein
authored
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.
1 parent 6decc15 commit 1c6e3fe

File tree

6 files changed

+21
-17
lines changed

6 files changed

+21
-17
lines changed

‎api/config/v1/config.go‎

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

9999
// Disable renaming / device selection in Sharing.TimeSlicing.Resources
100-
config.Sharing.TimeSlicing.disableResoureRenaming(logger, "timeSlicing")
100+
if (config.sharing.TimeSlicing != nil) {
101+
config.Sharing.TimeSlicing.disableResoureRenaming(logger, "timeSlicing")
102+
}
101103
// Disable renaming / device selection in Sharing.MPS.Resources
102-
config.Sharing.MPS.disableResoureRenaming(logger, "mps")
104+
if (config.Sharing.MPS != nil) {
105+
config.Sharing.MPS.disableResoureRenaming(logger, "mps")
106+
}
103107
}
104108

105109
// 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: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
5858
devices: []resource.Device{
5959
rt.NewFullGPU(),
6060
},
61-
timeSlicing: spec.ReplicatedResources{
61+
timeSlicing: &spec.ReplicatedResources{
6262
Resources: []spec.ReplicatedResource{
6363
{
6464
Name: "nvidia.com/gpu",
@@ -83,7 +83,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
8383
rt.NewFullGPU(),
8484
rt.NewFullGPU(),
8585
},
86-
timeSlicing: spec.ReplicatedResources{
86+
timeSlicing: &spec.ReplicatedResources{
8787
Resources: []spec.ReplicatedResource{
8888
{
8989
Name: "nvidia.com/gpu",
@@ -107,7 +107,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
107107
devices: []resource.Device{
108108
rt.NewMigEnabledDevice(),
109109
},
110-
timeSlicing: spec.ReplicatedResources{
110+
timeSlicing: &spec.ReplicatedResources{
111111
Resources: []spec.ReplicatedResource{
112112
{
113113
Name: "nvidia.com/gpu",
@@ -129,7 +129,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
129129
rt.NewMigEnabledDevice(),
130130
rt.NewMigEnabledDevice(),
131131
},
132-
timeSlicing: spec.ReplicatedResources{
132+
timeSlicing: &spec.ReplicatedResources{
133133
Resources: []spec.ReplicatedResource{
134134
{
135135
Name: "nvidia.com/gpu",
@@ -151,7 +151,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
151151
rt.NewMigEnabledDevice(),
152152
rt.NewFullGPU(),
153153
},
154-
timeSlicing: spec.ReplicatedResources{
154+
timeSlicing: &spec.ReplicatedResources{
155155
Resources: []spec.ReplicatedResource{
156156
{
157157
Name: "nvidia.com/gpu",

‎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: 3 additions & 3 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",

‎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)