Move the overlay snapshotter over to using the WithTransaction convenience
method. This simplifies needing to check if we need to rollback a transaction
and saves us from needing to manually Commit ourselves.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Most snapshotters end up manually handling the rollback logic, either
by calling `t.Rollback()` in every failure path, setting up a custom
defer func to log on certain errors, or just deferring `t.Rollback()`
even for `snapshotter.Commit()` which *will* cause `t.Rollback()` to return
an error afaict, but it's just never checked and luckily bolt handles this
alright...
The devmapper snapshotter has a solution to this which is to have a
method that starts either a read-only or writable transaction inside
the method, and you pass in a callback to do your bidding and any
failures are rolled back, and if it's writable will handle the commit
for you. This seems like the right model to me, it removes the burden
from the snapshot author to remember to either defer/call rollback
in every method for every failure case.
This change exposes the convenience method from devmapper to the
snapshots/storage package as a method off of `storage.MetaStore` and moves
over the devmapper snapshotter to use this.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Cleaning up TODO's. If we're on >= 5.11 we need userxattr so check
the kernel version to skip the manual check via mounting. It feels
odd to use contrib/seccomp here but the alternative is pulling that
kernel parsing code out into the main pkgs. Another is using the moby
parser but that's in moby/moby which is also a dep we don't want here..
Signed-off-by: Danny Canter <danny@dcantah.dev>
`ioutil` has been deprecated by golang. All the code in `ioutil` just
forwards functionality to code in either the `io` or `os` packages.
See https://github.com/golang/go/pull/51961 for more info.
Signed-off-by: Jeff Widman <jeff@jeffwidman.com>
This change does a couple things to remove some cruft/unused functionality
in the Windows snapshotter, as well as add a way to specify the rootfs
size in bytes for a Windows container via a new field added in the CRI api in
k8s 1.24. Setting the rootfs/scratch volume size was assumed to be working
prior to this but turns out not to be the case.
Previously I'd added a change to pass any annotations in the containerd
snapshot form (containerd.io/snapshot/*) as labels for the containers
rootfs snapshot. This was added as a means for a client to be able to provide
containerd.io/snapshot/io.microsoft.container.storage.rootfs.size-gb as an
annotation and have that be translated to a label and ultimately set the
size for the scratch volume in Windows. However, this actually only worked if
interfacing with the CRI api directly (crictl) as Kubernetes itself will
fail to validate annotations that if split by "/" end up with > 2 parts,
which the snapshot labels will (containerd.io / snapshot / foobarbaz).
With this in mind, passing the annotations and filtering to
containerd.io/snapshot/* is moot, so I've removed this code in favor of
a new `snapshotterOpts()` function that will return platform specific
snapshotter options if ones exist. Now on Windows we can just check if
RootfsSizeInBytes is set on the WindowsContainerResources struct and
then return a snapshotter option that sets the right label.
So all in all this change:
- Gets rid of code to pass CRI annotations as labels down to snapshotters.
- Gets rid of the functionality to create a 1GB sized scratch disk if
the client provided a size < 20GB. This code is not used currently and
has a few logical shortcomings as it won't be able to create the disk
if a container is already running and using the same base layer. WCIFS
(driver that handles the unioning of windows container layers together)
holds open handles to some files that we need to delete to create the
1GB scratch disk is the underlying problem.
- Deprecates the containerd.io/snapshot/io.microsoft.container.storage.rootfs.size-gb
label in favor of a new containerd.io/snapshot/windows/rootfs.sizebytes label.
The previous label/annotation wasn't being used by us, and from a cursory
github search wasn't being used by anyone else either. Now that there is a CRI
field to specify the size, this should just be a field that users can set
on their pod specs and don't need to concern themselves with what it eventually
gets translated to, but non-CRI clients can still use the new label/deprecated
label as usual.
- Add test to cri integration suite to validate expanding the rootfs size.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
When upperdirLabel specified, overlay Update will throw tx closed error since Commit is invoked before GetInfo
Signed-off-by: cardy.tang <zuniorone@gmail.com>
Previouslty "Size" was reserved by protoc-gen-gogoctrd and user-generated
"Size" was automatically renamed to "Size_" to avoid conflicts.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
This commit removes the following gogoproto extensions;
- gogoproto.nullable
- gogoproto.customename
- gogoproto.unmarshaller_all
- gogoproto.stringer_all
- gogoproto.sizer_all
- gogoproto.marshaler_all
- gogoproto.goproto_unregonized_all
- gogoproto.goproto_stringer_all
- gogoproto.goproto_getters_all
None of them are supported by Google's toolchain (see #6564).
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
While executing mke2fs, 'Not enough space to build proposed filesystem while setting up superblock' error is happend on Ubuntu20.04
Signed-off-by: Shinichi Morimoto <shnmorimoto@gmail.com>
This commit removes gogoproto.enumvalue_customname,
gogoproto.goproto_enum_prefix and gogoproto.enum_customname.
All of them make proto-generated Go code more idiomatic, but we already
don't use these enums in our external-surfacing types and they are anyway
not supported by Google's official toolchain (see #6564).
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
The directory created by `T.TempDir` is automatically removed when the
test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Two xfs file systems with same UUID can not be mounted on the same system.
However devmapper snapshots will have same UUID as original filesystem.
This patch fixes the bug by mounting a xfs file system with "nouuid" option.
Signed-off-by: Henry Wang <henwang@amazon.com>
Add file system options for config file, so that user can use
non-default file system parameters for the fs type of choice
Using file system options in config file overwrites the default
options already being used.
Signed-off-by: Alakesh Haloi <alakeshh@amazon.com>
