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

cloudinit: leverage virtio-blk to push cloud-init configuration #207

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

lstocchi
Copy link
Contributor

@lstocchi lstocchi commented Oct 11, 2024

This PR updates the documentation to explain how to push cloud-init configuration as an ISO image by leveraging virtue-blk.

It also adds an e2e test to verify vfkit works with fedora and the cloud-init configuration is actually applied.

it closes #123

Copy link

openshift-ci bot commented Oct 11, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@lstocchi lstocchi marked this pull request as ready for review October 15, 2024 09:26
@lstocchi lstocchi removed the request for review from anjannath October 15, 2024 09:26
@lstocchi lstocchi changed the title cloudinit: add --device cloud-init,path support cloudinit: leverage virtue-blk to push cloud-init configuration Oct 17, 2024
@cfergeau cfergeau changed the title cloudinit: leverage virtue-blk to push cloud-init configuration cloudinit: leverage virtio-blk to push cloud-init configuration Oct 18, 2024
Copy link
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

Overall looks good apart from a bunch of minor comments.

doc/usage.md Outdated
@@ -158,6 +157,20 @@ made to the copy-on-write image, and not to the backing file. Only the
modified data will use actual disk space.
A copy-on-write image can be created using `cp -c` or [clonefile(2)](http://www.manpagez.com/man/2/clonefile/).

#### Cloud-init

The `--device virtio-blk` option could also be used to supply an initial configuration to cloud-init through an disk image.
Copy link
Collaborator

Choose a reason for hiding this comment

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

can also be used... a disk image

doc/usage.md Outdated
It is also possible to add further configurations by using the network-config and vendor-data files.
See https://cloudinit.readthedocs.io/en/latest/reference/datasources/nocloud.html#runtime-configurations for more details.

To create the ISO image you could use the following command within a folder containing the user-data and meta-data files
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd use "can" here as well.

doc/usage.md Show resolved Hide resolved
doc/usage.md Outdated Show resolved Hide resolved
doc/usage.md Outdated
@@ -170,6 +183,11 @@ This adds a virtio-blk device to the VM which will be backed by the raw image at
--device virtio-blk,path=/Users/virtuser/vfkit.img
```

To also push the cloud-init configuration you can add an additional virtio-blk device backed by an image containing the cloud-init configuration files
Copy link
Collaborator

Choose a reason for hiding this comment

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

"push" reads a bit odd here, maybe "provide" would work?

}
if _, err := file.Seek(0, 0); err != nil {
return "", err
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this header reading required? you do not seem to be making use of it?

@@ -64,6 +121,16 @@ func NewPuipuiProvider() *PuiPuiProvider {
return &PuiPuiProvider{}
}

type FedoraProvider struct {
vmlinuz string
Copy link
Collaborator

Choose a reason for hiding this comment

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

diskImage or such would be easier to understand.

it adds the documentation to explain how to push cloud-init configuration by leveraging the virtio-blk device

Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
test/osprovider.go Outdated Show resolved Hide resolved
it adds an e2e test which downloads fedora 40 and create a VM using it. Then mounts an ISO image containing the user-data and meta-data files needed by cloud-init to apply our configuration.
The meta-data file is empty but the user-data file contains info about a new user that has to be added to the system.
After the VM gets started it verifies that the user has been actually added by connecting using SSH.
At the end, the VM is stopped.
The test is not run on macOS12 as it does not support efi bootloader.

Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
Copy link
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link

openshift-ci bot commented Oct 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 4f9ec1f into crc-org:main Oct 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add --cloud-init cfgfile support
2 participants