Skip to content

fix: log after test exit in TestAgent/StartupScript #1726

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.

Sign up for GitHub

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 2 commits into from
May 24, 2022
Merged

fix: log after test exit in TestAgent/StartupScript #1726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78cd9e0
fix: log after test exit in `TestAgent/StartupScript`
coadler May 24, 2022
37f2511
move protocol to agent
coadler May 24, 2022
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
29 changes: 17 additions & 12 deletions agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,19 @@ import (
"time"

"github.com/armon/circbuf"
"github.com/gliderlabs/ssh"
"github.com/google/uuid"

"github.com/pkg/sftp"
"go.uber.org/atomic"
gossh "golang.org/x/crypto/ssh"
"golang.org/x/xerrors"

"cdr.dev/slog"
"github.com/coder/coder/agent/usershell"
"github.com/coder/coder/peer"
"github.com/coder/coder/peerbroker"
"github.com/coder/coder/pty"
"github.com/coder/retry"

"github.com/pkg/sftp"

"github.com/gliderlabs/ssh"
gossh "golang.org/x/crypto/ssh"
"golang.org/x/xerrors"
)

type Options struct {
Expand Down Expand Up @@ -174,17 +171,25 @@ func (*agent) runStartupScript(ctx context.Context, script string) error {
defer func() {
_ = writer.Close()
}()

caller := "-c"
if runtime.GOOS == "windows" {
caller = "/c"
}

cmd := exec.CommandContext(ctx, shell, caller, script)
cmd.Stdout = writer
cmd.Stderr = writer
err = cmd.Run()
if err != nil {
// cmd.Run does not return a context canceled error, it returns "signal: killed".
if ctx.Err() != nil {
return ctx.Err()
}

return xerrors.Errorf("run: %w", err)
}

return nil
}

Expand All @@ -208,11 +213,11 @@ func (a *agent) handlePeerConn(ctx context.Context, conn *peer.Conn) {
}

switch channel.Protocol() {
case "ssh":
case peer.ProtocolSSH:
go a.sshServer.HandleConn(channel.NetConn())
case "reconnecting-pty":
case peer.ProtocolReconnectingPTY:
go a.handleReconnectingPTY(ctx, channel.Label(), channel.NetConn())
case "dial":
case peer.ProtocolDial:
go a.handleDial(ctx, channel.Label(), channel.NetConn())
default:
a.logger.Warn(ctx, "unhandled protocol from channel",
Expand Down Expand Up @@ -478,8 +483,8 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
a.logger.Warn(ctx, "start reconnecting pty command", slog.F("id", id))
}

// Default to buffer 64KB.
circularBuffer, err := circbuf.NewBuffer(64 * 1024)
// Default to buffer 64KiB.
circularBuffer, err := circbuf.NewBuffer(64 << 10)
if err != nil {
a.logger.Warn(ctx, "create circular buffer", slog.Error(err))
return
Expand Down
5 changes: 4 additions & 1 deletion agent/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,9 @@ func TestAgent(t *testing.T) {
tempPath := filepath.Join(os.TempDir(), "content.txt")
content := "somethingnice"
setupAgent(t, agent.Metadata{
StartupScript: "echo " + content + " > " + tempPath,
StartupScript: fmt.Sprintf("echo %s > %s", content, tempPath),
}, 0)

var gotContent string
require.Eventually(t, func() bool {
content, err := os.ReadFile(tempPath)
Expand Down Expand Up @@ -202,6 +203,7 @@ func TestAgent(t *testing.T) {
// it seems like it could be either.
t.Skip("ConPTY appears to be inconsistent on Windows.")
}

conn := setupAgent(t, agent.Metadata{}, 0)
id := uuid.NewString()
netConn, err := conn.ReconnectingPTY(id, 100, 100)
Expand All @@ -228,6 +230,7 @@ func TestAgent(t *testing.T) {
}
}
}

matchEchoCommand := func(line string) bool {
return strings.Contains(line, "echo test")
}
Expand Down
6 changes: 3 additions & 3 deletions agent/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Conn struct {
// be reconnected to via ID.
func (c *Conn) ReconnectingPTY(id string, height, width uint16) (net.Conn, error) {
channel, err := c.CreateChannel(context.Background(), fmt.Sprintf("%s:%d:%d", id, height, width), &peer.ChannelOptions{
Protocol: "reconnecting-pty",
Protocol: peer.ProtocolReconnectingPTY,
})
if err != nil {
return nil, xerrors.Errorf("pty: %w", err)
Expand All @@ -47,7 +47,7 @@ func (c *Conn) ReconnectingPTY(id string, height, width uint16) (net.Conn, error
// SSH dials the built-in SSH server.
func (c *Conn) SSH() (net.Conn, error) {
channel, err := c.CreateChannel(context.Background(), "ssh", &peer.ChannelOptions{
Protocol: "ssh",
Protocol: peer.ProtocolSSH,
})
if err != nil {
return nil, xerrors.Errorf("dial: %w", err)
Expand Down Expand Up @@ -87,7 +87,7 @@ func (c *Conn) DialContext(ctx context.Context, network string, addr string) (ne
}

channel, err := c.CreateChannel(ctx, u.String(), &peer.ChannelOptions{
Protocol: "dial",
Protocol: peer.ProtocolDial,
Unordered: strings.HasPrefix(network, "udp"),
})
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions peer/channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ func newChannel(conn *Conn, dc *webrtc.DataChannel, opts *ChannelOptions) *Chann
return channel
}

const (
ProtocolReconnectingPTY = "reconnecting-pty"
ProtocolSSH = "ssh"
ProtocolDial = "dial"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should toss these in the agent instead, since the protocol types are disconnected from peer.


type ChannelOptions struct {
// ID is a channel ID that should be used when `Negotiated`
// is true.
Expand Down