Skip to content

feat: Use environment variables and startup script in agent #1147

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.

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
Apr 25, 2022
Merged

Conversation

kylecarbs
Copy link
Member

These values were ignored. Environment variables are applied to
new sessions and are refreshed on reconnect. This is cool because
a workspace could be updated with new environment variables without
requiring a complete start/stop.

The startup script is only run once regardless of changes, which
feels like the expected behavior.

@kylecarbs kylecarbs requested a review from coadler April 25, 2022 17:03
@kylecarbs kylecarbs self-assigned this Apr 25, 2022
@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #1147 (c665f8d) into main (09405dd) will decrease coverage by 0.01%.
The diff coverage is 61.22%.

@@            Coverage Diff             @@
##             main    #1147      +/-   ##
==========================================
- Coverage   66.31%   66.30%   -0.02%     
==========================================
  Files         261      257       -4     
  Lines       16251    16141     -110     
  Branches      156      156              
==========================================
- Hits        10777    10702      -75     
+ Misses       4362     4337      -25     
+ Partials     1112     1102      -10     
Flag Coverage Δ
unittest-go-macos-latest 53.73% <55.10%> (+0.05%) ⬆️
unittest-go-postgres- 65.69% <55.10%> (-0.02%) ⬇️
unittest-go-ubuntu-latest 56.34% <61.22%> (+0.05%) ⬆️
unittest-go-windows-2022 ?
unittest-js 67.28% <ø> (ø)
Impacted Files Coverage Δ
cli/gitssh.go 44.44% <ø> (ø)
coderd/workspaceagents.go 58.92% <40.00%> (-0.71%) ⬇️
codersdk/workspaceagents.go 53.35% <45.00%> (+1.21%) ⬆️
agent/agent.go 65.07% <68.18%> (+0.65%) ⬆️
coderd/coderd.go 97.48% <100.00%> (+0.01%) ⬆️
cli/configssh.go 61.41% <0.00%> (-7.09%) ⬇️
pty/ptytest/ptytest.go 86.95% <0.00%> (-4.35%) ⬇️
cli/templateinit.go 58.62% <0.00%> (-3.45%) ⬇️
provisioner/echo/serve.go 56.80% <0.00%> (-2.41%) ⬇️
cli/cliui/provisionerjob.go 76.42% <0.00%> (-2.15%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09405dd...c665f8d. Read the comment docs.

@kylecarbs kylecarbs force-pushed the startup branch 3 times, most recently from e54c0bd to 1155d45 Compare April 25, 2022 17:39
@kylecarbs kylecarbs requested a review from johnstcn April 25, 2022 17:49
agent/agent.go Outdated
Comment on lines 97 to 99
if !a.startupScript.Load() {
// The startup script has not ran yet!
a.startupScript.Store(true)
Copy link
Contributor

@coadler coadler Apr 25, 2022

Choose a reason for hiding this comment

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

This is technically racey right? I think this needs to be a CAS (compare and swap).

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct. I shall fix

These values were ignored. Environment variables are applied to
new sessions, and are refreshed on reconnect. This is cool because
a workspace could be updated with new environment variables without
requiring a complete start/stop.

The startup script is only ran once regardless of changes, which
feels like the expected behavior.
return false
}
if runtime.GOOS == "windows" {
// Windows uses UTF16! 🪟🪟🪟
Copy link
Contributor

Choose a reason for hiding this comment

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

👻

Copy link
Member Author

Choose a reason for hiding this comment

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

spooky is right

@kylecarbs kylecarbs enabled auto-merge (squash) April 25, 2022 18:20
@kylecarbs kylecarbs merged commit a2dd618 into main Apr 25, 2022
@kylecarbs kylecarbs deleted the startup branch April 25, 2022 18:30
kylecarbs added a commit that referenced this pull request Jun 10, 2022
These values were ignored. Environment variables are applied to
new sessions, and are refreshed on reconnect. This is cool because
a workspace could be updated with new environment variables without
requiring a complete start/stop.

The startup script is only ran once regardless of changes, which
feels like the expected behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants