feat: show agent version in UI and CLI #3709
Conversation
bf1436e to
e7e8b97
Compare
e7e8b97 to
bef419c
Compare
| if outdated { | ||
| return Styles.Warn.Render(agentVersion + " (outdated)") | ||
| } | ||
| return Styles.Keyword.Render(agentVersion) |
There was a problem hiding this comment.
In theory, the agent could be newer than the server too. Should we show this somehow? (This will be more relevant if users are using a server version without bundled agent binaries #3593, but could also happen on server downgrades.)
There was a problem hiding this comment.
That'll be apparent by comparing the version shown here with the version shown in the footer.
coderd/database/migrations/000039_add_workspace_agent_version.up.sql
Outdated
Show resolved
Hide resolved
| @@ -172,6 +178,10 @@ func workspaceAgent() *cobra.Command { | |||
| return xerrors.Errorf("add executable to $PATH: %w", err) | |||
| } | |||
|
|
|||
| if err := client.PostWorkspaceAgentVersion(cmd.Context(), version); err != nil { | |||
There was a problem hiding this comment.
Suggestion: We could consider discarding this API endpoint and make the version a part of the connect-to-server process. This way the version would only be updated on successful connects. I don't really see a need to be able to post an update for the version any other time during an agents lifecycle.
There was a problem hiding this comment.
UploadWorkspaceAgentKeys uses an endpoint POST /api/v2/workspaceagents/me/keys as well and I believe we only upload keys once per agent. This feels like a separate refactor.
There was a problem hiding this comment.
I think that scenario (UploadWorkspaceAgentKeys) is slightly different since it's a pre-requisite for establishing the connection over WireGuard (I could be wrong, though). But I won't push for this change so feel free to omit.
My motivation behind the suggestion was simply to ensure that the version tracked in the DB is consistent with "the last version of the agent that connected to the server". With the pre-post, in essence, it becomes "the last version that tried to talk to the server", but connection could've failed.
There was a problem hiding this comment.
@ammario has a PR open for metrics to be pushed to coderd. It could be done as apart of that process. Version reporting is a sort of metric?
| @@ -65,6 +67,8 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({ | |||
| }) => { | |||
| const styles = useStyles() | |||
| const navigate = useNavigate() | |||
| const xServices = useContext(XServiceContext) | |||
| const [buildInfoState] = useActor(xServices.buildInfoXService) | |||
There was a problem hiding this comment.
Saw your comment about Storybook and XState. That's why we have XState only in the pages right now. There must be a way to set up Storybook to get around it, but we haven't put the time in yet (just adding the XServiceProvider to the Storybook decorator doesn't get us all the way there), and I generally like that it makes us separate "connected" components from visual ones. The problem is in the workspace page, where there is so much that we end up with a deep component tree. Our solution so far has been to send things from XState down from the WorkspacePage as props but make objects of some bundles of props to make it less messy.
There was a problem hiding this comment.
Gotcha, that makes sense. I'll go ahead with that approach for now!
There was a problem hiding this comment.
Oh wait, I was doing an import wrong - I think just adding the provider does work! If you want to try, the place to add it is .storybook/preview.js.
|
What do you think about explicitly stating that there's a version mismatch between the agent and server? Along with a tooltip of how to resolve that (stopping and starting in our case)? |
Added a tooltip to that effect. |
This PR adds the ability for agents to set their version upon start.
This is then reported in the UI and CLI.
Fixes #2083
Looks like this:
TODO list: