Skip to content

Commit aa98dc6

Browse files
crosbymichaelcrosbymichael
authored
Merge pull request #3754 from estesp/cp-1.3-3743
[release/1.3] cherry-pick: Handle large output in v2 shim with TTY
2 parents 7aaa8fc + 0f6aab1 commit aa98dc6

5 files changed

Lines changed: 114 additions & 7 deletions

File tree

‎container_test.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1578,3 +1578,105 @@ func TestShimSockLength(t *testing.T) {
15781578

15791579
<-statusC
15801580
}
1581+
1582+
func TestContainerExecLargeOutputWithTTY(t *testing.T) {
1583+
t.Parallel()
1584+
1585+
client, err := newClient(t, address)
1586+
if err != nil {
1587+
t.Fatal(err)
1588+
}
1589+
defer client.Close()
1590+
1591+
var (
1592+
image Image
1593+
ctx, cancel = testContext(t)
1594+
id = t.Name()
1595+
)
1596+
defer cancel()
1597+
1598+
image, err = client.GetImage(ctx, testImage)
1599+
if err != nil {
1600+
t.Fatal(err)
1601+
}
1602+
1603+
container, err := client.NewContainer(ctx, id, WithNewSnapshot(id, image), WithNewSpec(oci.WithImageConfig(image), withProcessArgs("sleep", "999")))
1604+
if err != nil {
1605+
t.Fatal(err)
1606+
}
1607+
defer container.Delete(ctx, WithSnapshotCleanup)
1608+
1609+
task, err := container.NewTask(ctx, empty())
1610+
if err != nil {
1611+
t.Fatal(err)
1612+
}
1613+
defer task.Delete(ctx)
1614+
1615+
finishedC, err := task.Wait(ctx)
1616+
if err != nil {
1617+
t.Fatal(err)
1618+
}
1619+
1620+
if err := task.Start(ctx); err != nil {
1621+
t.Fatal(err)
1622+
}
1623+
1624+
for i := 0; i < 100; i++ {
1625+
spec, err := container.Spec(ctx)
1626+
if err != nil {
1627+
t.Fatal(err)
1628+
}
1629+
1630+
// start an exec process without running the original container process info
1631+
processSpec := spec.Process
1632+
withExecArgs(processSpec, "sh", "-c", `seq -s " " 1000000`)
1633+
1634+
stdout := bytes.NewBuffer(nil)
1635+
1636+
execID := t.Name() + "_exec"
1637+
process, err := task.Exec(ctx, execID, processSpec, cio.NewCreator(withByteBuffers(stdout), withProcessTTY()))
1638+
if err != nil {
1639+
t.Fatal(err)
1640+
}
1641+
processStatusC, err := process.Wait(ctx)
1642+
if err != nil {
1643+
t.Fatal(err)
1644+
}
1645+
1646+
if err := process.Start(ctx); err != nil {
1647+
t.Fatal(err)
1648+
}
1649+
1650+
// wait for the exec to return
1651+
status := <-processStatusC
1652+
code, _, err := status.Result()
1653+
if err != nil {
1654+
t.Fatal(err)
1655+
}
1656+
1657+
if code != 0 {
1658+
t.Errorf("expected exec exit code 0 but received %d", code)
1659+
}
1660+
if _, err := process.Delete(ctx); err != nil {
1661+
t.Fatal(err)
1662+
}
1663+
1664+
const expectedSuffix = "999999 1000000"
1665+
stdoutString := stdout.String()
1666+
if !strings.Contains(stdoutString, expectedSuffix) {
1667+
t.Fatalf("process output does not end with %q at iteration %d, here are the last 20 characters of the output:\n\n %q", expectedSuffix, i, stdoutString[len(stdoutString)-20:])
1668+
}
1669+
1670+
}
1671+
1672+
if err := task.Kill(ctx, syscall.SIGKILL); err != nil {
1673+
t.Error(err)
1674+
}
1675+
<-finishedC
1676+
}
1677+
1678+
func withProcessTTY() cio.Opt {
1679+
return func(opt *cio.Streams) {
1680+
cio.WithTerminal(opt)
1681+
}
1682+
}

‎pkg/process/io.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ import (
4040

4141
var bufPool = sync.Pool{
4242
New: func() interface{} {
43-
buffer := make([]byte, 32<<10)
43+
// setting to 4096 to align with PIPE_BUF
44+
// http://man7.org/linux/man-pages/man7/pipe.7.html
45+
buffer := make([]byte, 4096)
4446
return &buffer
4547
},
4648
}

‎runtime/v1/shim/service.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ var (
5555
empty = &ptypes.Empty{}
5656
bufPool = sync.Pool{
5757
New: func() interface{} {
58-
buffer := make([]byte, 32<<10)
58+
buffer := make([]byte, 4096)
5959
return &buffer
6060
},
6161
}

‎runtime/v1/shim/service_linux.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
5555
io.CopyBuffer(epollConsole, in, *bp)
5656
// we need to shutdown epollConsole when pipe broken
5757
epollConsole.Shutdown(p.epoller.CloseConsole)
58+
epollConsole.Close()
5859
}()
5960
}
6061

@@ -73,9 +74,8 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
7374
p := bufPool.Get().(*[]byte)
7475
defer bufPool.Put(p)
7576
io.CopyBuffer(outw, epollConsole, *p)
76-
epollConsole.Close()
77-
outr.Close()
7877
outw.Close()
78+
outr.Close()
7979
wg.Done()
8080
}()
8181
cwg.Wait()

‎runtime/v2/runc/platform.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ import (
3232

3333
var bufPool = sync.Pool{
3434
New: func() interface{} {
35-
buffer := make([]byte, 32<<10)
35+
// setting to 4096 to align with PIPE_BUF
36+
// http://man7.org/linux/man-pages/man7/pipe.7.html
37+
buffer := make([]byte, 4096)
3638
return &buffer
3739
},
3840
}
@@ -77,6 +79,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
7779
io.CopyBuffer(epollConsole, in, *bp)
7880
// we need to shutdown epollConsole when pipe broken
7981
epollConsole.Shutdown(p.epoller.CloseConsole)
82+
epollConsole.Close()
8083
}()
8184
}
8285

@@ -95,9 +98,9 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
9598
buf := bufPool.Get().(*[]byte)
9699
defer bufPool.Put(buf)
97100
io.CopyBuffer(outw, epollConsole, *buf)
98-
epollConsole.Close()
99-
outr.Close()
101+
100102
outw.Close()
103+
outr.Close()
101104
wg.Done()
102105
}()
103106
cwg.Wait()

0 commit comments

Comments
 (0)