Skip to content

feat: show agent version in UI and CLI #3709

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 5 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion cli/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"cdr.dev/slog/sloggers/sloghuman"
"github.com/coder/coder/agent"
"github.com/coder/coder/agent/reaper"
"github.com/coder/coder/buildinfo"
"github.com/coder/coder/cli/cliflag"
"github.com/coder/coder/codersdk"
"github.com/coder/retry"
Expand Down Expand Up @@ -73,7 +74,12 @@ func workspaceAgent() *cobra.Command {
return nil
}

logger.Info(cmd.Context(), "starting agent", slog.F("url", coderURL), slog.F("auth", auth))
version := buildinfo.Version()
logger.Info(cmd.Context(), "starting agent",
slog.F("url", coderURL),
slog.F("auth", auth),
slog.F("version", version),
)
client := codersdk.New(coderURL)

if pprofEnabled {
Expand Down Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@johnstcn johnstcn Aug 30, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member

@mafredri mafredri Aug 30, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@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?

logger.Error(cmd.Context(), "post agent version: %w", slog.Error(err), slog.F("version", version))
}

closer := agent.New(client.ListenWorkspaceAgent, &agent.Options{
Logger: logger,
EnvironmentVariables: map[string]string{
Expand Down
10 changes: 10 additions & 0 deletions cli/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/coder/coder/cli/clitest"
Expand Down Expand Up @@ -59,6 +60,9 @@ func TestWorkspaceAgent(t *testing.T) {
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
require.NoError(t, err)
if assert.NotEmpty(t, resources) && assert.NotEmpty(t, resources[0].Agents) {
assert.NotEmpty(t, resources[0].Agents[0].Version)
}
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
require.NoError(t, err)
defer dialer.Close()
Expand Down Expand Up @@ -114,6 +118,9 @@ func TestWorkspaceAgent(t *testing.T) {
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
require.NoError(t, err)
if assert.NotEmpty(t, resources) && assert.NotEmpty(t, resources[0].Agents) {
assert.NotEmpty(t, resources[0].Agents[0].Version)
}
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
require.NoError(t, err)
defer dialer.Close()
Expand Down Expand Up @@ -169,6 +176,9 @@ func TestWorkspaceAgent(t *testing.T) {
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
require.NoError(t, err)
if assert.NotEmpty(t, resources) && assert.NotEmpty(t, resources[0].Agents) {
assert.NotEmpty(t, resources[0].Agents[0].Version)
}
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
require.NoError(t, err)
defer dialer.Close()
Expand Down
51 changes: 38 additions & 13 deletions cli/cliui/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strconv"

"github.com/jedib0t/go-pretty/v6/table"
"golang.org/x/mod/semver"

"github.com/coder/coder/coderd/database"

Expand All @@ -18,6 +19,7 @@ type WorkspaceResourcesOptions struct {
HideAgentState bool
HideAccess bool
Title string
ServerVersion string
}

// WorkspaceResources displays the connection status and tree-view of provided resources.
Expand Down Expand Up @@ -48,6 +50,7 @@ func WorkspaceResources(writer io.Writer, resources []codersdk.WorkspaceResource
row := table.Row{"Resource"}
if !options.HideAgentState {
row = append(row, "Status")
row = append(row, "Version")
}
if !options.HideAccess {
row = append(row, "Access")
Expand Down Expand Up @@ -91,21 +94,12 @@ func WorkspaceResources(writer io.Writer, resources []codersdk.WorkspaceResource
}
if !options.HideAgentState {
var agentStatus string
var agentVersion string
if !options.HideAgentState {
switch agent.Status {
case codersdk.WorkspaceAgentConnecting:
since := database.Now().Sub(agent.CreatedAt)
agentStatus = Styles.Warn.Render("⦾ connecting") + " " +
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]")
case codersdk.WorkspaceAgentDisconnected:
since := database.Now().Sub(*agent.DisconnectedAt)
agentStatus = Styles.Error.Render("⦾ disconnected") + " " +
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]")
case codersdk.WorkspaceAgentConnected:
agentStatus = Styles.Keyword.Render("⦿ connected")
}
agentStatus = renderAgentStatus(agent)
agentVersion = renderAgentVersion(agent.Version, options.ServerVersion)
}
row = append(row, agentStatus)
row = append(row, agentStatus, agentVersion)
}
if !options.HideAccess {
sshCommand := "coder ssh " + options.WorkspaceName
Expand All @@ -122,3 +116,34 @@ func WorkspaceResources(writer io.Writer, resources []codersdk.WorkspaceResource
_, err := fmt.Fprintln(writer, tableWriter.Render())
return err
}

func renderAgentStatus(agent codersdk.WorkspaceAgent) string {
switch agent.Status {
case codersdk.WorkspaceAgentConnecting:
since := database.Now().Sub(agent.CreatedAt)
return Styles.Warn.Render("⦾ connecting") + " " +
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]")
case codersdk.WorkspaceAgentDisconnected:
since := database.Now().Sub(*agent.DisconnectedAt)
return Styles.Error.Render("⦾ disconnected") + " " +
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]")
case codersdk.WorkspaceAgentConnected:
return Styles.Keyword.Render("⦿ connected")
default:
return Styles.Warn.Render("○ unknown")
}
}

func renderAgentVersion(agentVersion, serverVersion string) string {
if agentVersion == "" {
agentVersion = "(unknown)"
}
if !semver.IsValid(serverVersion) || !semver.IsValid(agentVersion) {
return Styles.Placeholder.Render(agentVersion)
}
outdated := semver.Compare(agentVersion, serverVersion) < 0
if outdated {
return Styles.Warn.Render(agentVersion + " (outdated)")
}
return Styles.Keyword.Render(agentVersion)
Copy link
Member

Choose a reason for hiding this comment

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

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.)

Copy link
Member Author

Choose a reason for hiding this comment

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

That'll be apparent by comparing the version shown here with the version shown in the footer.

}
50 changes: 50 additions & 0 deletions cli/cliui/resources_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package cliui

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestRenderAgentVersion(t *testing.T) {
t.Parallel()
testCases := []struct {
name string
agentVersion string
serverVersion string
expected string
}{
{
name: "OK",
agentVersion: "v1.2.3",
serverVersion: "v1.2.3",
expected: "v1.2.3",
},
{
name: "Outdated",
agentVersion: "v1.2.3",
serverVersion: "v1.2.4",
expected: "v1.2.3 (outdated)",
},
{
name: "AgentUnknown",
agentVersion: "",
serverVersion: "v1.2.4",
expected: "(unknown)",
},
{
name: "ServerUnknown",
agentVersion: "v1.2.3",
serverVersion: "",
expected: "v1.2.3",
},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
actual := renderAgentVersion(testCase.agentVersion, testCase.serverVersion)
assert.Equal(t, testCase.expected, actual)
})
}
}
5 changes: 5 additions & 0 deletions cli/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ func show() *cobra.Command {
if err != nil {
return err
}
buildInfo, err := client.BuildInfo(cmd.Context())
if err != nil {
return xerrors.Errorf("get server version: %w", err)
}
workspace, err := namedWorkspace(cmd, client, args[0])
if err != nil {
return xerrors.Errorf("get workspace: %w", err)
Expand All @@ -28,6 +32,7 @@ func show() *cobra.Command {
}
return cliui.WorkspaceResources(cmd.OutOrStdout(), resources, cliui.WorkspaceResourcesOptions{
WorkspaceName: workspace.Name,
ServerVersion: buildInfo.Version,
})
},
}
Expand Down
1 change: 1 addition & 0 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ func New(options *Options) *API {
r.Route("/me", func(r chi.Router) {
r.Use(httpmw.ExtractWorkspaceAgent(options.Database))
r.Get("/metadata", api.workspaceAgentMetadata)
r.Post("/version", api.postWorkspaceAgentVersion)
r.Get("/listen", api.workspaceAgentListen)
r.Get("/gitsshkey", api.agentGitSSHKey)
r.Get("/turn", api.workspaceAgentTurn)
Expand Down
1 change: 1 addition & 0 deletions coderd/coderdtest/authtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
"GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true},
"GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true},
"GET:/api/v2/workspaceagents/me/derp": {NoAuthorize: true},
"POST:/api/v2/workspaceagents/me/version": {NoAuthorize: true},
"GET:/api/v2/workspaceagents/me/wireguardlisten": {NoAuthorize: true},
"POST:/api/v2/workspaceagents/me/keys": {NoAuthorize: true},
"GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true},
Expand Down
16 changes: 16 additions & 0 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,22 @@ func (q *fakeQuerier) UpdateWorkspaceAgentKeysByID(_ context.Context, arg databa
return sql.ErrNoRows
}

func (q *fakeQuerier) UpdateWorkspaceAgentVersionByID(_ context.Context, arg database.UpdateWorkspaceAgentVersionByIDParams) error {
q.mutex.Lock()
defer q.mutex.Unlock()

for index, agent := range q.provisionerJobAgents {
if agent.ID != arg.ID {
continue
}

agent.Version = arg.Version
q.provisionerJobAgents[index] = agent
return nil
}
return sql.ErrNoRows
}

func (q *fakeQuerier) UpdateProvisionerJobByID(_ context.Context, arg database.UpdateProvisionerJobByIDParams) error {
q.mutex.Lock()
defer q.mutex.Unlock()
Expand Down
5 changes: 4 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE ONLY workspace_agents DROP COLUMN version;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE ONLY workspace_agents ADD COLUMN version text DEFAULT ''::text NOT NULL;
COMMENT ON COLUMN workspace_agents.version IS 'Version tracks the version of the currently running workspace agent. Workspace agents register their version upon start.';
2 changes: 2 additions & 0 deletions coderd/database/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions coderd/database/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading