|
| 1 | +package ray |
| 2 | + |
| 3 | +import ( |
| 4 | + "context" |
| 5 | + "maps" |
| 6 | + "testing" |
| 7 | + |
| 8 | + rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1" |
| 9 | + "github.com/stretchr/testify/assert" |
| 10 | + |
| 11 | + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/plugins" |
| 12 | +) |
| 13 | + |
| 14 | +// A RayJob that leaves RayStartParams unset — the common case, and the one that falls through to |
| 15 | +// the plugin's configured defaults. |
| 16 | +func rayJobWithoutStartParams() *plugins.RayJob { |
| 17 | + return &plugins.RayJob{ |
| 18 | + RayCluster: &plugins.RayCluster{ |
| 19 | + HeadGroupSpec: &plugins.HeadGroupSpec{}, |
| 20 | + WorkerGroupSpec: []*plugins.WorkerGroupSpec{{GroupName: workerGroupName, Replicas: 1, MinReplicas: 1, MaxReplicas: 1}}, |
| 21 | + }, |
| 22 | + ShutdownAfterJobFinishes: true, |
| 23 | + TtlSecondsAfterFinished: 120, |
| 24 | + } |
| 25 | +} |
| 26 | + |
| 27 | +// Other tests in this package replace the global config without restoring it, which can leave the |
| 28 | +// default start parameters empty and skip the branch under test. Pin a config with non-empty |
| 29 | +// defaults, and put the original back afterwards. |
| 30 | +func withDefaultStartParams(t *testing.T) { |
| 31 | + origConfig := *GetConfig() |
| 32 | + t.Cleanup(func() { assert.NoError(t, SetConfig(&origConfig)) }) |
| 33 | + |
| 34 | + assert.NoError(t, SetConfig(&Config{ |
| 35 | + Defaults: DefaultConfig{ |
| 36 | + HeadNode: NodeConfig{ |
| 37 | + StartParameters: map[string]string{DisableUsageStatsStartParameter: "true"}, |
| 38 | + IPAddress: "$MY_POD_IP", |
| 39 | + }, |
| 40 | + WorkerNode: NodeConfig{ |
| 41 | + StartParameters: map[string]string{DisableUsageStatsStartParameter: "true"}, |
| 42 | + IPAddress: "$MY_POD_IP", |
| 43 | + }, |
| 44 | + }, |
| 45 | + })) |
| 46 | +} |
| 47 | + |
| 48 | +// BuildResource must not write into the plugin's shared configuration. |
| 49 | +// |
| 50 | +// When a task leaves RayStartParams unset, the head/worker start params fall back to |
| 51 | +// cfg.Defaults.{Head,Worker}Node.StartParameters. Those maps belong to the process-wide plugin |
| 52 | +// config, so filling in include-dashboard / node-ip-address / dashboard-host mutates config that |
| 53 | +// every later task reads. |
| 54 | +func TestBuildResource_DoesNotMutateSharedConfig(t *testing.T) { |
| 55 | + withDefaultStartParams(t) |
| 56 | + |
| 57 | + cfg := GetConfig() |
| 58 | + headDefaults := cfg.Defaults.HeadNode.StartParameters |
| 59 | + workerDefaults := cfg.Defaults.WorkerNode.StartParameters |
| 60 | + |
| 61 | + headBefore := maps.Clone(headDefaults) |
| 62 | + workerBefore := maps.Clone(workerDefaults) |
| 63 | + |
| 64 | + handler := rayJobResourceHandler{} |
| 65 | + taskTemplate := dummyRayTaskTemplate("ray-id", rayJobWithoutStartParams()) |
| 66 | + taskCtx := dummyRayTaskContext(taskTemplate, resourceRequirements, nil, "", serviceAccount) |
| 67 | + |
| 68 | + _, err := handler.BuildResource(context.TODO(), taskCtx) |
| 69 | + assert.NoError(t, err) |
| 70 | + |
| 71 | + assert.Equal(t, headBefore, cfg.Defaults.HeadNode.StartParameters, |
| 72 | + "BuildResource mutated the shared head-node start parameters") |
| 73 | + assert.Equal(t, workerBefore, cfg.Defaults.WorkerNode.StartParameters, |
| 74 | + "BuildResource mutated the shared worker-node start parameters") |
| 75 | +} |
| 76 | + |
| 77 | +// Two RayJobs built independently must not share one RayStartParams map. |
| 78 | +// |
| 79 | +// constructRayJob assigns the resolved start params straight into the CR |
| 80 | +// (RayStartParams: headNodeRayStartParams). When they came from the shared defaults, every CR |
| 81 | +// built that way — and the plugin config itself — is the same map instance. |
| 82 | +func TestBuildResource_RayJobsDoNotShareStartParamsMap(t *testing.T) { |
| 83 | + withDefaultStartParams(t) |
| 84 | + |
| 85 | + handler := rayJobResourceHandler{} |
| 86 | + |
| 87 | + build := func() *rayv1.RayJob { |
| 88 | + taskTemplate := dummyRayTaskTemplate("ray-id", rayJobWithoutStartParams()) |
| 89 | + taskCtx := dummyRayTaskContext(taskTemplate, resourceRequirements, nil, "", serviceAccount) |
| 90 | + obj, err := handler.BuildResource(context.TODO(), taskCtx) |
| 91 | + assert.NoError(t, err) |
| 92 | + return obj.(*rayv1.RayJob) |
| 93 | + } |
| 94 | + |
| 95 | + first := build() |
| 96 | + second := build() |
| 97 | + |
| 98 | + first.Spec.RayClusterSpec.HeadGroupSpec.RayStartParams["num-cpus"] = "tainted-by-first-rayjob" |
| 99 | + |
| 100 | + assert.NotContains(t, second.Spec.RayClusterSpec.HeadGroupSpec.RayStartParams, "num-cpus", |
| 101 | + "editing one RayJob's start params changed another RayJob's") |
| 102 | + assert.NotContains(t, GetConfig().Defaults.HeadNode.StartParameters, "num-cpus", |
| 103 | + "editing a RayJob's start params changed the shared plugin config") |
| 104 | +} |
0 commit comments