-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
test composefs on rawhide #22425
test composefs on rawhide #22425
Conversation
Thanks @edsantiago LGTM |
Ephemeral COPR build failed. @containers/packit-build please check. |
c59cb11
to
0abdfd7
Compare
Failures so far:
|
Tacked in #22422 Maybe file new issues for the other ones |
I'm like 90% sure that 628 is a valid integer, but I don't have enough fingers and toes to confirm. |
569e322
to
d694396
Compare
9b2045e
to
1dfde92
Compare
0ae6ca6
to
870b3db
Compare
870b3db
to
1f9d502
Compare
1f9d502
to
e2b064c
Compare
This is with c-storage @ main as of this morning. |
that is expected. composefs doesn't work with rootless as it needs to mount EROFS |
69f7e01
to
b316833
Compare
4c382de
to
0360870
Compare
0360870
to
c24f2b3
Compare
Vendoring in c-image, storage, common @ main yields:
cc @mheon |
That is the docker update: #23093 |
48f5e78
to
803f791
Compare
Run root e2e & system tests using composefs on rawhide. Write magic settings to storage.conf. That part is easy. e2e tests, however, ignore storage.conf. They require everything to be specified on the command line. And "everything", in the case of composefs, includes a long complicated --pull-options string which in turn requires containers-storage PR 1966 which, as of this writing, is finally vendored into podman. Signed-off-by: Ed Santiago <santiago@redhat.com>
803f791
to
d4c0e7e
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.
This is ready for review, although I foresee (and welcome!) challenges to some of my decisions.
@@ -263,7 +263,7 @@ Set default `--storage-driver` value. | |||
|
|||
#### **STORAGE_OPTS** | |||
|
|||
Set default `--storage-opts` value. | |||
Set default `--storage-opt` value. |
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.
Completely unrelated to my PR, I'm just sneaking this in as a drive-by fix
// Look for STORAGE_OPTIONS_OVERLAY / STORAGE_OPTIONS_VFS | ||
extraOptions := os.Getenv("STORAGE_OPTIONS_" + strings.ToUpper(storageFs)) | ||
if extraOptions != "" { | ||
storageOptions += " --storage-opt " + extraOptions | ||
} |
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.
New behavior. I think it makes sense to require separate envariables for each storage fs, but please shoot down if you disagree.
STORAGE_FS = "overlay" //nolint:revive,stylecheck | ||
STORAGE_OPTIONS = "--storage-driver overlay" //nolint:revive,stylecheck | ||
ROOTLESS_STORAGE_FS = "overlay" //nolint:revive,stylecheck | ||
ROOTLESS_STORAGE_OPTIONS = "--storage-driver overlay" //nolint:revive,stylecheck |
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.
ROOTLESS_STORAGE_OPTIONS was nuked in #18544. (The rest of this block is whitespace-only. Same with the next two config_xxx.go files).
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.
LGTM
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, giuseppe, Luap99 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 |
Run root e2e & system tests using composefs on rawhide.
Write magic settings to storage.conf. That part is easy.
e2e tests, however, ignore storage.conf. They require everything
to be specified on the command line. And "everything", in the
case of composefs, includes a long complicated --pull-options
string which in turn requires containers-storage PR 1966
which, as of this writing, is finally vendored into podman.
Signed-off-by: Ed Santiago santiago@redhat.com