Skip to content

Commit

Permalink
minor changes to address review feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Mrudul Harwani <[email protected]>
  • Loading branch information
mharwani committed Jul 19, 2023
1 parent 203e3ab commit 8763da2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 19 deletions.
17 changes: 7 additions & 10 deletions cmd/nerdctl/container_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,36 +148,34 @@ func TestLogsWithRunningContainer(t *testing.T) {
t.Parallel()
base := testutil.NewBase(t)
containerName := testutil.Identifier(t)
defer base.Cmd("rm", containerName).Run()
expected := []string{}
for i := 1; i <= 10; i++ {
expected = append(expected, fmt.Sprint(i))
defer base.Cmd("rm", "-f", containerName).Run()
expected := make([]string, 10)
for i := 0; i < 10; i++ {
expected[i] = fmt.Sprint(i + 1)
}

base.Cmd("run", "-d", "--name", containerName, testutil.CommonImage,
"sh", "-euc", "for i in `seq 1 10`; do echo $i; sleep 1; done").AssertOK()
base.Cmd("logs", "-f", containerName).AssertOutContainsAll(expected...)
base.Cmd("rm", "-f", containerName).AssertOK()
}

func TestLogsWithoutNewline(t *testing.T) {
func TestLogsWithoutNewlineOrEOF(t *testing.T) {
t.Parallel()
base := testutil.NewBase(t)
containerName := testutil.Identifier(t)
defer base.Cmd("rm", containerName).Run()
defer base.Cmd("rm", "-f", containerName).Run()
expected := []string{"Hello World!", "There is no newline"}
base.Cmd("run", "-d", "--name", containerName, testutil.CommonImage,
"printf", "'Hello World!\nThere is no newline'").AssertOK()
time.Sleep(3 * time.Second)
base.Cmd("logs", "-f", containerName).AssertOutContainsAll(expected...)
base.Cmd("rm", "-f", containerName).AssertOK()
}

func TestLogsAfterRestartingContainer(t *testing.T) {
t.Parallel()
base := testutil.NewBase(t)
containerName := testutil.Identifier(t)
defer base.Cmd("rm", containerName).Run()
defer base.Cmd("rm", "-f", containerName).Run()
base.Cmd("run", "-d", "--name", containerName, testutil.CommonImage,
"printf", "'Hello World!\nThere is no newline'").AssertOK()
expected := []string{"Hello World!", "There is no newline"}
Expand All @@ -187,5 +185,4 @@ func TestLogsAfterRestartingContainer(t *testing.T) {
base.Cmd("start", containerName)
time.Sleep(3 * time.Second)
base.Cmd("logs", "-f", containerName).AssertOutContainsAll(expected...)
base.Cmd("rm", "-f", containerName).AssertOK()
}
10 changes: 5 additions & 5 deletions pkg/cmd/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
// 1, nerdctl run --name demo -it imagename
// 2, ctrl + c to stop demo container
// 3, nerdctl start/restart demo
logConfig, err := generateLogConfig(dataStore, id, options.LogDriver, options.LogOpt, &options.GOptions)
logConfig, err := generateLogConfig(dataStore, id, options.LogDriver, options.LogOpt, options.GOptions.Namespace, options.GOptions.Address)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -656,9 +656,9 @@ func writeCIDFile(path, id string) error {
}

// generateLogConfig creates a LogConfig for the current container store
func generateLogConfig(dataStore string, id string, logDriver string, logOpt []string, gOpt *types.GlobalCommandOptions) (logConfig logging.LogConfig, err error) {
func generateLogConfig(dataStore string, id string, logDriver string, logOpt []string, ns, hostAddress string) (logConfig logging.LogConfig, err error) {
var u *url.URL
logConfig.HostAddress = gOpt.Address
logConfig.HostAddress = hostAddress
if u, err = url.Parse(logDriver); err == nil && u.Scheme != "" {
logConfig.LogURI = logDriver
} else {
Expand All @@ -676,7 +676,7 @@ func generateLogConfig(dataStore string, id string, logDriver string, logOpt []s
if err != nil {
return
}
if err = logDriverInst.Init(dataStore, gOpt.Namespace, id); err != nil {
if err = logDriverInst.Init(dataStore, ns, id); err != nil {
return
}

Expand All @@ -685,7 +685,7 @@ func generateLogConfig(dataStore string, id string, logDriver string, logOpt []s
return
}

logConfigFilePath := logging.LogConfigFilePath(dataStore, gOpt.Namespace, id)
logConfigFilePath := logging.LogConfigFilePath(dataStore, ns, id)
if err = os.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
return
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func loggingProcessAdapter(ctx context.Context, driver Driver, dataStore, hostAd
if err := driver.PreProcess(dataStore, config); err != nil {
return err
}
exit, err := getContainerWait(ctx, hostAddress, config)
exitCh, err := getContainerWait(ctx, hostAddress, config)
if err != nil {
return err
}
Expand All @@ -172,8 +172,8 @@ func loggingProcessAdapter(ctx context.Context, driver Driver, dataStore, hostAd
stdoutR, stdoutW := io.Pipe()
stderrR, stderrW := io.Pipe()
copyStream := func(reader io.Reader, writer *io.PipeWriter) {
// copy using a buffer of size 16K
buf := make([]byte, 16*1024)
// copy using a buffer of size 32K
buf := make([]byte, 32<<10)
_, err := io.CopyBuffer(writer, reader, buf)
if err != nil {
logrus.Errorf("failed to copy stream: %s", err)
Expand Down Expand Up @@ -208,7 +208,7 @@ func loggingProcessAdapter(ctx context.Context, driver Driver, dataStore, hostAd
}()
go func() {
// close stdout and stderr upon container exit
<-exit
<-exitCh
stdoutW.Close()
stderrW.Close()
}()
Expand Down

0 comments on commit 8763da2

Please sign in to comment.