-
Notifications
You must be signed in to change notification settings - Fork 170
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
Prep patches for layered image builds #3754
Conversation
Just noticed this while looking at related code.
Right now, `--oscontainer` is handled by a systemd unit which runs `rpm-ostree rebase` and then reboots the system. The main disadvantage with this is that it means we don't actually get any logging of the pivot step since it happens before SSH comes up. But also, I want to make that switch support local OCI archives, but the way it's currently implemented won't work with that since there's no way to `DropFile()` it before this unit kicks in. Move handling that arg to `runTest()` time.
As part of openshift/os#799, we'll want to be able to run tests against the layered image. We want to be able to do that by pointing at the local file instead of having to push it to a registry first (which in the pipeline usually happens at the end).
Pass through `jq .`; this fixes the indentation of one key. Also for consistency, use lowercase for the `$id` of the `path` property in artifacts.
8cc63c6
to
7580894
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superficial LGTM since I don't have a deep knowledge of the context
for _, m := range tcluster.Machines() { | ||
tcluster.RunCmdSyncf(m, "sudo rpm-ostree rebase --experimental %s", Options.OSContainer) | ||
m.Reboot() | ||
tcluster.RunCmdSyncf(m, "sudo rpm-ostree rebase --experimental %s", rebase_arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only distantly related, but I think we can drop the --experimental
now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not tested but LGTM
Split out of #3753.
See individual commit messages.