AtomicWriteFile truly is atomic, it only changes the file
on success. So there's no point notifying other processes about
a changed file if we failed, they are going to see the same JSON data.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't call Validate() on a digest a second time if it returns an error
the first time, when it's easy to just remember the error it returned
the first time.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
We now use the golang error wrapping format specifier `%w` instead of the
deprecated github.com/pkg/errors package.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Adds AddNames and RemoveNames so operations which are invoked in parallel
manner can use it without destroying names from storage.
For instance
We are deleting names which were already written in store.
This creates faulty behavior when builds are invoked in parallel manner, as
this removes names for other builds.
To fix this behavior we must append to already written names and
override if needed. But this should be optional and not break public API
Following patch will be used by parallel operations at podman or buildah end, directly or indirectly.
Signed-off-by: Aditya R <arajan@redhat.com>
The "if err == nil" is always true, because
there are no prior assignments to err; but it
creates a new scope, making the "err = r.Save()"
call write to an ignored variable.
Remove the unnecessary condition to avoid that.
(Note that the code still continues on after a failure
to save, i.e. it returns a reference to a created in-memory-only
container along with the error value.)
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
currently it is not possible to delete a mapped layer as it is always
referenced by the image.
Change it allow deleting the mapping if it is not used by any
container or other layers.
It will be useful in CRI-O as a lot of thrown away containers will be
created and the mapped images would keep growing without a way of
cleaning them up.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This commit adds a new `NamesHistory` field to the `images.json`, which
is basically a deduped list of names the image had in the past. The
first entry of the list is the latest history entry.
The main use case for this feature is to tell the end-user which
names/tags an image had in the past if it does not contain any `names`
any more.
Detailed use case:
1. Pulling `image:v1` into the local registry: `names: [ "image:v1" ]`
2. Pushing a new image as `image:v1` into the remote registry
3. Pulling `image:v1` again into the local registry:
- first image: `names: [ "image:v1" ]`
- previous v1 image: `names: [], names-history: [ "image:v1" ]`
4. An consumer of the storage API can now process the image name and
still display `image` as REPOSITORY, like:
* Before:
```
> podman images
REPOSITORY TAG IMAGE ID CREATED SIZE
image v1 25b62d1b654a 13 seconds ago 2.07 kB
<none> <none> b134eff7b955 17 seconds ago 2.07 kB
```
* After:
```
> podman images
REPOSITORY TAG IMAGE ID CREATED SIZE
image v1 25b62d1b654a 13 seconds ago 2.07 kB
image <none> b134eff7b955 17 seconds ago 2.07 kB
```
5. Since the `NamesHistory` is a slice we would be able to tell the
end-user which names an image ID had before.
The change should be backwards compatible with previous versions of
containers/storage.
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Wrap the ID or the digest to ErrImageUnknown errors to avoid ambiguity
which image is unknown. Consumers of the storage library may have
multiple subsequent calls to the storage API where it can be unclear
which image is unknown. Wrapping the ID and digest attempts to avoid
this ambiguity.
Related-to: github.com/containers/libpod/issues/2979
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Use RLock() to lock stores that we know are read-only, and panic in
Lock() if we know that we're not a read-write lock.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a RecursiveLock() API to allow for recursive acquisitions of a
writer lock within the same process space. This is yet another
requirement for the copy-detection mechanism in containers/image where
multiple goroutines can be pulling the same blob. Having a recursive
lock avoids a complex synchronization mechanism as the commit order is
determinted by the corresponding index in the image.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Drop our dependency on the image library's manifest package by requiring
that callers pass its Digest() function to us as a callback. This makes
our CLI test/diagnostic tool calculate digests of s1 manifests
incorrectly, but that's not something that we were testing.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Remove compatibility hacks for older versions which didn't track size or
digest information for big data items, hopefully without any impact.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Don't attempt to remove conflicting names or finish layer cleanups if we
only have a read-only lock on layer or image stores, since doing either
means we'd have to modify the list of layers or images, and our lock
that we've obtained doesn't allow us to do that.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Clarify that Locker.Locked() checks if we have a write lock, since
that's what we care about whenever we check it.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Implement reader-writer locks to allow allow multiple readers to hold
the lock in parallel.
* The locks are still based on fcntl(2).
* Changing the lock from a reader to a writer and vice versa will block
on the syscall.
* A writer lock can be held only by one process. To protect against
concurrent accesses by gourtines within the same process space, use a
writer mutex.
* Extend the Locker interface with the `RLock()` method to acquire a
reader lock. If the lock is set to be read-only, all calls to
`Lock()` will be redirected to `RLock()`. A reader lock is only
released via fcntl(2) when all gourtines within the same process space
have unlocked it. This is done via an internal counter which is
protected (among other things) by an internal state mutex.
* Panic on violations of the lock protocol, namely when calling
`Unlock()` on an unlocked lock. This helps detecting violations in
the code but also protects the storage from corruption. Doing this
has revealed some bugs fixed in ealier commits.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
Change how we compute digests for BigData items with names that start
with "manifest" so that we use the image library's manifest.Digest()
function, which knows how to preprocess schema1 manifests to get the
right value, instead of just trying to finesse it.
Track the digests of multiple manifest-named items for images.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When removing an image, remove the image's mapped top layers before the
image's "main" top layer, in case the graph driver is hiding a
dependency between the mapped layers and the "real" one (as it's allowed
to do).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
I have experienced "layer not known" corruption triggered by concurrent
buildah/skopeo processes, and hopefully lock sanity checks will help to
prevent this kind of problem.
Signed-off-by: Zac Medico <zmedico@gmail.com>
Allow images to have multiple top layers which should only differ by
which UID/GID mappings are used in them, to make creating multiple
containres which use the same mappings faster.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When creating new Layers, Images, or Containers, only try to copy the
newly-created results if we actually created them.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Always copy slices and maps in Layer, Image, and Container structures
before handing them back to callers so that, even if they modify them
directly, they won't accidentally mess with our in-memory copies of
those fields in the copies of the structures that we're using.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add explicitly-settable digest values for images that are treated mostly
like the implicit digests that we track for manifests, for the sake of
v1 images which have manifests that need to be preprocessed before being
digested to produce an image's digest value. We'll still have the
digest of the unprocessed manifest, but that shouldn't hurt.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a "digest" of an image that is equal to the digest of its big data
item named "manifest", if it has one, that we can index and use for
locating images.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Teach image and container store types to also track the digests of "big
data" items that we have them store.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When we read itms from disk, if maps in the structures are empty, they
won't be allocated as part of the decoding process. When we
subsequently go to read or write something from such a map, make sure
it's been initialized.
Add some validation of names that we convert to file names, and of
digest values, so that we can be more precise about the error code we
return when there's a problem with the values.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
We need to be able to create images which consist of just a list of
manifests, and those don't contain layers, so relax CreateImage()'s
requirement that a layer be specified.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Take a guess at the final size of some slices that we build up item by
item, and try to allocate enough capacity for them before starting to
build them. It's probably not a big speedup, though.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
We already deduplicated names in Store.SetNames(), but we weren't also
doing that when creating layers, images, and containers, or in the
individual store SetNames() methods.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Use the standard library's "errors" package to create errors so that
backtraces in wrapped errors terminate at the point where the error was
first wrapped, and not at the line where we created the error, which
isn't as useful for troubleshooting.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Add a field to ImageOptions that allows a caller to specify a date of
creation when calling CreateImage(), if there's a value in the image
metadata that would be more useful than the default (which is "now" at
the time CreateImage() is called).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
When Delete:ing a layer or a container the code was always allocating a
new slice just to remove an element from the original slice.
Profiling cri-o with c/storage showed that doing it at every delete is
pretty expensive:
```
. . 309: newContainers := []Container{}
. . 310: for _, candidate := range r.containers
{
. . 311: if candidate.ID != id {
528.17kB 528.17kB 312: newContainers =
append(newContainers, candidate)
. . 313: }
. . 314: }
. . 552: newLayers := []Layer{}
. . 553: for _, candidate := range
r.layers {
. . 554: if candidate.ID != id {
1.03MB 1.03MB 555: newLayers =
append(newLayers, candidate)
. . 556: }
. . 557: }
. . 558: r.layers = newLayers
```
This patch just filters out the element to remove from the original
slice w/o allocating a new slice. After this patch, no memory overhead
anymore is shown in the profiler.
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
Add a Created field to Layer, Image, and Container structures that we
intialize when creating one of them.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Implement read-only versions of layer and image store interfaces which
allocate read-only locks and which return errors whenever a write
function is called (which should only be possible after a type
assertion, since they're not part of the read-only interfaces).
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Split the LayerStore and ImageStore interfaces into read-only and
write-only subset interfaces, and make the proper stores into unions of
the read-only and write-only method sets.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
We need to be able to acquire locks on storage areas which aren't
mounted read-write, which return errors when we attempt to open a file
in the mode where we can take write locks on them. This patch adds a
read-only lock type for use in those cases.
A given file can be opened for read-locking or write-locking, but not
both. Our Locker interface gains an IsReadWrite() method to let callers
tell the difference.
Based on patches by Dan Walsh <dwalsh@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Miloslav Trmač