-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add a pre-flight action to ensure workers are able to execute actions #325
Add a pre-flight action to ensure workers are able to execute actions #325
Conversation
Dir string `short:"d" long:"dir" default:"plz-out/mettle" description:"Directory to run actions in"` | ||
Dir string `short:"d" long:"dir" default:"plz-out/mettle" description:"Base directory to run actions in"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor one I found while testing this: all the workers share the same temp directory in this setup. They don't normally clash because working dirs are named after the action digest, and we deduplicate actions, but it happens with the preflight check because it's the same one on each worker.
I don't think this would be an issue often otherwise, but I'm giving each its own directory for good measure.
// runPreflightAction runs a simple known action to ensure we can process them correctly. | ||
func (w *worker) runPreflightAction() error { | ||
log.Notice("Preparing pre-flight action...") | ||
const fileContents = "thirty-five ham and cheese sandwiches\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a lot of sandwiches! It's probably fine but worth keeping an eye on it whilst this gets rolled out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup agreed. We should be able to roll this out to just our staging environment first so it'll only be a few that get it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right but if we're only rolling it out to staging, won't there be fewer consumers? Perhaps we should take this straight to prod to avoid wasted sandwiches.
This seems useful while we're dealing with the Ubuntu Noble upgrade & machines that potentially aren't able to run things properly; this will cause them to die before they can become active & start poisoning the worker fleet.