This commit was automatically cherry-picked
by buildah-vendor-treadmill v0.3
from the buildah vendor treadmill PR, #13808
* Fix conflict caused by Ed's local-registry PR in buildah
* Wire in "new" --retry and --retry-delay, these existed for longer
but where non functional.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
A field we missed versus Docker. Matches the format of our
existing Ports list in the NetworkConfig, but only includes
exposed ports (and maps these to struct{}, as they never go to
real ports on the host).
Fixes https://issues.redhat.com/browse/RHEL-60382
Signed-off-by: Matt Heon <mheon@redhat.com>
These flags can affect the output of the HealtCheck log. Currently, when a container is configured with HealthCheck, the output from the HealthCheck command is only logged to the container status file, which is accessible via `podman inspect`.
It is also limited to the last five executions and the first 500 characters per execution.
This makes debugging past problems very difficult, since the only information available about the failure of the HealthCheck command is the generic `healthcheck service failed` record.
- The `--health-log-destination` flag sets the destination of the HealthCheck log.
- `none`: (default behavior) `HealthCheckResults` are stored in overlay containers. (For example: `$runroot/healthcheck.log`)
- `directory`: creates a log file named `<container-ID>-healthcheck.log` with JSON `HealthCheckResults` in the specified directory.
- `events_logger`: The log will be written with logging mechanism set by events_loggeri. It also saves the log to a default directory, for performance on a system with a large number of logs.
- The `--health-max-log-count` flag sets the maximum number of attempts in the HealthCheck log file.
- A value of `0` indicates an infinite number of attempts in the log file.
- The default value is `5` attempts in the log file.
- The `--health-max-log-size` flag sets the maximum length of the log stored.
- A value of `0` indicates an infinite log length.
- The default value is `500` log characters.
Add --health-max-log-count flag
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Add --health-max-log-size flag
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Add --health-log-destination flag
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
We were only splitting on tabs, not spaces, so we returned just a
single line most of the time, not an array of the fields in the
output of `ps`. Unfortunately, some of these fields are allowed
to contain spaces themselves, which makes things complicated, but
we got lucky in that Docker took the simplest possible solution
and just assumed that only one field would contain spaces and it
would always be the last one, which is easy enough to duplicate
on our end.
Fixes#23981
Signed-off-by: Matt Heon <mheon@redhat.com>
This is very similar to commit 3280da0500, we cannot check the state
then unlock to then lock again and do the action. Everything must
happen under one lock. To fix this move the code into the HTTPAttach
function in libpod. The locking here is a bit weird because attach
blocks for the lifetime of attach which can be very long so we must
unlock before performing the attach.
Fixes#23757
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This started off as an attempt to make `podman stop` on a
container started with `--rm` actually remove the container,
instead of just cleaning it up and waiting for the cleanup
process to finish the removal.
In the process, I realized that `podman run --rmi` was rather
broken. It was only done as part of the Podman CLI, not the
cleanup process (meaning it only worked with attached containers)
and the way it was wired meant that I was fairly confident that
it wouldn't work if I did a `podman stop` on an attached
container run with `--rmi`. I rewired it to use the same
mechanism that `podman run --rm` uses, so it should be a lot more
durable now, and I also wired it into `podman inspect` so you can
tell that a container will remove its image.
Tests have been added for the changes to `podman run --rmi`. No
tests for `stop` on a `run --rm` container as that would be racy.
Fixes#22852
Fixes RHEL-39513
Signed-off-by: Matt Heon <mheon@redhat.com>
The new golangci-lint version 1.60.1 has problems with typecheck when
linting remote files. We have certain pakcages that should never be
inlcuded in remote but the typecheck tries to compile all of them but
this never works and it seems to ignore the exclude files we gave it.
To fix this the proper way is to mark all packages we only use locally
with !remote tags. This is a bit ugly but more correct. I also moved the
DecodeChanges() code around as it is called from the client so the
handles package which should only be remote doesn't really fit anyway.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Users do not realize that the entire context directory is being copied
into the podman machine when doing a podman --remote build.
Adding information about the context directory might help them
understand this.
Improves: https://github.com/containers/podman/issues/23287
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
The current code did something like this:
lock()
getState()
unlock()
if state != running
lock()
getState() == running -> error
unlock()
This of course is wrong because between the first unlock() and second
lock() call another process could have modified the state. This meant
that sometimes you would get a weird error on start because the internal
setup errored as the container was already running.
In general any state check without holding the lock is incorrect and
will result in race conditions. As such refactor the code to combine
both StartAndAttach and Attach() into one function that can handle both.
With that we can move the running check into the locked code.
Also use typed error for this specific error case then the callers can
check and ignore the specific error when needed. This also allows us to
fix races in the compat API that did a similar racy state check.
This commit changes slightly how we output the result, previously a
start on already running container would never print the id/name of the
container which is confusing and sort of breaks idempotence. Now it will
include the output except when --all is used. Then it only reports the
ids that were actually started.
Fixes#23246
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Fixes compile issues with new docker changes, then fix all the new
depreciation warnings.
Also there seem to be larger pre-existing problems with the
/containers/json API output as the HostConfig field seems to be missing
but I don't have time to deal with that currently.
Note this does not include changes for the new docker API 1.46.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The docker API uses only a single arg for platform and multiple
platforms are given as comma separated list.
Fixes#22071
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The function that's handing us events will return an error after closing
the channel over which it's sending events, and its caller (in its own
goroutine) will then send that error over another channel.
The logic that started the goroutine is likely to notice that the events
channel is closed before noticing that the error channel has a result
for it to read, so any error that would have been communicated would be
lost.
When we finish reading events, check if the reader returned an error
before telling our caller that there was no error.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
This is something Docker does, and we did not do until now. Most
difficult/annoying part was the REST API, where I did not really
want to modify the struct being sent, so I made the new restart
policy parameters query parameters instead.
Testing was also a bit annoying, because testing restart policy
always is.
Signed-off-by: Matt Heon <mheon@redhat.com>
The Docker endpoint here is kind of a nightmare - accepts a full
Resources block, including a large number of scary things like
devices. But it only documents (and seems to use) a small subset
of those. This implements support for that subset. We can always
extend things to implement more later if we have a need.
Signed-off-by: Matt Heon <mheon@redhat.com>
when listing images through the restful service, consumers want to know
if the image they are listing is a manifest or not because the libpod
endpoint returns both images and manifest lists.
in addition, we now add `arch` and `os` as fields in the libpod endpoint
for image listing as well.
Fixes: #22184Fixes: #22185
Signed-off-by: Brent Baude <bbaude@redhat.com>
Not sure why this only triggers now but this code was broken for a
while. It is racy as reported on the issue but because it changes the
actual map part of the network backend it means it can also alter the
behavior of the network which is very bad.
Fixes#22330
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Fix following issues:
- create container API handler ignores Annotations from HostConfig
- inspect container API handler does not provide Annotations as
part of HostConfig
Signed-off-by: diplane <diplane3d@gmail.com>
I am really not sure why the caller even should have the option to set
this. We should always use the correct isolation type based on the
privileges the server runs under never the client. podman-remote build
seems to send the default based on its local privs which was wrong as
well. To fix this I also changed the client to send the default if the
isolation flag is not set.
Fixes#22109
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Like docker podman network inspect should output the information of
running container with their ip/mac address on this network.
However the output format is not docker compatible as this cannot
include all the info we have and the previous output was already not
compatible so this is not new.
New example output:
```
[
{
...
"containers": {
"7c0d295779cee4a6db7adc07a99e635909413a390eeab9f951edbc4aac406bf1": {
"name": "c2",
"interfaces": {
"eth0": {
"subnets": [
{
"ipnet": "10.89.0.4/24",
"gateway": "10.89.0.1"
},
{
"ipnet": "fda3:b4da:da1e:7e9d::4/64",
"gateway": "fda3:b4da:da1e:7e9d::1"
}
],
"mac_address": "1a:bd:ca:ea:4b:3a"
}
}
},
"b17c6651ae6d9cc7d5825968e01d6b1e67f44460bb0c140bcc32bd9d436ac11d": {
"name": "c1",
"interfaces": {
"eth0": {
"subnets": [
{
"ipnet": "10.89.0.3/24",
"gateway": "10.89.0.1"
},
{
"ipnet": "fda3:b4da:da1e:7e9d::3/64",
"gateway": "fda3:b4da:da1e:7e9d::1"
}
],
"mac_address": "f6:50:e6:22:d9:55"
}
}
}
}
}
]
```
Fixes#14126
Fixes https://issues.redhat.com/browse/RHEL-3153
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Moving from Go module v4 to v5 prepares us for public releases.
Move done using gomove [1] as with the v3 and v4 moves.
[1] https://github.com/KSubedi/gomove
Signed-off-by: Matt Heon <mheon@redhat.com>
This mirrors how the Docker API handles things, allowing us to be
more compatible with Docker and more verbose on the Libpod API.
Stats are given as per network interface in the container, but
still aggregated for `podman stats` and `podman pod stats`
display (so the CLI does not change, only the Libpod and Compat
APIs).
Signed-off-by: Matt Heon <mheon@redhat.com>
The pasta network mode has been added in podman v4.4 and this causes a
conflict with named networks that could also be called "pasta". To not
break anything we had special logic to prefer the named network over the
network mode. Now with 5.0 we can break this and remove this awkward
special handling from the code.
Containers created with 4.X that use a named network pasta will also
continue to work fine, this chnage will only effect the creation of new
containers with a named network pasta and instead always used the
network mode pasta. We now also block the creation of networks with the
name "pasta".
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When inspecting a container that does not define any health check, the health field should return nil. This matches docker behavior.
Signed-off-by: Ashley Cui <acui@redhat.com>
SpecGen is our primary container creation abstraction, and is
used to connect our CLI to the Libpod container creation backend.
Because container creation has a million options (I exaggerate
only slightly), the struct is composed of several other structs,
many of which are quite large.
The core problem is that SpecGen is also an API type - it's used
in remote Podman. There, we have a client and a server, and we
want to respect the server's containers.conf. But how do we tell
what parts of SpecGen were set by the client explicitly, and what
parts were not? If we're not using nullable values, an explicit
empty string and a value never being set are identical - and we
can't tell if it's safe to grab a default from the server's
containers.conf.
Fortunately, we only really need to do this for booleans. An
empty string is sufficient to tell us that a string was unset
(even if the user explicitly gave us an empty string for an
option, filling in a default from the config file is acceptable).
This makes things a lot simpler. My initial attempt at this
changed everything, including strings, and it was far larger and
more painful.
Also, begin the first steps of removing all uses of
containers.conf defaults from client-side. Two are gone entirely,
the rest are marked as remove-when-possible.
[NO NEW TESTS NEEDED] This is just a refactor.
Signed-off-by: Matt Heon <mheon@redhat.com>
* Add BaseHostsFile to container configuration
* Do not copy /etc/hosts file from host when creating a container using Docker API
Signed-off-by: Gavin Lam <gavin.oss@tutamail.com>
When committing containers to create new images, accept a container
config blob being passed in the body of the API request by adding a
Config field to our API structures. Populate it from the body of
requests that we receive, and use its contents as the body of requests
that we make.
Make the libpod commit endpoint split changes values at newlines, just
like the compat endpoint does.
Pass both the config blob and the "changes" slice to buildah's Commit()
API, so that it can handle cases where they overlap or conflict.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>