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

store: If ostree >= 2024.3, retain content in /var #602

Merged
merged 2 commits into from
Feb 11, 2024

Conversation

cgwalters
Copy link
Member

This is intended to pair with ostreedev/ostree#3166

If we detect a new enough ostree version, then by default don't remap
content in /var, assuming that ostree itself will handle it.

In order to unit test this (without depending on the ostree version
that happens to be on the host) add an API to the importer
which allows overriding the version.


@@ -471,6 +473,7 @@ impl ImageImporter {
proxy_img,
target_imgref: None,
no_imgref: false,
ostree_v2024_3: ostree::check_version(2024, 3),
Copy link
Member Author

Choose a reason for hiding this comment

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

Now of course, we in theory need to care about the case where a new ostree is deploying an old target. But personally, I don't care too much about this; we'll get the new ostree out pretty quickly. And also the emphasis on the bootc side is on self-installing operating systems, which don't have this problem.

Copy link
Member

@jmarrero jmarrero Feb 10, 2024

Choose a reason for hiding this comment

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

Does this mean we need to make sure not to ship ostree 2024.3 in any version of OCP that has 2024.2 or older and commit only to backports?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good topic to raise. There's a lot of complexity and nuance here. First, all of this code is really only relevant in the case of deploying a container image with content in /var.

Our stock FCOS/RHCOS images don't have any. Now until the previous change here in #569 we just dropped all container /var content on the floor.

This change is taking it from using systemd-tmpfiles with C+ where we have "add new files" semantics to "at most once" semantics.

Now that all said:

Now of course, we in theory need to care about the case where a new ostree is deploying an old target.

For OCP (and really most cases) we are always going from old ➡️ new, as one would expect.

So I don't think this is a problem.

The bigger thing to do here is to verify that these new semantics for /var work as expected before we do another release.

@cgwalters cgwalters marked this pull request as ready for review February 9, 2024 23:56
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

I plan to add another variant of this behavior in the future,
and having a proper structure is better than a single `bool`.
This is intended to pair with ostreedev/ostree#3166

If we detect a new enough ostree version, then by default don't remap
content in `/var`, assuming that ostree itself will handle it.

In order to unit test this (without depending on the ostree version
that happens to be on the host) add an API to the importer
which allows overriding the version.
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.

None yet

2 participants