There is no reason to disallow exposed sctp ports at all. As root we can
publish them find and as rootless it should error later anyway.
And for the case mentioned in the issue it doesn't make sense as the
port is not even published thus it is just part of the metadata which is
totally in all cases.
Fixes#23911
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Fix one minor issue with vfkit error handling. First checking if err !=
nil OR errors.Is() is pointless as the err != is already true.
Second nilerr complains because we return nil when we hit an error
branch, in this case this is correct because an error means VM is
stopped.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
It qemu cannot be compiled anyway so make sure we do not try to compile
parts where the typechecker complains about on windows.
Also all the e2e test files are only used on linux as well.
pkg/machine/wsl also reports some error but to many for me to fix them
now. One minor problem was fixed in pkg/machine/machine_windows.go.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
Now that we have propert !remote tags set everywhere we can just rely on
that and do not need to skip any dirs.
Also on linux do not lint three times, one remote run is enough.
We still have to skip the test dir for windows/macos though or we need
to add linux build tags there everywhere as well. This seems simpler.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@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>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
Fixes new spotted issues around printf() formats and using os.Setenv()
in tests.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
Fix up a couple of versions in comments in the
pkg/api/server/register_images.go file. Based on comments
from #23440
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
The value of the pointer might be changed while creating the container
causing unexpected side effects.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Like commit 55749af0c7 but for podman *pod* stats not the normal podman
stats. We must ignore ErrCtrStopped here as well as this will happen
when the container process exited.
While at it remove a useless argument from the function as it was always
nil and restructure the logic flow to make it easier to read.
Fixes#23334
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
stats read from the cgroup, and in order to know the cgroup we check the
pid for the cgroup. However there is a window where the pid exited and
podman did not yet updated its internal state. In this case the code
returns ErrCtrStopped so we should ignore this error as well.
Fixes#23334
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
If a pod is removed when calling podman pod stats there is a race where
the command might fail with no such pod. This is not a user error, like
the ps/ls command skip it and move to the next one.
Fixes#23327
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The pod name was queried without holding the container lock, thus it was
possible that the pod was deleted in the meantime and podman just failed
with "no such pod" as the errors.Is() check matched the wrong error.
Move it into the locked code this should prevent anyone from removing
the pod while the container is part of it. Also fix the returned error,
there is no reason to special case one specific error just wrap any error
here so callers at least know where it happened. However this is not
good enough because the batch doesn't update the state which means it
see everything before the container was locked. In this case it might be
possible the ctr and pod was already removed so let the caller skip both
ctr and pod removed errors.
Fixes#23282
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When the file is empty it is possible our code panics as bar.ProxyReader
returns nil when the bar is finished which is the case for 0 size as it
doesn't have to read anything from there. However as this happens on
different goroutines it is race and most of the time still works.
To fix this simply skip the progress bar setup for empty files.
While at it fix the deprecated argument in the tests.
Fixes#23281
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When a container or volume is removed during the loop this is not a
problem and we should just skip it as it is not a user bug and just a
normal race.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
If a container is removed during the loop we should just ignore it. No
reason to log this as error to the user.
Fixes#23279
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Ensure that the map of builders we create links the
builder name and not the farm name to the image engine
of that node. This was a regression introduced during the
farm conf rework.
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Adds a `NetworkAlias=` key to both .container and .pod quadlet files,
which translates to the `--network-alias` option to `podman run` and
`podman pod create` respectively. Can be repeated multiple times.
Signed-off-by: Félix Saparelli <felix@passcod.name>
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>
The new docker types have conflicting swagger:model names, there is both
network.CreateResponse and container.CreateResponse. However both have
a different fields (Warning and Warnings) and both are marked as
required. The swagger generate sees both and somehow merges them but
then only shows fields from one type but at the same time list all
fields as required. This causes the swagger validation to fail:
- "Warning" is present in required but not defined as property in definition "CreateResponse"
To work around that we exlcude the netwok types from the swagger
generation which makes it work again. Looking at the final type info in
the browser it still shows the type onfo on the compat network endpints
so it doesn't even loose any valuable information AFAICS.
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>
This commit gets tests working under the new local-registry system:
* amend a few image names, mostly just sticking to a consistent
list of those images in our registry cache. Mostly minor
tag updates.
* trickier: pull_test: change some error messages, and remove
a test that's now a NOP. Basically, with a local (unprotected)
registry we always get "404 manifest unknown"; with a real
registry we'll get "403 I can't tell you".
* trickiest: seccomp_test: build our own images at run time,
with our desired labels. Until now we've been pulling
prebuilt images, but those will not copy to the local
cache registry. Something about v1? Anyhow, I gave up
trying to cache them, and the workaround is straightforward.
Also took the liberty of strengthening a few error-message checks
Signed-off-by: Ed Santiago <santiago@redhat.com>
We should never try to reexxec when we are already root with
CAP_SYS_ADMIN. The code contained a bug when --cgroups=disabled is used
as it tried to perfom a reexec even when it was not needed.
Fixes: 900e29549a ("libpod: do not move podman with --cgroups=disabled")
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The code currently tried to avoid joining the userns from conmon
directly and rather joined to only read the pid file and then send this
back to use so we could join the userns. From the comment this was done
because we could not read the pid file. However this is no longer true
as of commit 49eb5af301 and file is no always owned by the real user.
This means we can just remove this special logic and join the namespace
directly there. A test has been added to check the rejoin logic with a
custom uidmapping.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
In case of timeouts actually log the command again and make sure to send
SIGABRT to the process as go will create a useful stack strace where we
can see where things are hanging. It also kill the process unlike the
default Eventually().Should(Exit()) call the leaves the process around.
The output will be captured by default in the log so we just see the
stack trace there.
And while at it bump the timout up to 10 mins, we are hitting hard
flakes in CI where machine init takes longer than 5 mins for unknown
reasons but this seems to be good enough.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Currently all podman machine rm errors in AfterEach were ignored.
This means some leaked and caused issues later on, see #22844.
To fix it first rework the logic to only remove machines when needed at
the place were they are created using DeferCleanup(), however
DeferCleanup() does not work well together with AfterEach() as it always
run AfterEach() before DeferCleanup(). As AfterEach() deletes the dir
the podman machine rm call can not be done afterwards.
As such migrate all cleanup to use DeferCleanup() and while I have to
touch this fix the code to remove the per file duplciation and define
the setup/cleanup once in the global scope.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
On linux and macos the connections are stored under the home dir by
default so it is not a problem there but on windows we first check
the APPDATA env and use this dir as config storage. This has the problem
that it is not cleaned up after each test as such connections might leak
into the following test causing failues there.
Fixes#22844
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
