Skip to content

Commit 7fc1849

Browse files
JoshVeeJoshVee
authored
Merge branch 'coder:main' into joshvee-ssh-header-command
2 parents c5e0ec1 + 3b50530 commit 7fc1849

File tree

105 files changed

+1342
-1364
lines changed

Some content is hidden

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

105 files changed

+1342
-1364
lines changed

‎.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ jobs:
136136
137137
# Check for any typos
138138
- name: Check for typos
139-
uses: crate-ci/typos@v1.16.19
139+
uses: crate-ci/typos@v1.16.20
140140
with:
141141
config: .github/workflows/typos.toml
142142

‎.github/workflows/stale.yaml

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,15 @@ jobs:
5252
with:
5353
token: ${{ github.token }}
5454
repository: ${{ github.repository }}
55-
retain_days: 1
56-
keep_minimum_runs: 1
55+
retain_days: 30
56+
keep_minimum_runs: 30
5757
delete_workflow_pattern: pr-cleanup.yaml
5858

5959
- name: Delete PR Deploy workflow skipped runs
6060
uses: Mattraks/delete-workflow-runs@v2
6161
with:
6262
token: ${{ github.token }}
6363
repository: ${{ github.repository }}
64-
retain_days: 0
65-
keep_minimum_runs: 0
66-
delete_run_by_conclusion_pattern: skipped
64+
retain_days: 30
65+
keep_minimum_runs: 30
6766
delete_workflow_pattern: pr-deploy.yaml

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ curl -L https://coder.com/install.sh | sh
7070

7171
You can run the install script with `--dry-run` to see the commands that will be used to install without executing them. You can modify the installation process by including flags. Run the install script with `--help` for reference.
7272

73-
> See [install](docs/install) for additional methods.
73+
> See [install](https://coder.com/docs/v2/latest/install) for additional methods.
7474
7575
Once installed, you can start a production deployment<sup>1</sup> with a single command:
7676

‎agent/agent.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,14 @@ func (a *agent) reportMetadataLoop(ctx context.Context) {
536536
continue
537537
case <-report:
538538
if len(updatedMetadata) > 0 {
539+
select {
540+
case <-reportSemaphore:
541+
default:
542+
// If there's already a report in flight, don't send
543+
// another one, wait for next tick instead.
544+
continue
545+
}
546+
539547
metadata := make([]agentsdk.Metadata, 0, len(updatedMetadata))
540548
for key, result := range updatedMetadata {
541549
metadata = append(metadata, agentsdk.Metadata{
@@ -545,14 +553,6 @@ func (a *agent) reportMetadataLoop(ctx context.Context) {
545553
delete(updatedMetadata, key)
546554
}
547555

548-
select {
549-
case <-reportSemaphore:
550-
default:
551-
// If there's already a report in flight, don't send
552-
// another one, wait for next tick instead.
553-
continue
554-
}
555-
556556
go func() {
557557
ctx, cancel := context.WithTimeout(ctx, reportTimeout)
558558
defer func() {
@@ -743,7 +743,7 @@ func (a *agent) run(ctx context.Context) error {
743743
return script.RunOnStart
744744
})
745745
if err != nil {
746-
a.logger.Warn(ctx, "startup script failed", slog.Error(err))
746+
a.logger.Warn(ctx, "startup script(s) failed", slog.Error(err))
747747
if errors.Is(err, agentscripts.ErrTimeout) {
748748
a.setLifecycle(ctx, codersdk.WorkspaceAgentLifecycleStartTimeout)
749749
} else {
@@ -1465,6 +1465,7 @@ func (a *agent) Close() error {
14651465
return script.RunOnStop
14661466
})
14671467
if err != nil {
1468+
a.logger.Warn(ctx, "shutdown script(s) failed", slog.Error(err))
14681469
if errors.Is(err, agentscripts.ErrTimeout) {
14691470
lifecycleState = codersdk.WorkspaceAgentLifecycleShutdownTimeout
14701471
} else {

‎agent/agent_test.go

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,13 @@ func TestAgent_Session_TTY_MOTD(t *testing.T) {
350350
unexpected: []string{},
351351
},
352352
{
353-
name: "Trim",
354-
manifest: agentsdk.Manifest{},
353+
name: "Trim",
354+
// Enable motd since it will be printed after the banner,
355+
// this ensures that we can test for an exact mount of
356+
// newlines.
357+
manifest: agentsdk.Manifest{
358+
MOTDFile: name,
359+
},
355360
banner: codersdk.ServiceBannerConfig{
356361
Enabled: true,
357362
Message: "\n\n\n\n\n\nbanner\n\n\n\n\n\n",
@@ -375,6 +380,7 @@ func TestAgent_Session_TTY_MOTD(t *testing.T) {
375380
}
376381
}
377382

383+
//nolint:tparallel // Sub tests need to run sequentially.
378384
func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
379385
t.Parallel()
380386
if runtime.GOOS == "windows" {
@@ -434,33 +440,38 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
434440
}
435441
//nolint:dogsled // Allow the blank identifiers.
436442
conn, client, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, setSBInterval)
437-
for _, test := range tests {
443+
444+
sshClient, err := conn.SSHClient(ctx)
445+
require.NoError(t, err)
446+
t.Cleanup(func() {
447+
_ = sshClient.Close()
448+
})
449+
450+
//nolint:paralleltest // These tests need to swap the banner func.
451+
for i, test := range tests {
438452
test := test
439-
// Set new banner func and wait for the agent to call it to update the
440-
// banner.
441-
ready := make(chan struct{}, 2)
442-
client.SetServiceBannerFunc(func() (codersdk.ServiceBannerConfig, error) {
443-
select {
444-
case ready <- struct{}{}:
445-
default:
446-
}
447-
return test.banner, nil
448-
})
449-
<-ready
450-
<-ready // Wait for two updates to ensure the value has propagated.
453+
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
454+
// Set new banner func and wait for the agent to call it to update the
455+
// banner.
456+
ready := make(chan struct{}, 2)
457+
client.SetServiceBannerFunc(func() (codersdk.ServiceBannerConfig, error) {
458+
select {
459+
case ready <- struct{}{}:
460+
default:
461+
}
462+
return test.banner, nil
463+
})
464+
<-ready
465+
<-ready // Wait for two updates to ensure the value has propagated.
451466

452-
sshClient, err := conn.SSHClient(ctx)
453-
require.NoError(t, err)
454-
t.Cleanup(func() {
455-
_ = sshClient.Close()
456-
})
457-
session, err := sshClient.NewSession()
458-
require.NoError(t, err)
459-
t.Cleanup(func() {
460-
_ = session.Close()
461-
})
467+
session, err := sshClient.NewSession()
468+
require.NoError(t, err)
469+
t.Cleanup(func() {
470+
_ = session.Close()
471+
})
462472

463-
testSessionOutput(t, session, test.expected, test.unexpected, nil)
473+
testSessionOutput(t, session, test.expected, test.unexpected, nil)
474+
})
464475
}
465476
}
466477

‎agent/agentscripts/agentscripts.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ import (
2727
var (
2828
// ErrTimeout is returned when a script times out.
2929
ErrTimeout = xerrors.New("script timed out")
30+
// ErrOutputPipesOpen is returned when a script exits leaving the output
31+
// pipe(s) (stdout, stderr) open. This happens because we set WaitDelay on
32+
// the command, which gives us two things:
33+
//
34+
// 1. The ability to ensure that a script exits (this is important for e.g.
35+
// blocking login, and avoiding doing so indefinitely)
36+
// 2. Improved command cancellation on timeout
37+
ErrOutputPipesOpen = xerrors.New("script exited without closing output pipes")
3038

3139
parser = cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional)
3240
)
@@ -97,7 +105,15 @@ func (r *Runner) Init(scripts []codersdk.WorkspaceAgentScript) error {
97105
// StartCron starts the cron scheduler.
98106
// This is done async to allow for the caller to execute scripts prior.
99107
func (r *Runner) StartCron() {
100-
r.cron.Start()
108+
// cron.Start() and cron.Stop() does not guarantee that the cron goroutine
109+
// has exited by the time the `cron.Stop()` context returns, so we need to
110+
// track it manually.
111+
err := r.trackCommandGoroutine(func() {
112+
r.cron.Run()
113+
})
114+
if err != nil {
115+
r.Logger.Warn(context.Background(), "start cron failed", slog.Error(err))
116+
}
101117
}
102118

103119
// Execute runs a set of scripts according to a filter.
@@ -240,7 +256,22 @@ func (r *Runner) run(ctx context.Context, script codersdk.WorkspaceAgentScript)
240256
err = cmdCtx.Err()
241257
case err = <-cmdDone:
242258
}
243-
if errors.Is(err, context.DeadlineExceeded) {
259+
switch {
260+
case errors.Is(err, exec.ErrWaitDelay):
261+
err = ErrOutputPipesOpen
262+
message := fmt.Sprintf("script exited successfully, but output pipes were not closed after %s", cmd.WaitDelay)
263+
details := fmt.Sprint(
264+
"This usually means a child process was started with references to stdout or stderr. As a result, this " +
265+
"process may now have been terminated. Consider redirecting the output or using a separate " +
266+
"\"coder_script\" for the process, see " +
267+
"https://coder.com/docs/v2/latest/templates/troubleshooting#startup-script-issues for more information.",
268+
)
269+
// Inform the user by propagating the message via log writers.
270+
_, _ = fmt.Fprintf(cmd.Stderr, "WARNING: %s. %s\n", message, details)
271+
// Also log to agent logs for ease of debugging.
272+
r.Logger.Warn(ctx, message, slog.F("details", details), slog.Error(err))
273+
274+
case errors.Is(err, context.DeadlineExceeded):
244275
err = ErrTimeout
245276
}
246277
return err
@@ -254,7 +285,7 @@ func (r *Runner) Close() error {
254285
}
255286
close(r.closed)
256287
r.cronCtxCancel()
257-
r.cron.Stop()
288+
<-r.cron.Stop().Done()
258289
r.cmdCloseWait.Wait()
259290
return nil
260291
}

‎agent/reconnectingpty/reconnectingpty.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ func (s *ptyState) waitForStateOrContext(ctx context.Context, state State) (Stat
196196
// until EOF or an error writing to ptty or reading from conn.
197197
func readConnLoop(ctx context.Context, conn net.Conn, ptty pty.PTYCmd, metrics *prometheus.CounterVec, logger slog.Logger) {
198198
decoder := json.NewDecoder(conn)
199-
var req codersdk.ReconnectingPTYRequest
200199
for {
200+
var req codersdk.ReconnectingPTYRequest
201201
err := decoder.Decode(&req)
202202
if xerrors.Is(err, io.EOF) {
203203
return

‎cli/dotfiles.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
func (r *RootCmd) dotfiles() *clibase.Cmd {
2323
var symlinkDir string
2424
var gitbranch string
25+
var dotfilesRepoDir string
2526

2627
cmd := &clibase.Cmd{
2728
Use: "dotfiles <git_repo_url>",
@@ -35,11 +36,10 @@ func (r *RootCmd) dotfiles() *clibase.Cmd {
3536
),
3637
Handler: func(inv *clibase.Invocation) error {
3738
var (
38-
dotfilesRepoDir = "dotfiles"
39-
gitRepo = inv.Args[0]
40-
cfg = r.createConfig()
41-
cfgDir = string(cfg)
42-
dotfilesDir = filepath.Join(cfgDir, dotfilesRepoDir)
39+
gitRepo = inv.Args[0]
40+
cfg = r.createConfig()
41+
cfgDir = string(cfg)
42+
dotfilesDir = filepath.Join(cfgDir, dotfilesRepoDir)
4343
// This follows the same pattern outlined by others in the market:
4444
// https://github.com/coder/coder/pull/1696#issue-1245742312
4545
installScriptSet = []string{
@@ -290,6 +290,13 @@ func (r *RootCmd) dotfiles() *clibase.Cmd {
290290
"If empty, will default to cloning the default branch or using the existing branch in the cloned repo on disk.",
291291
Value: clibase.StringOf(&gitbranch),
292292
},
293+
{
294+
Flag: "repo-dir",
295+
Default: "dotfiles",
296+
Env: "CODER_DOTFILES_REPO_DIR",
297+
Description: "Specifies the directory for the dotfiles repository, relative to global config directory.",
298+
Value: clibase.StringOf(&dotfilesRepoDir),
299+
},
293300
cliui.SkipPromptOption(),
294301
}
295302
return cmd

‎cli/dotfiles_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,68 @@ func TestDotfiles(t *testing.T) {
5050
require.NoError(t, err)
5151
require.Equal(t, string(b), "wow")
5252
})
53+
t.Run("SwitchRepoDir", func(t *testing.T) {
54+
t.Parallel()
55+
_, root := clitest.New(t)
56+
testRepo := testGitRepo(t, root)
57+
58+
// nolint:gosec
59+
err := os.WriteFile(filepath.Join(testRepo, ".bashrc"), []byte("wow"), 0o750)
60+
require.NoError(t, err)
61+
62+
c := exec.Command("git", "add", ".bashrc")
63+
c.Dir = testRepo
64+
err = c.Run()
65+
require.NoError(t, err)
66+
67+
c = exec.Command("git", "commit", "-m", `"add .bashrc"`)
68+
c.Dir = testRepo
69+
out, err := c.CombinedOutput()
70+
require.NoError(t, err, string(out))
71+
72+
inv, _ := clitest.New(t, "dotfiles", "--global-config", string(root), "--symlink-dir", string(root), "--repo-dir", "testrepo", "-y", testRepo)
73+
err = inv.Run()
74+
require.NoError(t, err)
75+
76+
b, err := os.ReadFile(filepath.Join(string(root), ".bashrc"))
77+
require.NoError(t, err)
78+
require.Equal(t, string(b), "wow")
79+
80+
stat, staterr := os.Stat(filepath.Join(string(root), "testrepo"))
81+
require.NoError(t, staterr)
82+
require.True(t, stat.IsDir())
83+
})
84+
t.Run("SwitchRepoDirRelative", func(t *testing.T) {
85+
t.Parallel()
86+
_, root := clitest.New(t)
87+
testRepo := testGitRepo(t, root)
88+
89+
// nolint:gosec
90+
err := os.WriteFile(filepath.Join(testRepo, ".bashrc"), []byte("wow"), 0o750)
91+
require.NoError(t, err)
92+
93+
c := exec.Command("git", "add", ".bashrc")
94+
c.Dir = testRepo
95+
err = c.Run()
96+
require.NoError(t, err)
97+
98+
c = exec.Command("git", "commit", "-m", `"add .bashrc"`)
99+
c.Dir = testRepo
100+
out, err := c.CombinedOutput()
101+
require.NoError(t, err, string(out))
102+
103+
inv, _ := clitest.New(t, "dotfiles", "--global-config", string(root), "--symlink-dir", string(root), "--repo-dir", "./relrepo", "-y", testRepo)
104+
err = inv.Run()
105+
require.NoError(t, err)
106+
107+
b, err := os.ReadFile(filepath.Join(string(root), ".bashrc"))
108+
require.NoError(t, err)
109+
require.Equal(t, string(b), "wow")
110+
111+
stat, staterr := os.Stat(filepath.Join(string(root), "relrepo"))
112+
require.NoError(t, staterr)
113+
require.True(t, stat.IsDir())
114+
})
53115
t.Run("InstallScript", func(t *testing.T) {
54116
t.Parallel()
55117
if runtime.GOOS == "windows" {

‎cli/server.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,14 @@ func WriteConfigMW(cfg *codersdk.DeploymentValues) clibase.MiddlewareFunc {
12061206
// isLocalURL returns true if the hostname of the provided URL appears to
12071207
// resolve to a loopback address.
12081208
func IsLocalURL(ctx context.Context, u *url.URL) (bool, error) {
1209+
// In tests, we commonly use "example.com" or "google.com", which
1210+
// are not loopback, so avoid the DNS lookup to avoid flakes.
1211+
if flag.Lookup("test.v") != nil {
1212+
if u.Hostname() == "example.com" || u.Hostname() == "google.com" {
1213+
return false, nil
1214+
}
1215+
}
1216+
12091217
resolver := &net.Resolver{}
12101218
ips, err := resolver.LookupIPAddr(ctx, u.Hostname())
12111219
if err != nil {

‎cli/templatecreate.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,18 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
5050
isTemplateSchedulingOptionsSet := failureTTL != 0 || inactivityTTL != 0 || maxTTL != 0
5151

5252
if isTemplateSchedulingOptionsSet || requireActiveVersion {
53+
if failureTTL != 0 || inactivityTTL != 0 {
54+
// This call can be removed when workspace_actions is no longer experimental
55+
experiments, exErr := client.Experiments(inv.Context())
56+
if exErr != nil {
57+
return xerrors.Errorf("get experiments: %w", exErr)
58+
}
59+
60+
if !experiments.Enabled(codersdk.ExperimentWorkspaceActions) {
61+
return xerrors.Errorf("--failure-ttl and --inactivity-ttl are experimental features. Use the workspace_actions CODER_EXPERIMENTS flag to set these configuration values.")
62+
}
63+
}
64+
5365
entitlements, err := client.Entitlements(inv.Context())
5466
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusNotFound {
5567
return xerrors.Errorf("your deployment appears to be an AGPL deployment, so you cannot set enterprise-only flags")
@@ -59,7 +71,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
5971

6072
if isTemplateSchedulingOptionsSet {
6173
if !entitlements.Features[codersdk.FeatureAdvancedTemplateScheduling].Enabled {
62-
return xerrors.Errorf("your license is not entitled to use advanced template scheduling, so you cannot set --failure-ttl or --inactivityTTL")
74+
return xerrors.Errorf("your license is not entitled to use advanced template scheduling, so you cannot set --failure-ttl, --inactivity-ttl, or --max-ttl")
6375
}
6476
}
6577

0 commit comments

Comments
 (0)