[1fanwang]

Why are the changes needed?

A RayJob task that doesn't set RayStartParams gets the plugin's configured defaults. BuildResource doesn't copy them — it assigns the config's map to the local variable and then writes include-dashboard, node-ip-address and dashboard-host straight into it.

That map is cfg.Defaults.HeadNode.StartParameters, owned by the process-wide plugin config. It's also what gets assigned to the CR (RayStartParams: headNodeRayStartParams). So the plugin config, and every RayJob built from the defaults, are all the same map.

Two things follow. The plugin rewrites its own configuration on the first build, and any later edit to one RayJob's start params lands in every other RayJob and in the config:

editing one RayJob's start params changed another RayJob's
  map[... "num-cpus":"tainted-by-first-rayjob"]

Concurrent builds also write that map with no synchronisation, which is a data race on a plain Go map.

The defaults map ships non-empty (disable-usage-stats), so the len(...) > 0 branch is taken on stock configuration. Worker groups take the same path.

What changes were proposed in this pull request?

Copy the resolved start parameters into the map BuildResource already allocates, instead of reassigning the variable to the caller's or the config's map. Applies to the head group and to each worker group.

The make(map[string]string) on the line above was already being discarded by the reassignment, so this just uses it.

How was this patch tested?

sharedconfig_repro_test.go. The first two fail without the change:

• TestBuildResource_DoesNotMutateSharedConfig — one build with no RayStartParams set, comparing the config's head and worker start parameters against a snapshot taken beforehand.
• TestBuildResource_RayJobsDoNotShareStartParamsMap — builds two RayJobs, writes a key into the first one's head start params, and checks the second RayJob and the plugin config are untouched.
• TestBuildResource_KeepsTaskSuppliedStartParams — a task supplying its own head and worker params still gets them through to the CR alongside the injected defaults.

Each test pins its own config, because other tests in the package call SetConfig without restoring it, which can empty the defaults and skip the branch under test.

cd flyteplugins
git checkout upstream/master -- go/tasks/plugins/k8s/ray/ray.go   # drop the fix
go test ./go/tasks/plugins/k8s/ray/ -count=1
git checkout HEAD -- go/tasks/plugins/k8s/ray/ray.go              # restore it
go test ./go/tasks/plugins/k8s/ray/ -count=1
go test ./go/tasks/plugins/k8s/... ./go/tasks/pluginmachinery/... -count=1

Raw output

Without the fix:

--- FAIL: TestBuildResource_DoesNotMutateSharedConfig (0.00s)
        	Error:      	Not equal:
        	            	expected: map[string]string{"disable-usage-stats":"true"}
        	            	actual  : map[string]string{"disable-usage-stats":"true", "node-ip-address":"$MY_POD_IP"}
        	Messages:   	BuildResource mutated the shared worker-node start parameters
--- FAIL: TestBuildResource_RayJobsDoNotShareStartParamsMap (0.00s)
        	Error:      	map[string]string{"dashboard-host":"0.0.0.0", "disable-usage-stats":"true", "include-dashboard":"true", "node-ip-address":"$MY_POD_IP", "num-cpus":"tainted-by-first-rayjob"} should not contain "num-cpus"
        	Messages:   	editing one RayJob's start params changed another RayJob's
        	Messages:   	editing a RayJob's start params changed the shared plugin config
FAIL
FAIL	github.com/flyteorg/flyte/flyteplugins/go/tasks/plugins/k8s/ray	0.775s

With the fix:

ok  	github.com/flyteorg/flyte/flyteplugins/go/tasks/plugins/k8s/ray	0.807s	coverage: 76.3% of statements

The wider run is green too:

ok  	github.com/flyteorg/flyte/flyteplugins/go/tasks/plugins/k8s/ray	4.869s
ok  	github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/catalog	6.950s
ok  	github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/flytek8s	1.225s
ok  	github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/internal/webapi	10.447s

Labels

fixed

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.23%. Comparing base (2c7f086) to head (befe83f).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7851   +/-   ##
=======================================
  Coverage   57.22%   57.23%           
=======================================
  Files         931      931           
  Lines       58269    58269           
=======================================
+ Hits        33347    33349    +2     
  Misses      21859    21859           
+ Partials     3063     3061    -2     

Flag	Coverage Δ
unittests-datacatalog	53.51% <ø> (ø)
unittests-flyteadmin	53.23% <ø> (ø)
unittests-flytecopilot	48.05% <ø> (ø)
unittests-flytectl	64.09% <ø> (ø)
unittests-flyteidl	75.71% <ø> (ø)
unittests-flyteplugins	60.42% <100.00%> (+0.02%)	⬆️
unittests-flytepropeller	53.78% <ø> (ø)
unittests-flytestdlib	64.39% <ø> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pingsutw]

thank you. would you mind update the v2 plugin on main as well?