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

libazureinit: get mounted CDROM device at runtime #79

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

dongsupark
Copy link
Collaborator

Get mounted device name of CDROM at runtime, instead of hard-coding /dev/sr0.

Fixes #66

Testing done

TBD

@dongsupark
Copy link
Collaborator Author

CI fails because the block-utils crate depends on libudev-dev package, which is not installed by Default on the default Ubuntu runner.
actions-rs cargo repo is meanwhile archived, so not likely to support such a feature.
Will look into the path forwards tomorrow.

@dongsupark
Copy link
Collaborator Author

Fixed the CI issue, it was easier than expected.

@dongsupark dongsupark marked this pull request as ready for review April 10, 2024 10:08
@dongsupark
Copy link
Collaborator Author

Fixed a syntax error in clippy-linting, which was made by #78.

libazureinit/src/media.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

let ovf_body = mounted.read_ovf_env_to_string()?;
let environment = media::parse_ovf_env(ovf_body.as_str())?;
// list of CDROM devices that is available with possible filesystems.
let ovf_devices = media::get_mount_device()?;
Copy link

Choose a reason for hiding this comment

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

consider decoupling the mount/ovf logic out of get_username()

is the cdrom only going to mounted if we detect password enabled via IMDS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds like a good idea.
I would say, let's do that in another PR, because I don't want to leave the bug fixes in other commits unmerged for so long time. Ok?

Copy link
Member

Choose a reason for hiding this comment

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

I agree it'd be good to move it out, and also that it's okay for it to be in another PR. I have a WIP branch tidying some of the APIs in this area anyway so I could probably add a commit moving things around here.

libazureinit/src/media.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
libazureinit/src/error.rs Outdated Show resolved Hide resolved
Get mounted device name of CDROM at runtime, instead of hard-coding
/dev/sr0. Valid filesystems would include iso9660 and udf.
Install libudev-dev package in the default Ubuntu runner, as it is
needed by block-utils Rust crate.
Fix syntax error in clippy-linting.yml, like "every step must
define a `uses` or `run` key".
Copy link
Member

@jeremycline jeremycline left a comment

Choose a reason for hiding this comment

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

@dongsupark dongsupark merged commit dd6fef6 into Azure:main Apr 17, 2024
3 checks passed
@dongsupark dongsupark deleted the cdrom-mount-device branch April 17, 2024 14:34
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.

azure-init should not assume provisioning media is always at /dev/sr0
4 participants