Instead of requiring callers to read the struct fields to check for an
error, provide the exit results via a function instead which is more
natural.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
In all of the examples, its recommended to call `Wait()` before starting
a process/task.
Since `Wait()` is a blocking call, this means it must be called from a
goroutine like so:
```go
statusC := make(chan uint32)
go func() {
status, err := task.Wait(ctx)
if err != nil {
// handle async err
}
statusC <- status
}()
task.Start(ctx)
<-statusC
```
This means there is a race here where there is no guarentee when the
goroutine is going to be scheduled, and even a bit more since this
requires an RPC call to be made.
In addition, this code is very messy and a common pattern for any caller
using Wait+Start.
Instead, this changes `Wait()` to use an async model having `Wait()`
return a channel instead of the code itself.
This ensures that when `Wait()` returns that the client has a handle on
the event stream (already made the RPC request) before returning and
reduces any sort of race to how the stream is handled by grpc since we
can't guarentee that we have a goroutine running and blocked on
`Recv()`.
Making `Wait()` async also cleans up the code in the caller drastically:
```go
statusC, err := task.Wait(ctx)
if err != nil {
return err
}
task.Start(ctx)
status := <-statusC
if status.Err != nil {
return err
}
```
No more spinning up goroutines and more natural error
handling for the caller.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
In order to enforce strict handling of snapshotter values on the
container object, the defaults have been moved to the client side. This
ensures that we correctly qualify the snapshotter under use when from
the container at the time it was created, rather than possibly losing
the metadata on a change of default.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This fixes a few bugs in the container store related to reading and
writing fields. Specifically, on update, the full field set wasn't being
returned to the caller, making it appear that the store was corrupted.
We now return the correctly updated field and store the missing field
that was omitted in the original implementation. In course, we also have
defined the update semantics of each field, as well as whether or not
they are required.
The big addition here is really the container metadata testsuite. It
covers listing, filtering, creates, updates and deletes in a vareity of
scenarios.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This reverts commit 06dc87ae59.
Revert "Change oom metric to const"
This reverts commit e800f08f9f.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Ensure all writers are closed at end of test for content
test suite. Prevents test from leaving lingering connections
to the content store.
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Fixes#1389
This reverts waiting on stdin to finish its IO copy before returning
out.
This was causing `ctr` to block for every container start/exit.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Because we merged the PR that added the API descriptors and modified the
API at the same time. This updates the protobufs to be consistent.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This removes the metric vec that was holding onto all task id and
namespace combinations forever, until containerd was restarted. This
was causing a memory leak with many task.
This also removes the shim cmd where the `Args` is quite large from the
reaper after the shim has been started cutting down on another leak.
This is the first pass through the reaper but more code is required to
fix all the issues when commands are added.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Make sure we wait for the client side copy goroutines to start coping
from the fifos before returning from the function.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Previously "`/`" in a container was always owned by `root:root` (0/0), even if
`withRemappedSnapshot` had been used. Meaning that if `withUserNamespace` is
used then `/` can be remapped to `nobody:nogroup` (65534/65534).
The fix is is twofold:
- incrementFS should operate on the root of the tree.
- when creating a new snapshot we must propagate the ownership of the topmost
"lower" directory into the new "upper".
Signed-off-by: Ian Campbell <ian.campbell@docker.com>
We were not checking the error value of `Kill` leading to deadlock if the
process didn't exist.
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
