Skip to content

Commit 84fd4a7

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Merge branch 'main' into bq/aks-missing-parameters-on-update
2 parents 0998ae3 + 1cc10f2 commit 84fd4a7

File tree

71 files changed

+2712
-639
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+2712
-639
lines changed

‎agent/agent.go

+21-18
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"os/exec"
1818
"os/user"
1919
"path/filepath"
20+
"reflect"
2021
"runtime"
2122
"sort"
2223
"strconv"
@@ -60,7 +61,7 @@ const (
6061

6162
// MagicSSHSessionTypeEnvironmentVariable is used to track the purpose behind an SSH connection.
6263
// This is stripped from any commands being executed, and is counted towards connection stats.
63-
MagicSSHSessionTypeEnvironmentVariable = "__CODER_SSH_SESSION_TYPE"
64+
MagicSSHSessionTypeEnvironmentVariable = "CODER_SSH_SESSION_TYPE"
6465
// MagicSSHSessionTypeVSCode is set in the SSH config by the VS Code extension to identify itself.
6566
MagicSSHSessionTypeVSCode = "vscode"
6667
// MagicSSHSessionTypeJetBrains is set in the SSH config by the JetBrains extension to identify itself.
@@ -122,9 +123,7 @@ func New(options Options) io.Closer {
122123
tempDir: options.TempDir,
123124
lifecycleUpdate: make(chan struct{}, 1),
124125
lifecycleReported: make(chan codersdk.WorkspaceAgentLifecycle, 1),
125-
// TODO: This is a temporary hack to make tests not flake.
126-
// @kylecarbs has a better solution in here: https://github.com/coder/coder/pull/6469
127-
connStatsChan: make(chan *agentsdk.Stats, 8),
126+
connStatsChan: make(chan *agentsdk.Stats, 1),
128127
}
129128
a.init(ctx)
130129
return a
@@ -159,11 +158,8 @@ type agent struct {
159158

160159
network *tailnet.Conn
161160
connStatsChan chan *agentsdk.Stats
161+
latestStat atomic.Pointer[agentsdk.Stats]
162162

163-
statRxPackets atomic.Int64
164-
statRxBytes atomic.Int64
165-
statTxPackets atomic.Int64
166-
statTxBytes atomic.Int64
167163
connCountVSCode atomic.Int64
168164
connCountJetBrains atomic.Int64
169165
connCountReconnectingPTY atomic.Int64
@@ -905,10 +901,13 @@ func (a *agent) handleSSHSession(session ssh.Session) (retErr error) {
905901
switch magicType {
906902
case MagicSSHSessionTypeVSCode:
907903
a.connCountVSCode.Add(1)
904+
defer a.connCountVSCode.Add(-1)
908905
case MagicSSHSessionTypeJetBrains:
909906
a.connCountJetBrains.Add(1)
907+
defer a.connCountJetBrains.Add(-1)
910908
case "":
911909
a.connCountSSHSession.Add(1)
910+
defer a.connCountSSHSession.Add(-1)
912911
default:
913912
a.logger.Warn(ctx, "invalid magic ssh session type specified", slog.F("type", magicType))
914913
}
@@ -1012,6 +1011,7 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
10121011
defer conn.Close()
10131012

10141013
a.connCountReconnectingPTY.Add(1)
1014+
defer a.connCountReconnectingPTY.Add(-1)
10151015

10161016
connectionID := uuid.NewString()
10171017
logger = logger.With(slog.F("id", msg.ID), slog.F("connection_id", connectionID))
@@ -1210,18 +1210,15 @@ func (a *agent) startReportingConnectionStats(ctx context.Context) {
12101210
ConnectionCount: int64(len(networkStats)),
12111211
ConnectionsByProto: map[string]int64{},
12121212
}
1213-
// Tailscale resets counts on every report!
1214-
// We'd rather have these compound, like Linux does!
12151213
for conn, counts := range networkStats {
12161214
stats.ConnectionsByProto[conn.Proto.String()]++
1217-
stats.RxBytes = a.statRxBytes.Add(int64(counts.RxBytes))
1218-
stats.RxPackets = a.statRxPackets.Add(int64(counts.RxPackets))
1219-
stats.TxBytes = a.statTxBytes.Add(int64(counts.TxBytes))
1220-
stats.TxPackets = a.statTxPackets.Add(int64(counts.TxPackets))
1215+
stats.RxBytes += int64(counts.RxBytes)
1216+
stats.RxPackets += int64(counts.RxPackets)
1217+
stats.TxBytes += int64(counts.TxBytes)
1218+
stats.TxPackets += int64(counts.TxPackets)
12211219
}
12221220

1223-
// Tailscale's connection stats are not cumulative, but it makes no sense to make
1224-
// ours temporary.
1221+
// The count of active sessions.
12251222
stats.SessionCountSSH = a.connCountSSHSession.Load()
12261223
stats.SessionCountVSCode = a.connCountVSCode.Load()
12271224
stats.SessionCountJetBrains = a.connCountJetBrains.Load()
@@ -1270,10 +1267,16 @@ func (a *agent) startReportingConnectionStats(ctx context.Context) {
12701267
// Convert from microseconds to milliseconds.
12711268
stats.ConnectionMedianLatencyMS /= 1000
12721269

1270+
lastStat := a.latestStat.Load()
1271+
if lastStat != nil && reflect.DeepEqual(lastStat, stats) {
1272+
a.logger.Info(ctx, "skipping stat because nothing changed")
1273+
return
1274+
}
1275+
a.latestStat.Store(stats)
1276+
12731277
select {
12741278
case a.connStatsChan <- stats:
1275-
default:
1276-
a.logger.Warn(ctx, "network stat dropped")
1279+
case <-a.closed:
12771280
}
12781281
}
12791282

‎agent/agent_test.go

+69-37
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ func TestAgent_Stats_SSH(t *testing.T) {
6868
session, err := sshClient.NewSession()
6969
require.NoError(t, err)
7070
defer session.Close()
71-
require.NoError(t, session.Run("echo test"))
71+
stdin, err := session.StdinPipe()
72+
require.NoError(t, err)
73+
err = session.Shell()
74+
require.NoError(t, err)
7275

7376
var s *agentsdk.Stats
7477
require.Eventuallyf(t, func() bool {
@@ -78,6 +81,9 @@ func TestAgent_Stats_SSH(t *testing.T) {
7881
}, testutil.WaitLong, testutil.IntervalFast,
7982
"never saw stats: %+v", s,
8083
)
84+
_ = stdin.Close()
85+
err = session.Wait()
86+
require.NoError(t, err)
8187
}
8288

8389
func TestAgent_Stats_ReconnectingPTY(t *testing.T) {
@@ -112,43 +118,69 @@ func TestAgent_Stats_ReconnectingPTY(t *testing.T) {
112118

113119
func TestAgent_Stats_Magic(t *testing.T) {
114120
t.Parallel()
121+
t.Run("StripsEnvironmentVariable", func(t *testing.T) {
122+
t.Parallel()
123+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
124+
defer cancel()
125+
//nolint:dogsled
126+
conn, _, _, _, _ := setupAgent(t, agentsdk.Metadata{}, 0)
127+
sshClient, err := conn.SSHClient(ctx)
128+
require.NoError(t, err)
129+
defer sshClient.Close()
130+
session, err := sshClient.NewSession()
131+
require.NoError(t, err)
132+
session.Setenv(agent.MagicSSHSessionTypeEnvironmentVariable, agent.MagicSSHSessionTypeVSCode)
133+
defer session.Close()
115134

116-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
117-
defer cancel()
118-
119-
//nolint:dogsled
120-
conn, _, stats, _, _ := setupAgent(t, agentsdk.Metadata{}, 0)
121-
sshClient, err := conn.SSHClient(ctx)
122-
require.NoError(t, err)
123-
defer sshClient.Close()
124-
session, err := sshClient.NewSession()
125-
require.NoError(t, err)
126-
session.Setenv(agent.MagicSSHSessionTypeEnvironmentVariable, agent.MagicSSHSessionTypeVSCode)
127-
defer session.Close()
128-
129-
command := "sh -c 'echo $" + agent.MagicSSHSessionTypeEnvironmentVariable + "'"
130-
expected := ""
131-
if runtime.GOOS == "windows" {
132-
expected = "%" + agent.MagicSSHSessionTypeEnvironmentVariable + "%"
133-
command = "cmd.exe /c echo " + expected
134-
}
135-
output, err := session.Output(command)
136-
require.NoError(t, err)
137-
require.Equal(t, expected, strings.TrimSpace(string(output)))
138-
var s *agentsdk.Stats
139-
require.Eventuallyf(t, func() bool {
140-
var ok bool
141-
s, ok = <-stats
142-
return ok && s.ConnectionCount > 0 && s.RxBytes > 0 && s.TxBytes > 0 &&
143-
// Ensure that the connection didn't count as a "normal" SSH session.
144-
// This was a special one, so it should be labeled specially in the stats!
145-
s.SessionCountVSCode == 1 &&
146-
// Ensure that connection latency is being counted!
147-
// If it isn't, it's set to -1.
148-
s.ConnectionMedianLatencyMS >= 0
149-
}, testutil.WaitLong, testutil.IntervalFast,
150-
"never saw stats: %+v", s,
151-
)
135+
command := "sh -c 'echo $" + agent.MagicSSHSessionTypeEnvironmentVariable + "'"
136+
expected := ""
137+
if runtime.GOOS == "windows" {
138+
expected = "%" + agent.MagicSSHSessionTypeEnvironmentVariable + "%"
139+
command = "cmd.exe /c echo " + expected
140+
}
141+
output, err := session.Output(command)
142+
require.NoError(t, err)
143+
require.Equal(t, expected, strings.TrimSpace(string(output)))
144+
})
145+
t.Run("Tracks", func(t *testing.T) {
146+
t.Parallel()
147+
if runtime.GOOS == "window" {
148+
t.Skip("Sleeping for infinity doesn't work on Windows")
149+
}
150+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
151+
defer cancel()
152+
//nolint:dogsled
153+
conn, _, stats, _, _ := setupAgent(t, agentsdk.Metadata{}, 0)
154+
sshClient, err := conn.SSHClient(ctx)
155+
require.NoError(t, err)
156+
defer sshClient.Close()
157+
session, err := sshClient.NewSession()
158+
require.NoError(t, err)
159+
session.Setenv(agent.MagicSSHSessionTypeEnvironmentVariable, agent.MagicSSHSessionTypeVSCode)
160+
defer session.Close()
161+
stdin, err := session.StdinPipe()
162+
require.NoError(t, err)
163+
err = session.Shell()
164+
require.NoError(t, err)
165+
var s *agentsdk.Stats
166+
require.Eventuallyf(t, func() bool {
167+
var ok bool
168+
s, ok = <-stats
169+
return ok && s.ConnectionCount > 0 && s.RxBytes > 0 && s.TxBytes > 0 &&
170+
// Ensure that the connection didn't count as a "normal" SSH session.
171+
// This was a special one, so it should be labeled specially in the stats!
172+
s.SessionCountVSCode == 1 &&
173+
// Ensure that connection latency is being counted!
174+
// If it isn't, it's set to -1.
175+
s.ConnectionMedianLatencyMS >= 0
176+
}, testutil.WaitLong, testutil.IntervalFast,
177+
"never saw stats: %+v", s,
178+
)
179+
// The shell will automatically exit if there is no stdin!
180+
_ = stdin.Close()
181+
err = session.Wait()
182+
require.NoError(t, err)
183+
})
152184
}
153185

154186
func TestAgent_SessionExec(t *testing.T) {

‎cli/cliui/provisionerjob.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ type ProvisionerJobOptions struct {
4141
Silent bool
4242
}
4343

44+
type ProvisionerJobError struct {
45+
Message string
46+
Code codersdk.JobErrorCode
47+
}
48+
49+
var _ error = new(ProvisionerJobError)
50+
51+
func (err *ProvisionerJobError) Error() string {
52+
return err.Message
53+
}
54+
4455
// ProvisionerJob renders a provisioner job with interactive cancellation.
4556
func ProvisionerJob(ctx context.Context, writer io.Writer, opts ProvisionerJobOptions) error {
4657
if opts.FetchInterval == 0 {
@@ -181,7 +192,10 @@ func ProvisionerJob(ctx context.Context, writer io.Writer, opts ProvisionerJobOp
181192
return nil
182193
case codersdk.ProvisionerJobFailed:
183194
}
184-
err = xerrors.New(job.Error)
195+
err = &ProvisionerJobError{
196+
Message: job.Error,
197+
Code: job.ErrorCode,
198+
}
185199
jobMutex.Unlock()
186200
flushLogBuffer()
187201
return err

‎cli/server.go

+1-9
Original file line numberDiff line numberDiff line change
@@ -1044,14 +1044,6 @@ flags, and YAML configuration. The precedence is as follows:
10441044
}
10451045
}()
10461046

1047-
hasFirstUser, err := client.HasFirstUser(ctx)
1048-
if err != nil {
1049-
cmd.Println("\nFailed to check for the first user: " + err.Error())
1050-
} else if !hasFirstUser {
1051-
cmd.Println("\nGet started by creating the first user (in a new terminal):")
1052-
cmd.Println(cliui.Styles.Code.Render("coder login " + cfg.AccessURL.String()))
1053-
}
1054-
10551047
cmd.Println("\n==> Logs will stream in below (press ctrl+c to gracefully exit):")
10561048

10571049
// Updates the systemd status from activating to activated.
@@ -1367,7 +1359,7 @@ func printLogo(cmd *cobra.Command) {
13671359
return
13681360
}
13691361

1370-
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s - Software development on your infrastucture\n", cliui.Styles.Bold.Render("Coder "+buildinfo.Version()))
1362+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s - Your Self-Hosted Remote Development Platform\n", cliui.Styles.Bold.Render("Coder "+buildinfo.Version()))
13711363
}
13721364

13731365
func loadCertificates(tlsCertFiles, tlsKeyFiles []string) ([]tls.Certificate, error) {

‎cli/templatecreate.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cli
22

33
import (
4+
"errors"
45
"fmt"
56
"io"
67
"os"
@@ -196,7 +197,8 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers
196197
},
197198
})
198199
if err != nil {
199-
if !provisionerd.IsMissingParameterError(err.Error()) {
200+
var jobErr *cliui.ProvisionerJobError
201+
if errors.As(err, &jobErr) && !provisionerd.IsMissingParameterErrorCode(string(jobErr.Code)) {
200202
return nil, nil, err
201203
}
202204
}
@@ -233,7 +235,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers
233235
}
234236
}
235237

236-
if provisionerd.IsMissingParameterError(version.Job.Error) {
238+
if provisionerd.IsMissingParameterErrorCode(string(version.Job.ErrorCode)) {
237239
valuesBySchemaID := map[string]codersdk.ComputedParameter{}
238240
for _, parameterValue := range parameterValues {
239241
valuesBySchemaID[parameterValue.SchemaID.String()] = parameterValue

0 commit comments

Comments
 (0)