Skip to content

fix port args - #14

Open
KubeKyrie wants to merge 1 commit into
RLinf:mainfrom
KubeKyrie:codex/fix-sshd-port-flag
Open

KubeKyrie wants to merge 1 commit into
RLinf:mainfrom
KubeKyrie:codex/fix-sshd-port-flag

Conversation

@KubeKyrie

Copy link
Copy Markdown

--port rather than -port

@lkygithub

Copy link
Copy Markdown
Collaborator

--port rather than -port

Hi @KubeKyrie , thanks for the PR and for the detailed write-up — and sorry for the
slow response.

Heads-up: we just merged a large internal iteration into main (#17), which
substantially reworks the Task execution path. As part of that change, the shell
scripts this PR patches (ray_head.sh / ray_worker.sh) have been removed, and
the sshd process is now declared in Go rather than assembled in shell:

  • entrypoint is rlark-tools supervisor, which builds a ProcessDeclaration
    (primary ray-launcher, optional sshd)
  • the sshd arguments are constructed in
    apps/rlark/pkg/agent/controllers/task/environment/kubernetes/ssh/ssh.go
    using long flags (--port=<n>, --authorized-key-file=<path>), so the
    -port / pflag shorthand mismatch you found can no longer occur
  • this is now covered by unit tests (ssh/ssh_test.go,
    supervisor/assembler_test.go)

So the root cause you reported is fixed on main, just via a different route.

Could you please:

  1. Rebase onto the latest main.
  2. Re-check whether anything from this PR is still missing after feature: runtime-neutral Task execution, Cluster resource, tenant-enforced APIs, and unified contracts #17.
  3. If something remains, update the PR accordingly and re-run the checks.
  4. If it's fully covered, feel free to close this PR — that's completely fine.

Either way, the report was genuinely valuable: it was a real regression
(the flag → pflag migration silently broke -port), and it was invisible in
practice because the failure happened in a backgrounded nohup process whose
error went to a container-local log file. Thanks for catching and documenting it.

Happy to answer any questions. Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants