feat: upgrade existing filesystem

[upils]

• Have you signed the CLA?

---

This commit enables Chisel upgrading an existing rootfs.
It detects the target directory contains the result of a previous execution
to then operate an upgrade of the content. This initial simple implementation
has the following limitationg:

• entries in the rootfs is not verified against the existing manifest.
• entries (files, symlinks) are systematically replaced, even if identical to new ones.

As a consequences:

• User modifications on chisel-managed content are overridden.
• In the context of an OCI image build, this "new" content is identified as
  different and thus the new OCI layer contains duplicated files.

[github-actions]

Command	Mean [s]	Min [s]	Max [s]	Relative
BASE	13.357 ± 0.114	13.191	13.522	1.01 ± 0.01
HEAD	13.280 ± 0.109	13.173	13.509	1.00

[letFunny]

First review. I am missing a couple of files but I wanted to publish it as quickly as possible so we have more time to iterate. This is looking great Paul, many things are looking dead simple which is always an extremely good sign. I have written some comments about how to make the PR shorter and how to make it, hopefully, even simpler. Let me know what you think.

I see some of the comments are similar to past discussions we had in upils#1. We spent some effort there, so please make sure all the comments were carried over to this one.

[letFunny]

Thank you Paul for all the back and forth and all the effort put in the PR. I added some nitpicky comments but looking at the whole PR I cannot spot anything big pending; I see some areas where I expect minor discussion, but in general it looks very good. I think it is ready for Gustavo to review, so please resolve the comments in the PR as I don't have permission to do so, and move it to the review queue.

[polarathene]

• In the context of an OCI image build, this "new" content is identified as different and thus the new OCI layer contains duplicated files.

FWIW: If using SOURCE_DATE_EPOCH with a supported builder to rewrite timestamps of contents in a layer to the epoch, identical files should no longer contribute additional size IIRC.

[upils]

@polarathene thanks for your suggestion.

However this env var is solving a slightly different problem. My understanding is that it is used in the context of reproducible builds, in order to always produce the same layers. It is then useful in registries to avoid duplicated layers between different images.

In our case, we try to minimize the differences between layers of a single image, by trying to only write the meaningful changes.
To give you more context, we are trying to accommodate the behavior of https://docs.kernel.org/filesystems/overlayfs.html (and its FUSE equivalent https://github.com/containers/fuse-overlayfs). As outlined in the description, this first iteration is indeed not optimized to avoid duplicated content but this is our end goal.

[letFunny]

Thanks Paul, I like the new comments. And thank you for postponing the previous change, this PR is long enough as it is.

[niemeyer]

Here is a first pass.

For future PRs, let's please try to break it down into smaller parts so it's lighter on you and on the reviewer to get it in quickly.

[niemeyer]

This must never ever happen if err != nil. This is an important invariant to keep in mind in every Go project we work on. If we skip this rule, we're straight into billion Euro mistake territory. If you asked to select a valid manifest, and it did not return an error, it must have selected a valid manifest!

Let's please fix the function implementation and drop this verification from every call site.

cc @letFunny

[letFunny]

I completely agree. I brought the same point in the past out of muscle memory but I couldn't find any document to justify it properly. Maybe we should include it in the go style document that Ed wrote here.

[upils]

I reworked it and added a custom error type. In cmd_cut.go the error message is not used but I wonder if it should be logged or not.

[niemeyer]

This is too loose. Imagine what happens if path is "/hmm/oops**", for instance, or just "/oops".

We need to be strict about how a manifest path looks like. Hopefully this is already the case elsewhere so there's already a pattern to follow. If not, please let me know.

[upils]

Following our discussion offline, see 85ea2b6. The validation function is heavily inspired from validateGeneratePath in internal/setup/yaml.go.

[niemeyer]

Can we please try harder to make this logic look nice?

As a hint, it often pays off to resist the temptation of manipulating important core functions, or long functions, and sprinkling it with previously missing needs of new functionality. Instead, it's worth considering what it is that you're adding new, and then consider which specific abstractions are needed for it.

[niemeyer]

There are more convenient APIs inside that package.

[niemeyer]

As Alberto says, we never promise to keep error messages intact. With that said, we can do better here because I still want to know what the unsupported schema was.

Let's please have an actual error type called UnknownSchemaError that contains the wrong schema version as a public Version field, and reports the message "unknown manifest schema version %q".