-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update bash scripting, remove jq requirement, add nvme mounting #648
Conversation
I recall this being discussed. I don't recall if an issue was opened. I'll go over this (and merge with main, since I'm the last to touch the start-task and I'm certain that's where the merge conflicts mainly are) on Monday. @jlester-msft could you please respond here with the issue number? Thank you! |
The L-series NVME issue is #454. That issue was asking for tes-runner to do this; however, we can have bash do this for now. I don't think we need to spend the dev time to code this into tes-runner right now. |
@jlester-msft This has been merged with |
Co-authored-by: Blair L Murri <[email protected]>
Co-authored-by: Blair L Murri <[email protected]>
fixes #616
fixes #454
This code change makes 3 main changes to
TesApi.Web/BatchScheduler.cs
:@
strings. Which makes it much easier to read and work with. Note thatset -e
makes it so any command which fails during bash script execution will trigger an exit. This means some commands like the grep innvme_devices=$(nvme list -o json | grep -oP ""\""DevicePath\"" : \""\K[^\""]+"" || true)
needs a|| true
so that if no match is found the script doesn't end. However, most of the other commands don't require&&
chaining anymore.${AZ_BATCH_NODE_ROOT_DIR}/tasks/workitems
(/mnt/batch/tasks/workitems
). Software RAID seems performant and lvm and mdadm RAID0 devices seem to have similar performance. mdadm is used as it is the easier/more mature solution.These changes have been tested on a few large example workflows. And on some L-series test workloads. Could require additional hardening but seems to be working well enough at the moment.
@BMurri + @MattMcL4475