Skip to content
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

feat(pegboard): add container runner and manager #1144

Conversation

MasterPtato
Copy link
Contributor

@MasterPtato MasterPtato commented Sep 11, 2024

Fixes RVTEE-605
Fixes RVTEE-607

Changes

Copy link
Contributor Author

MasterPtato commented Sep 11, 2024

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @MasterPtato and the rest of your teammates on Graphite Graphite

Copy link
Contributor

graphite-app bot commented Sep 11, 2024

Your org requires the Graphite merge queue for merging into main

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

You can enable merging using labels in your Graphite merge queue settings.

Copy link

linear bot commented Sep 12, 2024

RVTEE-605

RVTEE-607

@MasterPtato MasterPtato force-pushed the 09-11-feat_pegboard_add_container_runner_and_manager branch from dcdba4c to b6008e8 Compare September 12, 2024 00:35
@MasterPtato MasterPtato force-pushed the 09-10-feat_pegboard_add_client_wf branch from f5ba448 to d86899e Compare September 14, 2024 01:38
@MasterPtato MasterPtato force-pushed the 09-11-feat_pegboard_add_container_runner_and_manager branch from b6008e8 to 1bcc0f3 Compare September 14, 2024 01:38
@MasterPtato MasterPtato marked this pull request as ready for review September 25, 2024 02:26
@MasterPtato MasterPtato force-pushed the 09-10-feat_pegboard_add_client_wf branch from d86899e to 458de12 Compare September 28, 2024 00:24
@MasterPtato MasterPtato force-pushed the 09-11-feat_pegboard_add_container_runner_and_manager branch from 1bcc0f3 to 91cd21c Compare September 28, 2024 00:24
@MasterPtato MasterPtato force-pushed the 09-10-feat_pegboard_add_client_wf branch from 458de12 to 784fa01 Compare September 29, 2024 21:46
@MasterPtato MasterPtato force-pushed the 09-11-feat_pegboard_add_container_runner_and_manager branch from 91cd21c to 070d36b Compare September 29, 2024 21:46
@MasterPtato MasterPtato force-pushed the 09-10-feat_pegboard_add_client_wf branch from 784fa01 to 4bc4443 Compare October 1, 2024 18:08
@MasterPtato MasterPtato force-pushed the 09-11-feat_pegboard_add_container_runner_and_manager branch from 070d36b to ba4b363 Compare October 1, 2024 18:08
This was referenced Oct 1, 2024
}

#[derive(Debug, Serialize, Deserialize)]
pub enum Stakeholder {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stakeholder name is odd

/// identify the reasons for program crashes from the container's output.
const MAX_PREVIEW_LINES: usize = 128;

fn main() -> anyhow::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should have a main_inner that catches errors and writes it to a file in order to catch container-runner errors

match command {
protocol::Command::StartContainer {
container_id,
image_artifact_url,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if we should pass the ats ips and let the container runner decide the ip. this makes RVTEE-625 super simple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will likely be the case down the line. Do you want me to implement it for MVP?

let container_path = ctx.container_path(self.container_id);

let mut env = vec![
(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need the *_META_* and lowercase stuff since we no longer use nomad. this was an arfifact of using nomad's meta functionality.

Copy link
Contributor

graphite-app bot commented Oct 9, 2024

Merge activity

NathanFlurry pushed a commit that referenced this pull request Oct 9, 2024
<!-- Please make sure there is an issue that this PR is correlated to. -->
Fixes RVTEE-605
Fixes RVTEE-607
## Changes

<!-- If there are frontend changes, please include screenshots. -->
@graphite-app graphite-app bot closed this Oct 9, 2024
@graphite-app graphite-app bot deleted the 09-11-feat_pegboard_add_container_runner_and_manager branch October 9, 2024 04:50
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