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

Enable passing cloud-init config via stdin #628

Merged
merged 5 commits into from
Feb 28, 2019
Merged

Enable passing cloud-init config via stdin #628

merged 5 commits into from
Feb 28, 2019

Conversation

gerboland
Copy link
Contributor

No description provided.

Copy link
Contributor

@townsend2010 townsend2010 left a comment

Choose a reason for hiding this comment

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

Cool! A couple of comments already and I'll test the functionality a little later.

src/client/cmd/launch.cpp Outdated Show resolved Hide resolved
src/client/cmd/launch.cpp Show resolved Hide resolved
Copy link
Contributor

@townsend2010 townsend2010 left a comment

Choose a reason for hiding this comment

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

Good stuff here. However, if a user puts in incorrect YAML, yaml-cpp will throw and then the output passed back to the client is kind of vague.

For example, I did this:

$ multipass launch --cloud-init -
foo
man
chu<Ctrl-D>
launch failed: operator[] call on a scalar

Not a very helpful message as it's not clear where that really came from (although in this extreme case, it is:)) But I imagine a slight error in the YAML may produce a similar error message and then it's not so clear.

My thinking is that we put the YAML::Load in launch() and perhaps prepare_user_data() in a try/catch block so we can catch this and translate it into at least a better error message indicating it's coming from the cloud-init YAML parsing.

Copy link
Contributor

@townsend2010 townsend2010 left a comment

Choose a reason for hiding this comment

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

@gerboland,

After thinking about it, the yaml-cpp exceptions I mentioned should be handled as a separate issue as that can impact a passed in file as well and is out of scope for this PR. As such, this is good and I'm approving.

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.

3 participants