Skip to content

Commit

Permalink
fix: nil deref in ReadPreemptionStatus (#8979)
Browse files Browse the repository at this point in the history
  • Loading branch information
carolinaecalderon authored Mar 8, 2024
1 parent 6e1acf4 commit f67c473
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion master/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ func ReadRMPreemptionStatus(rpName string) bool {
for _, r := range config.ResourceManagers() {
for _, rpConfig := range r.ResourcePools {
if rpConfig.PoolName == rpName {
if rpConfig.Scheduler == nil {
if rpConfig.Scheduler != nil {
return rpConfig.Scheduler.GetPreemption()
}
// if not found, fall back to resource manager config
Expand Down
15 changes: 15 additions & 0 deletions master/internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,13 @@ func TestMultiRMPreemptionAndPriority(t *testing.T) {
DefaultPriority: &prio1,
}},
}}},
// nil preemption case
{
ResourceManager: &ResourceManagerConfig{KubernetesRM: &KubernetesResourceManagerConfig{
Name: "nil-rm", DefaultScheduler: "not-preemption-scheduler",
}},
ResourcePools: []ResourcePoolConfig{{PoolName: "nil-rp"}},
},
},
}

Expand Down Expand Up @@ -867,4 +874,12 @@ func TestMultiRMPreemptionAndPriority(t *testing.T) {

priority = ReadPriority("default234", model.CommandConfig{})
require.Equal(t, prio1, priority)

// 'nil-rp' RP exists under 'nil-rm' RM, so the preemption
// & priority default to the RMs.
status = ReadRMPreemptionStatus("nil-rp")
require.False(t, status)

priority = ReadPriority("nil-rp", model.CommandConfig{})
require.Equal(t, KubernetesDefaultPriority, priority)
}
3 changes: 1 addition & 2 deletions master/internal/experiment_job_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,11 @@ func (e *internalExperiment) ToV1Job() (*jobv1.Job, error) {
WorkspaceId: int32(workspace.ID),
}

j.ResourcePool = e.activeConfig.Resources().ResourcePool()
j.IsPreemptible = config.ReadRMPreemptionStatus(j.ResourcePool)
j.Priority = int32(config.ReadPriority(j.ResourcePool, &e.activeConfig))
j.Weight = config.ReadWeight(j.ResourcePool, &e.activeConfig)

j.ResourcePool = e.activeConfig.Resources().ResourcePool()

return &j, nil
}

Expand Down

0 comments on commit f67c473

Please sign in to comment.