The remote client (podman --remote) was incorrectly throwing an error
when --detach-keys="" was specified for attach, run, or start commands.
According to documentation and the v1.7.0 release notes, specifying an
empty string should disable detaching, not cause an error.
Fixes: #27414
Signed-off-by: shiavm006 <shivammittal42006@gmail.com>
Tremendous amount of changes in here, but all should amount to
the same thing: changing Go import paths from v5 to v6.
Also bumped go.mod to github.com/containers/podman/v6 and updated
version to v6.0.0-dev.
Signed-off-by: Matt Heon <mheon@redhat.com>
Problem: While removing cgroupsv1 code, I noticed my neovim Go config
automatically changed fileperms to the new octal format and I didn't
want that polluting my diffs.
Decision: I thought it best to switch to the new octal format in a dedicated PR.
Action:
- Cursor switched to new octal format for all fileperm ocurrences in Go
source and test files.
- vendor/, docs/ and non-Go files were ignored.
- Reviewed manually.
Ref: https://go.dev/ref/spec#Go_1.13
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
* Added flags to point to TLS PEM files to use for exposing and connecting
to an encrypted remote API socket with server and client authentication.
* Added TLS fields for system connection ls templates.
* Added special "tls" format for system connection ls to list TLS fields
in human-readable table format.
* Updated remote integration and system tests to allow specifying a
"transport" to run the full suite against a unix, tcp, tls, or mtls
system service.
* Added system tests to verify basic operation of unix, tcp, tls, and mtls
services, clients, and connections.
Signed-off-by: Andrew Melnick <meln5674.5674@gmail.com>
For exec and attach use an upgrade request which the server responds
with HTTP 101 status. Since go 1.12 the Body can be casted to an
io.Writer and then use that to write to the server.
This does however not allow us to skip the ugly hack of overwriting
the default dialContext() because the ReadWriterCloser on the body
does not allow us to call CloseWrite() which is critical to correctly
close the stdin side. So we still have to extract the underlying
net.Conn for that.
Using the cast response.Body is important because the underlying http
lib reads from the socket to parse the header and response code of
course and it is possible that it read more content than needed that is
kept in its buffer but because we then only directly read from the
connection it was possible we miss the first few bytes of the message.
This should fix the issue and hopefully also some long standing
podman-remote missing output flakes in CI.
Fixes: #26951
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Do not ignore ErrUnexpectedEOF from DemuxHeader(), if we fail to parse
the header there must have been a clear protocal error between client
and server which should be reported and not silently ignored. I wonder
ig this might explain why we have missing remote exec/attach output
without any error, it is possible we are eating some internal errors due
this.
Commit ba8eba83ef added the ErrUnexpectedEOF check but without any
explanation why that would be needed. The tests from that commit pass
without it locally but not in CI. With some debugging best I found the
issue is actually a test bug. The channel is not consumed until it is
closed which means the main test exists before the log reading goroutine
is done. And if the main test exists the first step it does is to kill
the podman service which then can trigger the ErrUnexpectedEOF server on
the still open http connection and thus the test case failed there.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
DemuxFrame() already returns a byte slice with the correct length so
this makes it simpler and the caller does not need to check this at all.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
io.ReadFull() already returns ErrUnexpectedEOF if there was a short read
so this check is redundant and can be dropped.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Deadcode should that the ShouldRestart() API endpoint was never wired
into the router so the endpoint did not existed and the bindings called
a non existing endpoint which returnd 404 which the binding code
assumed means no restart.
As such remove all this code as it didn't do anything useful. And IMO
exposing a shouldrestart API always feeled wrong to me. The client
should not have to deal with this.
This commit does not change the behavior but it also does not make an
attempt to fix the broken restart handling with the rmeote client. Given
we do not seem to have any user reports about this it seems it is not
used.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Reported by staticcheck linter:
> pkg/bindings/containers/term_windows.go:51:5: SA4011: ineffective break statement. Did you mean to break out of the outer loop? (staticcheck)
> break
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Right now, if you call Update with only part of the options struct added, it panics. This fixes that by only adding them if they are not nil.
Signed-off-by: Astrid Gealer <astrid@gealer.email>
The --env is used to add new environment variable to container or
override the existing one. The --unsetenv is used to remove
the environment variable.
It is done by sharing "env" and "unsetenv" flags between both
"update" and "create" commands and later handling these flags
in the "update" command handler.
The list of environment variables to add/remove is stored
in newly added variables in the ContainerUpdateOptions.
The Container.Update API call is refactored to take
the ContainerUpdateOptions as an input to limit the number of its
arguments.
The Env and UnsetEnv lists are later handled using the envLib
package and the Container is updated.
The remote API is also extended to handle Env and EnvUnset.
Fixes: #24875
Signed-off-by: Jan Kaluza <jkaluza@redhat.com>
A lot of types are moved and now deprecated which causes lint issues.
IDResponse is copied into podman because that has no new 1 to 1
replacement. For some fields that we set as part of the docker API I
added the nolint directive as these fields might be used by API
consumers.
For the other types it is mostly a 1 to 1 move.
ParseUintList is deprecated but we can use the same function from
github.com/containers/storage/pkg/parsers instead.
Note that it containers breaking changes to pkg/bindings which we should
not do generally but given the prevoius commit already has a unavoidable
breaking change we might as well fix the IDResponse issue once now.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
New flags in a `podman update` can change the configuration of HealthCheck when the container is started, without having to restart or recreate the container.
This can help determine why a given container suddenly started failing HealthCheck without interfering with the services it provides. For example, reconfigure HealthCheck to keep logs longer than the usual last X results, store logs to other destinations, etc.
Fixes: https://issues.redhat.com/browse/RHEL-60561
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
When the client gets a 404 back we know the container does not exists,
if ignore is set as well we should just ignore the error client side.
seen in #23554
Signed-off-by: Paul Holzinger <pholzing@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>
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>
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>
When a new API call is added to the bindings we should guard it based on
the version and throw a useful error. Right now an old server that does
not implement a given endpoint would throw a "NOT FOUND" error which is
not good for callers.
Instead implement a custom error type to give a usefule error instead.
This allows bindings users to call errors.As() to know if they call and
to old version.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Commit f48a706abc added a new API endpoint to remove exec session
correctly. And the bindings try to call that endpoint for exec every
time. Now since client and server must not be the same version this
causes a problem if a new 4.8 client calls an older 4.7 server as it has
no idea about such endpoint and throws an ugly error. This is a common
scenario for podman machine setups.
The client does know the server version so it should make sure to not
call such endpoint if the server is older than 4.8.
I added a exec test to the machine tests as this can be reproduced with
podman machine as at the moment at least the VM image does not contain
podman 4.8. And it should at least make sure podman exec keeps working
for podman machine without regressions.
Fixes#20821
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
commit fa19e1baa2 partially introduced
the fix, but was merged too quickly and didn't work with remote.
Introduce a new binding to allow removing a session from the remote
client.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
In `pkg/bindings/containers/ResizeExecTTY`, `sessionID` is the actual required
parameter, instead of `nameOrId`.
[NO NEW TESTS NEEDED]
Signed-off-by: Yiliang Yang <yangyiliang@gmail.com>
