diff --git a/cmd/podman-mac-helper/main.go b/cmd/podman-mac-helper/main.go index ef57341bcd..d3e5310929 100644 --- a/cmd/podman-mac-helper/main.go +++ b/cmd/podman-mac-helper/main.go @@ -53,6 +53,7 @@ func main() { if err := rootCmd.Execute(); err != nil { fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error()) + os.Exit(1) } } diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index f603c51dcf..3a0e46be7c 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2012,7 +2012,11 @@ func (c *Container) generateResolvConf() error { // If the user provided dns, it trumps all; then dns masq; then resolv.conf keepHostServers := false if len(nameservers) == 0 { - keepHostServers = true + // when no network name servers or not netavark use host servers + // for aardvark dns we only want our single server in there + if len(networkNameServers) == 0 || networkBackend != string(types.Netavark) { + keepHostServers = true + } // first add the nameservers from the networks status nameservers = networkNameServers // slirp4netns has a built in DNS forwarder. diff --git a/libpod/container_log.go b/libpod/container_log.go index e33ede5dd1..3ae0c70e84 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -10,6 +10,7 @@ import ( "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/libpod/logs" + systemdDefine "github.com/containers/podman/v4/pkg/systemd/define" "github.com/nxadm/tail" "github.com/nxadm/tail/watch" "github.com/sirupsen/logrus" @@ -36,11 +37,15 @@ func (r *Runtime) Log(ctx context.Context, containers []*Container, options *log func (c *Container) ReadLog(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine, colorID int64) error { switch c.LogDriver() { case define.PassthroughLogging: + // if running under systemd fallback to a more native journald reading + if unitName, ok := c.config.Labels[systemdDefine.EnvVariable]; ok { + return c.readFromJournal(ctx, options, logChannel, colorID, unitName) + } return fmt.Errorf("this container is using the 'passthrough' log driver, cannot read logs: %w", define.ErrNoLogs) case define.NoLogging: return fmt.Errorf("this container is using the 'none' log driver, cannot read logs: %w", define.ErrNoLogs) case define.JournaldLogging: - return c.readFromJournal(ctx, options, logChannel, colorID) + return c.readFromJournal(ctx, options, logChannel, colorID, "") case define.JSONLogging: // TODO provide a separate implementation of this when Conmon // has support. diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index de5a66dee1..a708ad46da 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -15,7 +15,6 @@ import ( "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/libpod/logs" "github.com/containers/podman/v4/pkg/rootless" - "github.com/coreos/go-systemd/v22/journal" "github.com/coreos/go-systemd/v22/sdjournal" "github.com/sirupsen/logrus" ) @@ -32,22 +31,8 @@ func init() { logDrivers = append(logDrivers, define.JournaldLogging) } -// initializeJournal will write an empty string to the journal -// when a journal is created. This solves a problem when people -// attempt to read logs from a container that has never had stdout/stderr -func (c *Container) initializeJournal(ctx context.Context) error { - m := make(map[string]string) - m["SYSLOG_IDENTIFIER"] = "podman" - m["PODMAN_ID"] = c.ID() - history := events.History - m["PODMAN_EVENT"] = history.String() - container := events.Container - m["PODMAN_TYPE"] = container.String() - m["PODMAN_TIME"] = time.Now().Format(time.RFC3339Nano) - return journal.Send("", journal.PriInfo, m) -} - -func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine, colorID int64) error { +func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOptions, + logChannel chan *logs.LogLine, colorID int64, passthroughUnit string) error { // We need the container's events in the same journal to guarantee // consistency, see #10323. if options.Follow && c.runtime.config.Engine.EventsLogger != "journald" { @@ -83,10 +68,35 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption if err := journal.AddDisjunction(); err != nil { return fmt.Errorf("adding filter disjunction to journald logger: %w", err) } - match = sdjournal.Match{Field: "CONTAINER_ID_FULL", Value: c.ID()} - if err := journal.AddMatch(match.String()); err != nil { - return fmt.Errorf("adding filter to journald logger: %v: %w", match, err) + + if passthroughUnit != "" { + // Match based on systemd unit which is the container is cgroup + // so we get the exact logs for a single container even in the + // play kube case where a single unit starts more than one container. + unitTypeName := "_SYSTEMD_UNIT" + if rootless.IsRootless() { + unitTypeName = "_SYSTEMD_USER_UNIT" + } + // By default we will have our own systemd cgroup with the name libpod-.scope. + value := "libpod-" + c.ID() + ".scope" + if c.config.CgroupsMode == cgroupSplit { + // If cgroup split the container runs in the unit cgroup so we use this for logs, + // the good thing is we filter the podman events already out below. + // Thus we are left with the real container log and possibly podman output (e.g. logrus). + value = passthroughUnit + } + + match = sdjournal.Match{Field: unitTypeName, Value: value} + if err := journal.AddMatch(match.String()); err != nil { + return fmt.Errorf("adding filter to journald logger: %v: %w", match, err) + } + } else { + match = sdjournal.Match{Field: "CONTAINER_ID_FULL", Value: c.ID()} + if err := journal.AddMatch(match.String()); err != nil { + return fmt.Errorf("adding filter to journald logger: %v: %w", match, err) + } } + if err := journal.AddMatch(uidMatch.String()); err != nil { return fmt.Errorf("adding filter to journald logger: %v: %w", uidMatch, err) } @@ -178,26 +188,17 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption continue } - var message string - var formatError error - - if options.Multi { - message, formatError = journalFormatterWithID(entry) - } else { - message, formatError = journalFormatter(entry) - } - - if formatError != nil { - logrus.Errorf("Failed to parse journald log entry: %v", formatError) - return - } - - logLine, err := logs.NewJournaldLogLine(message, options.Multi) - logLine.ColorID = colorID + logLine, err := journalToLogLine(entry) if err != nil { - logrus.Errorf("Failed parse log line: %v", err) + logrus.Errorf("Failed parse journal entry: %v", err) return } + id := c.ID() + if len(id) > 12 { + id = id[:12] + } + logLine.CID = id + logLine.ColorID = colorID if options.UseName { logLine.CName = c.Name() } @@ -212,76 +213,37 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption return nil } -func journalFormatterWithID(entry *sdjournal.JournalEntry) (string, error) { - output, err := formatterPrefix(entry) - if err != nil { - return "", err - } - - id, ok := entry.Fields["CONTAINER_ID_FULL"] - if !ok { - return "", errors.New("no CONTAINER_ID_FULL field present in journal entry") - } - if len(id) > 12 { - id = id[:12] - } - output += fmt.Sprintf("%s ", id) - // Append message - msg, err := formatterMessage(entry) - if err != nil { - return "", err - } - output += msg - return output, nil -} - -func journalFormatter(entry *sdjournal.JournalEntry) (string, error) { - output, err := formatterPrefix(entry) - if err != nil { - return "", err - } - // Append message - msg, err := formatterMessage(entry) - if err != nil { - return "", err - } - output += msg - return output, nil -} +func journalToLogLine(entry *sdjournal.JournalEntry) (*logs.LogLine, error) { + line := &logs.LogLine{} -func formatterPrefix(entry *sdjournal.JournalEntry) (string, error) { usec := entry.RealtimeTimestamp - tsString := time.Unix(0, int64(usec)*int64(time.Microsecond)).Format(logs.LogTimeFormat) - output := fmt.Sprintf("%s ", tsString) + line.Time = time.Unix(0, int64(usec)*int64(time.Microsecond)) + priority, ok := entry.Fields["PRIORITY"] if !ok { - return "", errors.New("no PRIORITY field present in journal entry") + return nil, errors.New("no PRIORITY field present in journal entry") } switch priority { case journaldLogOut: - output += "stdout " + line.Device = "stdout" case journaldLogErr: - output += "stderr " + line.Device = "stderr" default: - return "", errors.New("unexpected PRIORITY field in journal entry") + return nil, errors.New("unexpected PRIORITY field in journal entry") } // if CONTAINER_PARTIAL_MESSAGE is defined, the log type is "P" if _, ok := entry.Fields["CONTAINER_PARTIAL_MESSAGE"]; ok { - output += fmt.Sprintf("%s ", logs.PartialLogType) + line.ParseLogType = logs.PartialLogType } else { - output += fmt.Sprintf("%s ", logs.FullLogType) + line.ParseLogType = logs.FullLogType } - return output, nil -} - -func formatterMessage(entry *sdjournal.JournalEntry) (string, error) { - // Finally, append the message - msg, ok := entry.Fields["MESSAGE"] + line.Msg, ok = entry.Fields["MESSAGE"] if !ok { - return "", errors.New("no MESSAGE field present in journal entry") + return nil, errors.New("no MESSAGE field present in journal entry") } - msg = strings.TrimSuffix(msg, "\n") - return msg, nil + line.Msg = strings.TrimSuffix(line.Msg, "\n") + + return line, nil } diff --git a/libpod/container_log_unsupported.go b/libpod/container_log_unsupported.go index bb74a810d5..d524561f42 100644 --- a/libpod/container_log_unsupported.go +++ b/libpod/container_log_unsupported.go @@ -11,10 +11,6 @@ import ( "github.com/containers/podman/v4/libpod/logs" ) -func (c *Container) readFromJournal(_ context.Context, _ *logs.LogOptions, _ chan *logs.LogLine, colorID int64) error { - return fmt.Errorf("journald logging only enabled with systemd on linux: %w", define.ErrOSNotSupported) -} - -func (c *Container) initializeJournal(ctx context.Context) error { +func (c *Container) readFromJournal(_ context.Context, _ *logs.LogOptions, _ chan *logs.LogLine, _ int64, _ string) error { return fmt.Errorf("journald logging only enabled with systemd on linux: %w", define.ErrOSNotSupported) } diff --git a/libpod/logs/log.go b/libpod/logs/log.go index 43da8d904d..664e467067 100644 --- a/libpod/logs/log.go +++ b/libpod/logs/log.go @@ -243,36 +243,6 @@ func NewLogLine(line string) (*LogLine, error) { return &l, nil } -// NewJournaldLogLine creates a LogLine from the specified line from journald. -// Note that if withID is set, the first item of the message is considerred to -// be the container ID and set as such. -func NewJournaldLogLine(line string, withID bool) (*LogLine, error) { - splitLine := strings.Split(line, " ") - if len(splitLine) < 4 { - return nil, fmt.Errorf("'%s' is not a valid container log line", line) - } - logTime, err := time.Parse(LogTimeFormat, splitLine[0]) - if err != nil { - return nil, fmt.Errorf("unable to convert time %s from container log: %w", splitLine[0], err) - } - var msg, id string - if withID { - id = splitLine[3] - msg = strings.Join(splitLine[4:], " ") - } else { - msg = strings.Join(splitLine[3:], " ") - // NO ID - } - l := LogLine{ - Time: logTime, - Device: splitLine[1], - ParseLogType: splitLine[2], - Msg: msg, - CID: id, - } - return &l, nil -} - // Partial returns a bool if the log line is a partial log type func (l *LogLine) Partial() bool { return l.ParseLogType == PartialLogType diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 33e31c63e3..b9e9a27b9e 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -541,12 +541,8 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai } switch ctr.config.LogDriver { - case define.NoLogging, define.PassthroughLogging: + case define.NoLogging, define.PassthroughLogging, define.JournaldLogging: break - case define.JournaldLogging: - if err := ctr.initializeJournal(ctx); err != nil { - return nil, fmt.Errorf("failed to initialize journal: %w", err) - } default: if ctr.config.LogPath == "" { ctr.config.LogPath = filepath.Join(ctr.config.StaticDir, "ctr.log") diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 704af4b0e4..bb9cda8875 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -277,10 +277,17 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { // FIXME can we use the IPAM driver and options? } + opts := nettypes.NetworkCreateOptions{ + IgnoreIfExists: !networkCreate.CheckDuplicate, + } ic := abi.ContainerEngine{Libpod: runtime} - newNetwork, err := ic.NetworkCreate(r.Context(), network, nil) + newNetwork, err := ic.NetworkCreate(r.Context(), network, &opts) if err != nil { - utils.InternalServerError(w, err) + if errors.Is(err, nettypes.ErrNetworkExists) { + utils.Error(w, http.StatusConflict, err) + } else { + utils.InternalServerError(w, err) + } return } diff --git a/pkg/api/server/handler_logging.go b/pkg/api/server/handler_logging.go index 38ee8321c1..2f62a28fd3 100644 --- a/pkg/api/server/handler_logging.go +++ b/pkg/api/server/handler_logging.go @@ -1,7 +1,10 @@ package server import ( + "bufio" + "errors" "io" + "net" "net/http" "time" @@ -33,6 +36,28 @@ func (l responseWriter) Write(b []byte) (int, error) { return l.ResponseWriter.Write(b) } +func (l responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + if wrapped, ok := l.ResponseWriter.(http.Hijacker); ok { + return wrapped.Hijack() + } + + return nil, nil, errors.New("ResponseWriter does not support hijacking") +} + +func (l responseWriter) Header() http.Header { + return l.ResponseWriter.Header() +} + +func (l responseWriter) WriteHeader(statusCode int) { + l.ResponseWriter.WriteHeader(statusCode) +} + +func (l responseWriter) Flush() { + if wrapped, ok := l.ResponseWriter.(http.Flusher); ok { + wrapped.Flush() + } +} + func loggingHandler() mux.MiddlewareFunc { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/test/apiv2/35-networks.at b/test/apiv2/35-networks.at index 07ba45efb2..dfa5b583b7 100644 --- a/test/apiv2/35-networks.at +++ b/test/apiv2/35-networks.at @@ -104,6 +104,9 @@ t GET networks/podman 200 \ # network create docker t POST networks/create Name=net3\ IPAM='{"Config":[]}' 201 +# create with same name should not error unless CheckDuplicate is set +t POST networks/create Name=net3 201 +t POST networks/create Name=net3\ CheckDuplicate=true 409 # network delete docker t DELETE networks/net3 204 diff --git a/test/system/035-logs.bats b/test/system/035-logs.bats index 8f378054af..e0ee3ed14d 100644 --- a/test/system/035-logs.bats +++ b/test/system/035-logs.bats @@ -319,14 +319,14 @@ function _log_test_follow_since() { # Now do the same with a running container to check #16950. run_podman ${events_backend} run --log-driver=$driver --name $cname -d $IMAGE \ - sh -c "sleep 0.5; while :; do echo $content && sleep 3; done" + sh -c "sleep 1; while :; do echo $content && sleep 5; done" # sleep is required to make sure the podman event backend no longer sees the start event in the log # This value must be greater or equal than the the value given in --since below sleep 0.2 # Make sure podman logs actually follows by giving a low timeout and check that the command times out - PODMAN_TIMEOUT=2 run_podman 124 ${events_backend} logs --since 0.1s -f $cname + PODMAN_TIMEOUT=3 run_podman 124 ${events_backend} logs --since 0.1s -f $cname assert "$output" =~ "^$content timeout: sending signal TERM to command.*" "logs --since -f on running container works" diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index d928596890..837d3a6192 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -383,11 +383,15 @@ metadata: spec: containers: - command: - - top + - sh + - -c + - echo a stdout; echo a stderr 1>&2; sleep inf image: $IMAGE name: a - command: - - top + - sh + - -c + - echo b stdout; echo b stderr 1>&2; sleep inf image: $IMAGE name: b EOF @@ -418,8 +422,18 @@ EOF for name in "a" "b"; do run_podman container inspect test_pod-${name} --format "{{.HostConfig.LogConfig.Type}}" assert $output != "passthrough" + # check that we can get the logs with passthrough when we run in a systemd unit + run_podman logs test_pod-$name + assert "$output" == "$name stdout +$name stderr" "logs work with passthrough" done + # we cannot assume the ordering between a b, this depends on timing and would flake in CI + # use --names so we do not have to get the ID + run_podman pod logs --names test_pod + assert "$output" =~ ".*^test_pod-a a stdout.*" "logs from container a shown" + assert "$output" =~ ".*^test_pod-b b stdout.*" "logs from container b shown" + # Add a simple `auto-update --dry-run` test here to avoid too much redundancy # with 255-auto-update.bats run_podman auto-update --dry-run --format "{{.Unit}},{{.Container}},{{.Image}},{{.Updated}},{{.Policy}}" diff --git a/test/system/251-system-service.bats b/test/system/251-system-service.bats index b4bc6ceaad..7a772e2ef4 100644 --- a/test/system/251-system-service.bats +++ b/test/system/251-system-service.bats @@ -69,3 +69,22 @@ function teardown() { systemctl stop $SERVICE_NAME } + +# Regression test for https://github.com/containers/podman/issues/17749 +@test "podman-system-service --log-level=trace should be able to hijack" { + skip_if_remote "podman system service unavailable over remote" + + port=$(random_free_port) + URL=tcp://127.0.0.1:$port + + systemd-run --unit=$SERVICE_NAME $PODMAN --log-level=trace system service $URL --time=0 + wait_for_port 127.0.0.1 $port + + out=o-$(random_string) + cname=c-$(random_string) + run_podman --url $URL run --name $cname $IMAGE echo $out + assert "$output" == "$out" "service is able to hijack and stream output back" + + run_podman --url $URL rm $cname + systemctl stop $SERVICE_NAME +} diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats index 1dc54a1bc4..d85f30057a 100644 --- a/test/system/252-quadlet.bats +++ b/test/system/252-quadlet.bats @@ -117,7 +117,7 @@ function service_cleanup() { cat > $quadlet_file <