X Tutup
Skip to content

Commit a033b85

Browse files
josebaliusjosebalius
authored
Merge pull request cli#4461 from cli/jg/liveshare-keepalive
codespace/liveshare keepalive: keep LS sessions alive with PF traffic
2 parents 12e5b94 + 97b52b3 commit a033b85

File tree

15 files changed

+433
-32
lines changed

15 files changed

+433
-32
lines changed

‎internal/codespaces/codespaces.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ type logger interface {
1515
Println(v ...interface{}) (int, error)
1616
}
1717

18+
// TODO(josebalius): clean this up once we standardrize
19+
// logging for codespaces
20+
type liveshareLogger interface {
21+
Println(v ...interface{})
22+
Printf(f string, v ...interface{})
23+
}
24+
1825
func connectionReady(codespace *api.Codespace) bool {
1926
return codespace.Connection.SessionID != "" &&
2027
codespace.Connection.SessionToken != "" &&
@@ -30,7 +37,7 @@ type apiClient interface {
3037

3138
// ConnectToLiveshare waits for a Codespace to become running,
3239
// and connects to it using a Live Share session.
33-
func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace) (*liveshare.Session, error) {
40+
func ConnectToLiveshare(ctx context.Context, log logger, sessionLogger liveshareLogger, apiClient apiClient, codespace *api.Codespace) (*liveshare.Session, error) {
3441
var startedCodespace bool
3542
if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable {
3643
startedCodespace = true
@@ -67,10 +74,12 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, co
6774
log.Println("Connecting to your codespace...")
6875

6976
return liveshare.Connect(ctx, liveshare.Options{
77+
ClientName: "gh",
7078
SessionID: codespace.Connection.SessionID,
7179
SessionToken: codespace.Connection.SessionToken,
7280
RelaySAS: codespace.Connection.RelaySAS,
7381
RelayEndpoint: codespace.Connection.RelayEndpoint,
7482
HostPublicKeys: codespace.Connection.HostPublicKeys,
83+
Logger: sessionLogger,
7584
})
7685
}

‎internal/codespaces/states.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"context"
66
"encoding/json"
77
"fmt"
8+
"io/ioutil"
9+
"log"
810
"net"
911
"strings"
1012
"time"
@@ -36,8 +38,10 @@ type PostCreateState struct {
3638
// PollPostCreateStates watches for state changes in a codespace,
3739
// and calls the supplied poller for each batch of state changes.
3840
// It runs until it encounters an error, including cancellation of the context.
39-
func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace, poller func([]PostCreateState)) (err error) {
40-
session, err := ConnectToLiveshare(ctx, log, apiClient, codespace)
41+
func PollPostCreateStates(ctx context.Context, logger logger, apiClient apiClient, codespace *api.Codespace, poller func([]PostCreateState)) (err error) {
42+
noopLogger := log.New(ioutil.Discard, "", 0)
43+
44+
session, err := ConnectToLiveshare(ctx, logger, noopLogger, apiClient, codespace)
4145
if err != nil {
4246
return fmt.Errorf("connect to Live Share: %w", err)
4347
}
@@ -54,15 +58,15 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient,
5458
}
5559
localPort := listen.Addr().(*net.TCPAddr).Port
5660

57-
log.Println("Fetching SSH Details...")
61+
logger.Println("Fetching SSH Details...")
5862
remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx)
5963
if err != nil {
6064
return fmt.Errorf("error getting ssh server details: %w", err)
6165
}
6266

6367
tunnelClosed := make(chan error, 1) // buffered to avoid sender stuckness
6468
go func() {
65-
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort)
69+
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, false)
6670
tunnelClosed <- fwd.ForwardToListener(ctx, listen) // error is non-nil
6771
}()
6872

‎pkg/cmd/codespace/common.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"errors"
88
"fmt"
99
"io"
10+
"io/ioutil"
11+
"log"
1012
"os"
1113
"sort"
1214
"strings"
@@ -211,6 +213,10 @@ func noArgsConstraint(cmd *cobra.Command, args []string) error {
211213
return nil
212214
}
213215

216+
func noopLogger() *log.Logger {
217+
return log.New(ioutil.Discard, "", 0)
218+
}
219+
214220
type codespace struct {
215221
*api.Codespace
216222
}

‎pkg/cmd/codespace/delete_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func TestDelete(t *testing.T) {
4444
},
4545
},
4646
wantDeleted: []string{"hubot-robawt-abc"},
47+
wantStdout: "Codespace deleted.\n",
4748
},
4849
{
4950
name: "by repo",
@@ -65,6 +66,7 @@ func TestDelete(t *testing.T) {
6566
},
6667
},
6768
wantDeleted: []string{"monalisa-spoonknife-123", "monalisa-spoonknife-c4f3"},
69+
wantStdout: "Codespaces deleted.\n",
6870
},
6971
{
7072
name: "unused",
@@ -87,6 +89,7 @@ func TestDelete(t *testing.T) {
8789
},
8890
},
8991
wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"},
92+
wantStdout: "Codespaces deleted.\n",
9093
},
9194
{
9295
name: "deletion failed",
@@ -148,6 +151,7 @@ func TestDelete(t *testing.T) {
148151
"Codespace hubot-robawt-abc has unsaved changes. OK to delete?": true,
149152
},
150153
wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"},
154+
wantStdout: "Codespaces deleted.\n",
151155
},
152156
}
153157
for _, tt := range tests {

‎pkg/cmd/codespace/logs.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err
5151
return fmt.Errorf("get or choose codespace: %w", err)
5252
}
5353

54-
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace)
54+
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, noopLogger(), a.apiClient, codespace)
5555
if err != nil {
5656
return fmt.Errorf("connecting to Live Share: %w", err)
5757
}
@@ -90,7 +90,7 @@ func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err
9090

9191
tunnelClosed := make(chan error, 1)
9292
go func() {
93-
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort)
93+
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, false)
9494
tunnelClosed <- fwd.ForwardToListener(ctx, listen) // error is non-nil
9595
}()
9696

‎pkg/cmd/codespace/output/logger.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@ import (
99
// NewLogger returns a Logger that will write to the given stdout/stderr writers.
1010
// Disable the Logger to prevent it from writing to stdout in a TTY environment.
1111
func NewLogger(stdout, stderr io.Writer, disabled bool) *Logger {
12+
enabled := !disabled
13+
if isTTY(stdout) && !enabled {
14+
enabled = false
15+
}
1216
return &Logger{
1317
out: stdout,
1418
errout: stderr,
15-
enabled: !disabled && isTTY(stdout),
19+
enabled: enabled,
1620
}
1721
}
1822

‎pkg/cmd/codespace/ports.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool)
6060

6161
devContainerCh := getDevContainer(ctx, a.apiClient, codespace)
6262

63-
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace)
63+
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, noopLogger(), a.apiClient, codespace)
6464
if err != nil {
6565
return fmt.Errorf("error connecting to Live Share: %w", err)
6666
}
@@ -194,7 +194,7 @@ func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName, sourcePor
194194
return fmt.Errorf("error getting codespace: %w", err)
195195
}
196196

197-
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace)
197+
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, noopLogger(), a.apiClient, codespace)
198198
if err != nil {
199199
return fmt.Errorf("error connecting to Live Share: %w", err)
200200
}
@@ -253,7 +253,7 @@ func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []st
253253
return fmt.Errorf("error getting codespace: %w", err)
254254
}
255255

256-
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace)
256+
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, noopLogger(), a.apiClient, codespace)
257257
if err != nil {
258258
return fmt.Errorf("error connecting to Live Share: %w", err)
259259
}
@@ -272,7 +272,7 @@ func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []st
272272
defer listen.Close()
273273
a.logger.Printf("Forwarding ports: remote %d <=> local %d\n", pair.remote, pair.local)
274274
name := fmt.Sprintf("share-%d", pair.remote)
275-
fwd := liveshare.NewPortForwarder(session, name, pair.remote)
275+
fwd := liveshare.NewPortForwarder(session, name, pair.remote, false)
276276
return fwd.ForwardToListener(ctx, listen) // error always non-nil
277277
})
278278
}

‎pkg/cmd/codespace/ssh.go‎

Lines changed: 74 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,46 @@ package codespace
33
import (
44
"context"
55
"fmt"
6+
"io/ioutil"
7+
"log"
68
"net"
9+
"os"
710

811
"github.com/cli/cli/v2/internal/codespaces"
912
"github.com/cli/cli/v2/pkg/liveshare"
1013
"github.com/spf13/cobra"
1114
)
1215

16+
type sshOptions struct {
17+
codespace string
18+
profile string
19+
serverPort int
20+
debug bool
21+
debugFile string
22+
}
23+
1324
func newSSHCmd(app *App) *cobra.Command {
14-
var sshProfile, codespaceName string
15-
var sshServerPort int
25+
var opts sshOptions
1626

1727
sshCmd := &cobra.Command{
1828
Use: "ssh [flags] [--] [ssh-flags] [command]",
1929
Short: "SSH into a codespace",
2030
RunE: func(cmd *cobra.Command, args []string) error {
21-
return app.SSH(cmd.Context(), args, sshProfile, codespaceName, sshServerPort)
31+
return app.SSH(cmd.Context(), args, opts)
2232
},
2333
}
2434

25-
sshCmd.Flags().StringVarP(&sshProfile, "profile", "", "", "Name of the SSH profile to use")
26-
sshCmd.Flags().IntVarP(&sshServerPort, "server-port", "", 0, "SSH server port number (0 => pick unused)")
27-
sshCmd.Flags().StringVarP(&codespaceName, "codespace", "c", "", "Name of the codespace")
35+
sshCmd.Flags().StringVarP(&opts.profile, "profile", "", "", "Name of the SSH profile to use")
36+
sshCmd.Flags().IntVarP(&opts.serverPort, "server-port", "", 0, "SSH server port number (0 => pick unused)")
37+
sshCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
38+
sshCmd.Flags().BoolVarP(&opts.debug, "debug", "d", false, "Log debug data to a file")
39+
sshCmd.Flags().StringVarP(&opts.debugFile, "debug-file", "", "", "Path of the file log to")
2840

2941
return sshCmd
3042
}
3143

3244
// SSH opens an ssh session or runs an ssh command in a codespace.
33-
func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceName string, localSSHServerPort int) (err error) {
45+
func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err error) {
3446
// Ensure all child tasks (e.g. port forwarding) terminate before return.
3547
ctx, cancel := context.WithCancel(ctx)
3648
defer cancel()
@@ -45,12 +57,22 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceNa
4557
authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login)
4658
}()
4759

48-
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
60+
codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
4961
if err != nil {
5062
return fmt.Errorf("get or choose codespace: %w", err)
5163
}
5264

53-
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace)
65+
var debugLogger *fileLogger
66+
if opts.debug {
67+
debugLogger, err = newFileLogger(opts.debugFile)
68+
if err != nil {
69+
return fmt.Errorf("error creating debug logger: %w", err)
70+
}
71+
defer safeClose(debugLogger, &err)
72+
a.logger.Println("Debug file located at: " + debugLogger.Name())
73+
}
74+
75+
session, err := codespaces.ConnectToLiveshare(ctx, a.logger, debugLogger, a.apiClient, codespace)
5476
if err != nil {
5577
return fmt.Errorf("error connecting to Live Share: %w", err)
5678
}
@@ -66,6 +88,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceNa
6688
return fmt.Errorf("error getting ssh server details: %w", err)
6789
}
6890

91+
localSSHServerPort := opts.serverPort
6992
usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell
7093

7194
// Ensure local port is listening before client (Shell) connects.
@@ -76,15 +99,15 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceNa
7699
defer listen.Close()
77100
localSSHServerPort = listen.Addr().(*net.TCPAddr).Port
78101

79-
connectDestination := sshProfile
102+
connectDestination := opts.profile
80103
if connectDestination == "" {
81104
connectDestination = fmt.Sprintf("%s@localhost", sshUser)
82105
}
83106

84107
a.logger.Println("Ready...")
85108
tunnelClosed := make(chan error, 1)
86109
go func() {
87-
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort)
110+
fwd := liveshare.NewPortForwarder(session, "sshd", remoteSSHServerPort, true)
88111
tunnelClosed <- fwd.ForwardToListener(ctx, listen) // always non-nil
89112
}()
90113

@@ -103,3 +126,43 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceNa
103126
return nil // success
104127
}
105128
}
129+
130+
// fileLogger is a wrapper around an log.Logger configured to write
131+
// to a file. It exports two additional methods to get the log file name
132+
// and close the file handle when the operation is finished.
133+
type fileLogger struct {
134+
*log.Logger
135+
136+
f *os.File
137+
}
138+
139+
// newFileLogger creates a new fileLogger. It returns an error if the file
140+
// cannot be created. The file is created on the specified path, if the path
141+
// is empty it is created in the temporary directory.
142+
func newFileLogger(file string) (fl *fileLogger, err error) {
143+
var f *os.File
144+
if file == "" {
145+
f, err = ioutil.TempFile("", "")
146+
if err != nil {
147+
return nil, fmt.Errorf("failed to create tmp file: %w", err)
148+
}
149+
} else {
150+
f, err = os.Create(file)
151+
if err != nil {
152+
return nil, err
153+
}
154+
}
155+
156+
return &fileLogger{
157+
Logger: log.New(f, "", log.LstdFlags),
158+
f: f,
159+
}, nil
160+
}
161+
162+
func (fl *fileLogger) Name() string {
163+
return fl.f.Name()
164+
}
165+
166+
func (fl *fileLogger) Close() error {
167+
return fl.f.Close()
168+
}

0 commit comments

Comments
 (0)
X Tutup