[worker] Wire SSH session into build - #4031
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## gwdp/eng-24547-build-tools-ssh-session #4031 +/- ##
==========================================================================
+ Coverage 63.95% 63.97% +0.03%
==========================================================================
Files 1033 1034 +1
Lines 47747 47778 +31
Branches 10043 10046 +3
==========================================================================
+ Hits 30530 30561 +31
Misses 17116 17116
Partials 101 101 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
38b2178 to
d311a40
Compare
534b4f1 to
a7b6154
Compare
d311a40 to
1bf156f
Compare
a7b6154 to
c9e54b9
Compare
c1bfc6f to
f7e016d
Compare
12f0a14 to
12feb7a
Compare
f7e016d to
c590f2f
Compare
12feb7a to
cb90fa2
Compare
c590f2f to
f04922c
Compare
cb90fa2 to
9409c52
Compare
f04922c to
1c89f46
Compare
9409c52 to
daee76d
Compare
1c89f46 to
3e17793
Compare
daee76d to
7154d05
Compare
3e17793 to
1e58c28
Compare
7154d05 to
420bbef
Compare
1e58c28 to
aff1e33
Compare
420bbef to
3dc29fa
Compare
aff1e33 to
8fe0c84
Compare
de1bff6 to
f6de25e
Compare
0297f39 to
0859dec
Compare
d5d5f1a to
c4bce9c
Compare
0859dec to
bb0577f
Compare
c4bce9c to
2aa0559
Compare
bb0577f to
81a5ffa
Compare
81a5ffa to
0eabaa0
Compare
sjchmiela
left a comment
There was a problem hiding this comment.
ugh sorry i had this review from 4 days ago not sent
7149fa3 to
62efd17
Compare
62efd17 to
7b94ba2
Compare
2e5bca4 to
61e8f23
Compare
4206350 to
2dad43c
Compare
sjchmiela
left a comment
There was a problem hiding this comment.
this is good, i think the code layout due to the stack nature of the feature has grown a bit too wide, but it looks great
| done = (async () => { | ||
| let result = BuildPhaseResult.SUCCESS; | ||
| try { | ||
| await TurtleSshSession.superviseSshSessionAsync({ |
There was a problem hiding this comment.
maybe the supervisesshsessionasync should be inlined here?
There was a problem hiding this comment.
or at least in this file
There was a problem hiding this comment.
Leaning toward keeping it as its own function on purpose. The full supervise loop has its own unit tests but also would bloat this file. Let me know if you are soft on this 🙇♂️
Same anti-flatten object wrapper; clearer that callers await tunnel teardown.
build-tools resolves upterm from PATH or downloads darwin-arm64 / linux-amd64 from gs://turtle-v2/upterm at runtime.
|
✅ Thank you for adding the changelog entry! |

Why
Wiring SSH into the worker build lifecycle: when a job has SSH enabled, open the session in
SSH_SESSIONand keep the VM up until supervision says it is safe to tear down.How
build(), pass ahasJobFinishedsignal into the supervisor, andawaitteardown infinallySSH_SESSIONopen in the log UI for the life of the tunnel (doNotMarkEnd, emit END on teardown)gs://turtle-v2/uptermvia build-tools; this PR does not vendor binaries into the worker packageTest Plan
Worker unit tests for the SSH phase. CI