Remove backend type from SDK/API

Still need to figure out a way to test both, but we would rather not add the ability to select the backend through the API.
coder · code-asher · Aug 14, 2023 · Jul 18, 2023 · Aug 8, 2023 · Aug 8, 2023
commit ee670454caf2159e56c6b5908eadd1f5638263c7
diff --git a/agent/agent.go b/agent/agent.go
@@ -1108,7 +1108,7 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
 		}
 		c <- rpty // Put it back for the next reconnect.
 	} else {
-		logger.Debug(ctx, "creating new reconnecting pty", slog.F("backend", msg.BackendType))
+		logger.Debug(ctx, "creating new reconnecting pty")
 
 		connected := false
 		defer func() {
@@ -1126,9 +1126,8 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
 		}
 
 		rpty = reconnectingpty.New(ctx, cmd, &reconnectingpty.Options{
-			BackendType: msg.BackendType,
-			Timeout:     a.reconnectingPTYTimeout,
-			Metrics:     a.metrics.reconnectingPTYErrors,
+			Timeout: a.reconnectingPTYTimeout,
+			Metrics: a.metrics.reconnectingPTYErrors,
 		}, logger.With(slog.F("message_id", msg.ID)))
 
 		if err = a.trackConnGoroutine(func() {

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -103,7 +103,7 @@ func TestAgent_Stats_ReconnectingPTY(t *testing.T) {
 	//nolint:dogsled
 	conn, _, stats, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
 
-	ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "bash", codersdk.ReconnectingPTYBackendTypeBuffered)
+	ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "bash")
 	require.NoError(t, err)
 	defer ptyConn.Close()
 
@@ -1601,18 +1601,15 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
 		t.Skip("ConPTY appears to be inconsistent on Windows.")
 	}
 
-	backends := []codersdk.ReconnectingPTYBackendType{
-		codersdk.ReconnectingPTYBackendTypeBuffered,
-		codersdk.ReconnectingPTYBackendTypeScreen,
-	}
+	backends := []string{"Buffered", "Screen"}
 
 	for _, backendType := range backends {
 		backendType := backendType
-		t.Run(string(backendType), func(t *testing.T) {
+		t.Run(backendType, func(t *testing.T) {
 			t.Parallel()
 			if runtime.GOOS == "darwin" {
 				t.Skip("`screen` is flaky on darwin")
-			} else if backendType == codersdk.ReconnectingPTYBackendTypeScreen {
+			} else if backendType == "Screen" {
 				_, err := exec.LookPath("screen")
 				if err != nil {
 					t.Skip("`screen` not found")
@@ -1625,14 +1622,14 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
 			//nolint:dogsled
 			conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
 			id := uuid.New()
-			netConn1, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
+			netConn1, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
 			require.NoError(t, err)
 			defer netConn1.Close()
 
 			scanner1 := bufio.NewScanner(netConn1)
 
 			// A second simultaneous connection.
-			netConn2, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
+			netConn2, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
 			require.NoError(t, err)
 			defer netConn2.Close()
 			scanner2 := bufio.NewScanner(netConn2)
@@ -1683,7 +1680,7 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
 
 			_ = netConn1.Close()
 			_ = netConn2.Close()
-			netConn3, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash", backendType)
+			netConn3, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
 			require.NoError(t, err)
 			defer netConn3.Close()
 

diff --git a/agent/reconnectingpty/reconnectingpty.go b/agent/reconnectingpty/reconnectingpty.go
@@ -32,8 +32,6 @@ type Options struct {
 	Timeout time.Duration
 	// Metrics tracks various error counters.
 	Metrics *prometheus.CounterVec
-	// BackendType indicates which backend to use for reconnections.
-	BackendType codersdk.ReconnectingPTYBackendType
 }
 
 // ReconnectingPTY is a pty that can be reconnected within a timeout and to
@@ -64,23 +62,20 @@ func New(ctx context.Context, cmd *pty.Cmd, options *Options, logger slog.Logger
 	}
 	// Screen seems flaky on Darwin.  Locally the tests pass 100% of the time (100
 	// runs) but in CI screen often incorrectly claims the session name does not
-	// exist even though screen -list shows it.
-	if runtime.GOOS == "darwin" {
-		options.BackendType = codersdk.ReconnectingPTYBackendTypeBuffered
-	} else if options.BackendType == "" || options.BackendType == codersdk.ReconnectingPTYBackendTypeAuto {
+	// exist even though screen -list shows it.  For now, restrict screen to
+	// Linux.
+	backendType := "buffered"
+	if runtime.GOOS == "linux" {
 		_, err := exec.LookPath("screen")
 		if err == nil {
-			options.BackendType = codersdk.ReconnectingPTYBackendTypeScreen
-		} else {
-			options.BackendType = codersdk.ReconnectingPTYBackendTypeBuffered
+			backendType = "screen"
 		}
-		logger.Debug(ctx, "auto backend selection", slog.F("backend", options.BackendType))
 	}
 
-	logger.Info(ctx, "start reconnecting pty")
+	logger.Info(ctx, "start reconnecting pty", slog.F("backend_type", backendType))
 
-	switch options.BackendType {
-	case codersdk.ReconnectingPTYBackendTypeScreen:
+	switch backendType {
+	case "screen":
 		return newScreen(ctx, cmd, options, logger)
 	default:
 		return newBuffered(ctx, cmd, options, logger)

diff --git a/cli/exp_scaletest.go b/cli/exp_scaletest.go
@@ -676,11 +676,10 @@ func (r *RootCmd) scaletestCreateWorkspaces() *clibase.Cmd {
 					config.ReconnectingPTY = &reconnectingpty.Config{
 						// AgentID is set by the test automatically.
 						Init: codersdk.WorkspaceAgentReconnectingPTYInit{
-							ID:          uuid.Nil,
-							Height:      24,
-							Width:       80,
-							Command:     runCommand,
-							BackendType: codersdk.ReconnectingPTYBackendTypeBuffered,
+							ID:      uuid.Nil,
+							Height:  24,
+							Width:   80,
+							Command: runCommand,
 						},
 						Timeout:       httpapi.Duration(runTimeout),
 						ExpectTimeout: runExpectTimeout,

diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go
@@ -161,7 +161,7 @@ func (p *QueryParamParser) Strings(vals url.Values, def []string, queryParam str
 // ValidEnum parses enum query params. Add more to the list as needed.
 type ValidEnum interface {
 	database.ResourceType | database.AuditAction | database.BuildReason | database.UserStatus |
-		database.WorkspaceStatus | codersdk.ReconnectingPTYBackendType
+		database.WorkspaceStatus
 
 	// Valid is required on the enum type to be used with ParseEnum.
 	Valid() bool

diff --git a/coderd/insights_test.go b/coderd/insights_test.go
@@ -348,11 +348,10 @@ func TestTemplateInsights(t *testing.T) {
 
 	// Start an rpty session to generate rpty usage stats.
 	rpty, err := client.WorkspaceAgentReconnectingPTY(ctx, codersdk.WorkspaceAgentReconnectingPTYOpts{
-		AgentID:     resources[0].Agents[0].ID,
-		Reconnect:   uuid.New(),
-		Width:       80,
-		Height:      24,
-		BackendType: codersdk.ReconnectingPTYBackendTypeBuffered,
+		AgentID:   resources[0].Agents[0].ID,
+		Reconnect: uuid.New(),
+		Width:     80,
+		Height:    24,
 	})
 	require.NoError(t, err)
 	defer rpty.Close()

diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go
@@ -11,7 +11,6 @@ import (
 	"net/http/cookiejar"
 	"net/http/httputil"
 	"net/url"
-	"os/exec"
 	"path"
 	"regexp"
 	"runtime"
@@ -57,81 +56,59 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
 			t.Skip("ConPTY appears to be inconsistent on Windows.")
 		}
 
-		backends := []codersdk.ReconnectingPTYBackendType{
-			codersdk.ReconnectingPTYBackendTypeBuffered,
-			codersdk.ReconnectingPTYBackendTypeScreen,
-		}
-
-		for _, backendType := range backends {
-			backendType := backendType
-			t.Run(string(backendType), func(t *testing.T) {
-				t.Parallel()
-				if runtime.GOOS == "darwin" {
-					t.Skip("`screen` is flaky on darwin")
-				} else if backendType == codersdk.ReconnectingPTYBackendTypeScreen {
-					_, err := exec.LookPath("screen")
-					if err != nil {
-						t.Skip("`screen` not found")
-					}
-				}
-
-				t.Run("OK", func(t *testing.T) {
-					t.Parallel()
-					appDetails := setupProxyTest(t, nil)
+		t.Run("OK", func(t *testing.T) {
+			t.Parallel()
+			appDetails := setupProxyTest(t, nil)
 
-					ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
-					defer cancel()
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+			defer cancel()
 
-					// Run the test against the path app hostname since that's where the
-					// reconnecting-pty proxy server we want to test is mounted.
-					client := appDetails.AppClient(t)
-					testReconnectingPTY(ctx, t, client, codersdk.WorkspaceAgentReconnectingPTYOpts{
-						AgentID:     appDetails.Agent.ID,
-						Reconnect:   uuid.New(),
-						Height:      80,
-						Width:       80,
-						Command:     "bash",
-						BackendType: backendType,
-					})
-				})
+			// Run the test against the path app hostname since that's where the
+			// reconnecting-pty proxy server we want to test is mounted.
+			client := appDetails.AppClient(t)
+			testReconnectingPTY(ctx, t, client, codersdk.WorkspaceAgentReconnectingPTYOpts{
+				AgentID:   appDetails.Agent.ID,
+				Reconnect: uuid.New(),
+				Height:    80,
+				Width:     80,
+				Command:   "bash",
+			})
+		})
 
-				t.Run("SignedTokenQueryParameter", func(t *testing.T) {
-					t.Parallel()
-					if appHostIsPrimary {
-						t.Skip("Tickets are not used for terminal requests on the primary.")
-					}
+		t.Run("SignedTokenQueryParameter", func(t *testing.T) {
+			t.Parallel()
+			if appHostIsPrimary {
+				t.Skip("Tickets are not used for terminal requests on the primary.")
+			}
 
-					appDetails := setupProxyTest(t, nil)
+			appDetails := setupProxyTest(t, nil)
 
-					u := *appDetails.PathAppBaseURL
-					if u.Scheme == "http" {
-						u.Scheme = "ws"
-					} else {
-						u.Scheme = "wss"
-					}
-					u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
+			u := *appDetails.PathAppBaseURL
+			if u.Scheme == "http" {
+				u.Scheme = "ws"
+			} else {
+				u.Scheme = "wss"
+			}
+			u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
 
-					ctx := testutil.Context(t, testutil.WaitLong)
-					issueRes, err := appDetails.SDKClient.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
-						URL:     u.String(),
-						AgentID: appDetails.Agent.ID,
-					})
-					require.NoError(t, err)
+			ctx := testutil.Context(t, testutil.WaitLong)
+			issueRes, err := appDetails.SDKClient.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
+				URL:     u.String(),
+				AgentID: appDetails.Agent.ID,
+			})
+			require.NoError(t, err)
 
-					// Make an unauthenticated client.
-					unauthedAppClient := codersdk.New(appDetails.AppClient(t).URL)
-					testReconnectingPTY(ctx, t, unauthedAppClient, codersdk.WorkspaceAgentReconnectingPTYOpts{
-						AgentID:     appDetails.Agent.ID,
-						Reconnect:   uuid.New(),
-						Height:      80,
-						Width:       80,
-						Command:     "bash",
-						SignedToken: issueRes.SignedToken,
-						BackendType: backendType,
-					})
-				})
+			// Make an unauthenticated client.
+			unauthedAppClient := codersdk.New(appDetails.AppClient(t).URL)
+			testReconnectingPTY(ctx, t, unauthedAppClient, codersdk.WorkspaceAgentReconnectingPTYOpts{
+				AgentID:     appDetails.Agent.ID,
+				Reconnect:   uuid.New(),
+				Height:      80,
+				Width:       80,
+				Command:     "bash",
+				SignedToken: issueRes.SignedToken,
 			})
-		}
+		})
 	})
 
 	t.Run("WorkspaceAppsProxyPath", func(t *testing.T) {

diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go
@@ -632,7 +632,6 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
 	reconnect := parser.Required("reconnect").UUID(values, uuid.New(), "reconnect")
 	height := parser.UInt(values, 80, "height")
 	width := parser.UInt(values, 80, "width")
-	backendType := httpapi.ParseCustom(parser, values, codersdk.ReconnectingPTYBackendTypeAuto, "backend", httpapi.ParseEnum[codersdk.ReconnectingPTYBackendType])
 	if len(parser.Errors) > 0 {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message:     "Invalid query parameters.",
@@ -671,7 +670,7 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
 	}
 	defer release()
 	log.Debug(ctx, "dialed workspace agent")
-	ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command"), backendType)
+	ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command"))
 	if err != nil {
 		log.Debug(ctx, "dial reconnecting pty server in workspace agent", slog.Error(err))
 		_ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial: %s", err))

diff --git a/codersdk/workspaceagentconn.go b/codersdk/workspaceagentconn.go
@@ -193,32 +193,13 @@ func (c *WorkspaceAgentConn) Close() error {
 	return c.Conn.Close()
 }
 
-type ReconnectingPTYBackendType string
-
-const (
-	ReconnectingPTYBackendTypeAuto     ReconnectingPTYBackendType = "auto"
-	ReconnectingPTYBackendTypeScreen   ReconnectingPTYBackendType = "screen"
-	ReconnectingPTYBackendTypeBuffered ReconnectingPTYBackendType = "buffered"
-)
-
-func (s ReconnectingPTYBackendType) Valid() bool {
-	switch s {
-	case ReconnectingPTYBackendTypeAuto, ReconnectingPTYBackendTypeBuffered,
-		ReconnectingPTYBackendTypeScreen:
-		return true
-	default:
-		return false
-	}
-}
-
 // WorkspaceAgentReconnectingPTYInit initializes a new reconnecting PTY session.
 // @typescript-ignore WorkspaceAgentReconnectingPTYInit
 type WorkspaceAgentReconnectingPTYInit struct {
-	ID          uuid.UUID
-	Height      uint16
-	Width       uint16
-	Command     string
-	BackendType ReconnectingPTYBackendType
+	ID      uuid.UUID
+	Height  uint16
+	Width   uint16
+	Command string
 }
 
 // ReconnectingPTYRequest is sent from the client to the server
@@ -233,7 +214,7 @@ type ReconnectingPTYRequest struct {
 // ReconnectingPTY spawns a new reconnecting terminal session.
 // `ReconnectingPTYRequest` should be JSON marshaled and written to the returned net.Conn.
 // Raw terminal output will be read from the returned net.Conn.
-func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string, backendType ReconnectingPTYBackendType) (net.Conn, error) {
+func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string) (net.Conn, error) {
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.End()
 
@@ -246,11 +227,10 @@ func (c *WorkspaceAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID,
 		return nil, err
 	}
 	data, err := json.Marshal(WorkspaceAgentReconnectingPTYInit{
-		ID:          id,
-		Height:      height,
-		Width:       width,
-		Command:     command,
-		BackendType: backendType,
+		ID:      id,
+		Height:  height,
+		Width:   width,
+		Command: command,
 	})
 	if err != nil {
 		_ = conn.Close()

diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go
@@ -552,12 +552,11 @@ func (c *Client) IssueReconnectingPTYSignedToken(ctx context.Context, req IssueR
 
 // @typescript-ignore:WorkspaceAgentReconnectingPTYOpts
 type WorkspaceAgentReconnectingPTYOpts struct {
-	AgentID     uuid.UUID
-	Reconnect   uuid.UUID
-	Width       uint16
-	Height      uint16
-	Command     string
-	BackendType ReconnectingPTYBackendType
+	AgentID   uuid.UUID
+	Reconnect uuid.UUID
+	Width     uint16
+	Height    uint16
+	Command   string
 
 	// SignedToken is an optional signed token from the
 	// issue-reconnecting-pty-signed-token endpoint. If set, the session token
@@ -578,7 +577,6 @@ func (c *Client) WorkspaceAgentReconnectingPTY(ctx context.Context, opts Workspa
 	q.Set("width", strconv.Itoa(int(opts.Width)))
 	q.Set("height", strconv.Itoa(int(opts.Height)))
 	q.Set("command", opts.Command)
-	q.Set("backend", string(opts.BackendType))
 	// If we're using a signed token, set the query parameter.
 	if opts.SignedToken != "" {
 		q.Set(SignedAppTokenQueryParameter, opts.SignedToken)