Skip to content

RFC: ostree commit: Add option --tree=prefix=PREFIX#1651

Open
wmanley wants to merge 1 commit intoostreedev:mainfrom
stb-tester:commit-prefix
Open

RFC: ostree commit: Add option --tree=prefix=PREFIX#1651
wmanley wants to merge 1 commit intoostreedev:mainfrom
stb-tester:commit-prefix

Conversation

@wmanley
Copy link
Member

@wmanley wmanley commented Jun 27, 2018

This enhances ostree commit's ability to compose trees. You can now run

ostree commit -b rootfs \
    --tree=ref=rootfs \
    --tree=prefix=/usr/lib/machines/container --tree=ref=container

and that will "mount" the container ref under usr/lib/machines/container
in rootfs. It is much faster than checking the tree out and checking it
back in again.

This is useful for our buildsystem where we ship containers in the rootfs image.

I'm still not sure of the UI. I'm hoping this PR will spur on discussion.

Alternatives

  • Have a single switch that affects all --tree options. The above incantation would become:

      ostree commit -b rootfs \
          --tree=ref=$(ostree commit --orphan --prefix=/usr/lib/machines/container --tree=ref=container) \
          --tree=ref=rootfs
    

TODO:

  • Don't hard code dirmeta_0755_0_0, write it explicitly instead

@jlebon
Copy link
Member

jlebon commented Jun 27, 2018

Hmm, a similar idea I've had was to copy the features of checkout --union/--union-add/--union-identical to commit. Then you could do e.g.

ostree commit --union --tree=ref=rootfs --tree=dir=overlay-rootfs

@wmanley
Copy link
Member Author

wmanley commented Jun 28, 2018

Hmm, a similar idea I've had was to copy the features of checkout --union/--union-add/--union-identical to commit

On that subject: I started implementing support for overlaying upper directories from overlayfs as part of ostree commit:
stb-tester@34dce7d

I stopped when I discovered that you need CAP_SYS_ADMIN to read the xattrs that overlayfs writes. I'm taking a different approach to make that fast.

@jlebon
Copy link
Member

jlebon commented Sep 19, 2018

Hmm OK, one thing I missed at first is that --tree=prefix affects every --tree afterwards, not just the next one, until the next --tree=prefix. Which is fine I suppose, though it's not clear to me whether the common case would be passing multiple tree at the same prefix, or different ones. In the latter case, it might be cleaner to come up with a syntax that unifies them into a single arg, e.g. something like:

ostree commit --tree=ref=foobar --subtree=/usr/lib/mysubtree:ref=baz

?

That way they're together so it's easier to tell and harder to get wrong/break during a refactor (and if someone really wants to target a subtree that has :, they should just use the API :)).

Hmm, a similar idea I've had was to copy the features of checkout --union/--union-add/--union-identical to commit.

Thinking more about this, I think they're mostly orthogonal and could come after if the need arises. Right now, commit args have --union semantics already; existing files in the mutable tree are overwritten. So my thought was to complement that with a --union-add, --union-identical, and a e.g. --no-union mode, where it errors out if trying to write a file that already exists (the equivalent of the default checkout mode).

@jlebon
Copy link
Member

jlebon commented Sep 19, 2018

bot, retest this please

@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 9367a1b) made this pull request unmergeable. Please resolve the merge conflicts.

This enhances `ostree commit`'s ability to compose trees.  You can now run

    ostree commit -b rootfs \
        --tree=ref=rootfs \
        --tree=prefix=/usr/lib/machines/container --tree=ref=container

and that will "mount" the container ref under usr/lib/machines/container
in rootfs.  It is much faster than checking the tree out and checking it
back in again.

This is useful for our buildsystem where we ship containers in the rootfs
image.
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wmanley
To complete the pull request process, please assign cgwalters
You can assign the PR to them by writing /assign @cgwalters in a comment when ready.

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

Details 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-ci-robot
Copy link
Collaborator

@wmanley: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented Jun 29, 2023

@wmanley: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/sanity 4779209 link true /test sanity
ci/prow/images 4779209 link true /test images
ci/prow/fcos-e2e 4779209 link true /test fcos-e2e

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

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.

4 participants