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

Draft: Cloud-init iso support #6

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Conversation

Toasterson
Copy link

No description provided.

@jclulow jclulow marked this pull request as draft June 14, 2022 01:42
Copy link
Member

@jclulow jclulow left a comment

Choose a reason for hiding this comment

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

I have taken a brief look at this. Some initial thoughts and questions!

Cargo.toml Outdated
@@ -2,7 +2,7 @@
name = "illumos-metadata-agent"
description = "Cloud metadata bootstrap software for illumos systems"
version = "0.1.0"
authors = ["Joshua M. Clulow <[email protected]>"]
authors = ["Joshua M. Clulow <[email protected]>", "Till Wegmueller <[email protected]>"]
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 just delete the authors field here, as it's going away for the most part.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, good to know, I'll do that

Cargo.toml Outdated
tempfile = "3"
anyhow = "1"
slog = "2.5"
slog-term = "2.5"
atty = "0.2"
failure = "0.1"
Copy link
Member

Choose a reason for hiding this comment

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

Rather than failure, I'd prefer to use thiserror where we need errors that can be handled programmatically, and continue to use anyhow where we do not.

(NB: failure is deprecated)

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I switched in other projects but this code sat around a bit. Will change over.

let mut count = 0;
for line_result in BufReader::new(output.stdout.as_slice()).lines() {
if let Ok(line) = line_result {
if count == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

You can use lines().enumerate() to get the count as part of the iterator.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks will do!

@@ -0,0 +1,101 @@
use crate::common::*;
Copy link
Member

Choose a reason for hiding this comment

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

This file should probably retain the Oxide Copyright notice from where the methods originated?

Copy link
Author

Choose a reason for hiding this comment

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

I am trying to remember if I took it wholesale or not but probably did. Will add it.

@@ -0,0 +1,16 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Where does this script get used?

Copy link
Author

Choose a reason for hiding this comment

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

The rust community probably does not know the concept as this originates from golang. The hack folder has helpers people can use during development. This script was made to ease testing of features that require network. It could be reworked into a SMF service or included into metadata-agent.

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

Successfully merging this pull request may close these issues.

2 participants