Skip to content

fix: logs updated on starting of a stopped container #3896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/nerdctl/compose/compose_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func startContainers(ctx context.Context, client *containerd.Client, containers
}

// in compose, always disable attach
if err := containerutil.Start(ctx, c, false, client, ""); err != nil {
if err := containerutil.Start(ctx, c, false, false, client, ""); err != nil {
return err
}
info, err := c.Info(ctx, containerd.WithoutRefreshedMetadata)
Expand Down
73 changes: 73 additions & 0 deletions cmd/nerdctl/container/container_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ package container
import (
"errors"
"fmt"
"io"
"os/exec"
"runtime"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -383,3 +385,74 @@ func TestLogsWithDetails(t *testing.T) {

testCase.Run(t)
}

func TestLogsWithStartContainer(t *testing.T) {
testCase := nerdtest.Setup()

// For windows we havent added support for dual logging so not adding the test.
testCase.Require = require.Not(require.Windows)

testCase.SubTests = []*test.Case{
{
Description: "Test logs are directed correctly for container start of a interactive container",
Setup: func(data test.Data, helpers test.Helpers) {
cmd := helpers.Command("run", "-it", "--name", data.Identifier(), testutil.CommonImage)
cmd.WithPseudoTTY()
cmd.WithFeeder(func() io.Reader {
return strings.NewReader("echo foo\nexit\n")
})

cmd.Run(&test.Expected{
ExitCode: 0,
})

},
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("rm", "-f", data.Identifier())
},
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
cmd := helpers.Command("start", "-ia", data.Identifier())
cmd.WithPseudoTTY()
cmd.WithFeeder(func() io.Reader {
return strings.NewReader("echo bar\nexit\n")
})
cmd.Run(&test.Expected{
ExitCode: 0,
})
cmd = helpers.Command("logs", data.Identifier())

return cmd
},
Expected: test.Expects(0, nil, expect.Contains("foo", "bar")),
},
{
Description: "Test logs are captured after stopping and starting a non-interactive container and continue capturing new logs",
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.CommonImage, "sh", "-c", "while true; do echo foo; sleep 1; done")
},
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("rm", "-f", data.Identifier())
},
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
helpers.Ensure("stop", data.Identifier())
initialLogs := helpers.Capture("logs", data.Identifier())
initialFooCount := strings.Count(initialLogs, "foo")
data.Labels().Set("initialFooCount", strconv.Itoa(initialFooCount))
helpers.Ensure("start", data.Identifier())
nerdtest.EnsureContainerStarted(helpers, data.Identifier())
return helpers.Command("logs", data.Identifier())
},
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
ExitCode: 0,
Output: func(stdout string, info string, t *testing.T) {
finalLogsCount := strings.Count(stdout, "foo")
initialFooCount, _ := strconv.Atoi(data.Labels().Get("initialFooCount"))
assert.Assert(t, finalLogsCount > initialFooCount, "Expected 'foo' count to increase after restart", info)
},
}
},
},
}
testCase.Run(t)
}
15 changes: 10 additions & 5 deletions cmd/nerdctl/container/container_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func StartCommand() *cobra.Command {
cmd.Flags().SetInterspersed(false)
cmd.Flags().BoolP("attach", "a", false, "Attach STDOUT/STDERR and forward signals")
cmd.Flags().String("detach-keys", consoleutil.DefaultDetachKeys, "Override the default detach keys")

cmd.Flags().BoolP("interactive", "i", false, "Attach container's STDIN")
return cmd
}

Expand All @@ -60,11 +60,16 @@ func startOptions(cmd *cobra.Command) (types.ContainerStartOptions, error) {
if err != nil {
return types.ContainerStartOptions{}, err
}
interactive, err := cmd.Flags().GetBool("interactive")
if err != nil {
return types.ContainerStartOptions{}, err
}
return types.ContainerStartOptions{
Stdout: cmd.OutOrStdout(),
GOptions: globalOptions,
Attach: attach,
DetachKeys: detachKeys,
Stdout: cmd.OutOrStdout(),
GOptions: globalOptions,
Attach: attach,
DetachKeys: detachKeys,
Interactive: interactive,
}, nil
}

Expand Down
8 changes: 1 addition & 7 deletions cmd/nerdctl/container/container_start_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,7 @@ func TestStartDetachKeys(t *testing.T) {
}

testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
flags := "-a"
// Started container must be interactive - which is apparently the default for nerdctl, which does not support
// the -i flag, while docker requires it explicitly
if nerdtest.IsDocker() {
flags += "i"
}
cmd := helpers.Command("start", flags, "--detach-keys=ctrl-a,ctrl-b", data.Identifier())
cmd := helpers.Command("start", "-ai", "--detach-keys=ctrl-a,ctrl-b", data.Identifier())
cmd.WithPseudoTTY()
cmd.WithFeeder(func() io.Reader {
// ctrl+a and ctrl+b (see https://en.wikipedia.org/wiki/C0_and_C1_control_codes)
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/types/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ type ContainerStartOptions struct {
Attach bool
// The key sequence for detaching a container.
DetachKeys string
// Attach stdin
Interactive bool
}

// ContainerKillOptions specifies options for `nerdctl (container) kill`.
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/container/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func Restart(ctx context.Context, client *containerd.Client, containers []string
if err := containerutil.Stop(ctx, found.Container, options.Timeout, options.Signal); err != nil {
return err
}
if err := containerutil.Start(ctx, found.Container, false, client, ""); err != nil {
if err := containerutil.Start(ctx, found.Container, false, false, client, ""); err != nil {
return err
}
_, err := fmt.Fprintln(options.Stdout, found.Req)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func Start(ctx context.Context, client *containerd.Client, reqs []string, option
if found.MatchCount > 1 {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
}
if err := containerutil.Start(ctx, found.Container, options.Attach, client, options.DetachKeys); err != nil {
if err := containerutil.Start(ctx, found.Container, options.Attach, options.Interactive, client, options.DetachKeys); err != nil {
return err
}
if !options.Attach {
Expand Down
6 changes: 3 additions & 3 deletions pkg/containerutil/containerutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func GenerateSharingPIDOpts(ctx context.Context, targetCon containerd.Container)
}

// Start starts `container` with `attach` flag. If `attach` is true, it will attach to the container's stdio.
func Start(ctx context.Context, container containerd.Container, flagA bool, client *containerd.Client, detachKeys string) (err error) {
func Start(ctx context.Context, container containerd.Container, flagA bool, flagI bool, client *containerd.Client, detachKeys string) (err error) {
// defer the storage of start error in the dedicated label
defer func() {
if err != nil {
Expand Down Expand Up @@ -243,7 +243,7 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie
}
flagT := process.Process.Terminal
var con console.Console
if flagA && flagT {
if (flagI || flagA) && flagT {
con, err = consoleutil.Current()
if err != nil {
return err
Expand Down Expand Up @@ -284,7 +284,7 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie
// source: https://github.com/containerd/nerdctl/blob/main/docs/command-reference.md#whale-nerdctl-start
attachStreamOpt = []string{"STDOUT", "STDERR"}
}
task, err := taskutil.NewTask(ctx, client, container, attachStreamOpt, false, flagT, true, con, logURI, detachKeys, namespace, detachC)
task, err := taskutil.NewTask(ctx, client, container, attachStreamOpt, flagI, flagT, true, con, logURI, detachKeys, namespace, detachC)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/errutil/exit_coder.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

package errutil

import "os"
import (
"os"
)

type ExitCoder interface {
error
Expand Down Expand Up @@ -46,7 +48,6 @@ func HandleExitCoder(err error) {
if err == nil {
return
}

if exitErr, ok := err.(ExitCoder); ok {
os.Exit(exitErr.ExitCode())
}
Expand Down
19 changes: 14 additions & 5 deletions pkg/taskutil/taskutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,24 @@ func NewTask(ctx context.Context, client *containerd.Client, container container
var ioCreator cio.Creator
if len(attachStreamOpt) != 0 {
log.G(ctx).Debug("attaching output instead of using the log-uri")
// when attaching a TTY we use writee for stdio and binary for log persistence
if flagT {
in, err := consoleutil.NewDetachableStdin(con, detachKeys, closer)
if err != nil {
return nil, err
var in io.Reader
if flagI {
// FIXME: check IsTerminal on Windows too
if runtime.GOOS != "windows" && !term.IsTerminal(0) {
return nil, errors.New("the input device is not a TTY")
}
var err error
in, err = consoleutil.NewDetachableStdin(con, detachKeys, closer)
if err != nil {
return nil, err
}
}
ioCreator = cio.NewCreator(cio.WithStreams(in, con, nil), cio.WithTerminal)
ioCreator = cioutil.NewContainerIO(namespace, logURI, true, in, con, nil)
} else {
streams := processAttachStreamsOpt(attachStreamOpt)
ioCreator = cio.NewCreator(cio.WithStreams(streams.stdIn, streams.stdOut, streams.stdErr))
ioCreator = cioutil.NewContainerIO(namespace, logURI, false, streams.stdIn, streams.stdOut, streams.stdErr)
}

} else if flagT && flagD {
Expand Down
Loading