Skip to content

fix: Use app IDs instead of the display name to report health #4944

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.

Sign up for GitHub

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
Nov 7, 2022
Merged

fix: Use app IDs instead of the display name to report health #4944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0b25cb
fix: Use app slugs instead of the display name to report health
kylecarbs Nov 7, 2022
a597d89
Update agent/apphealth.go
kylecarbs Nov 7, 2022
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
23 changes: 12 additions & 11 deletions agent/apphealth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"golang.org/x/xerrors"
"github.com/google/uuid"

"cdr.dev/slog"
"github.com/coder/coder/codersdk"
Expand All @@ -31,9 +32,9 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
}

hasHealthchecksEnabled := false
health := make(map[string]codersdk.WorkspaceAppHealth, 0)
health := make(map[uuid.UUID]codersdk.WorkspaceAppHealth, 0)
for _, app := range apps {
health[app.DisplayName] = app.Health
health[app.ID] = app.Health
if !hasHealthchecksEnabled && app.Health != codersdk.WorkspaceAppHealthDisabled {
hasHealthchecksEnabled = true
}
Expand All @@ -46,7 +47,7 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace

// run a ticker for each app health check.
var mu sync.RWMutex
failures := make(map[string]int, 0)
failures := make(map[uuid.UUID]int, 0)
for _, nextApp := range apps {
if !shouldStartTicker(nextApp) {
continue
Expand Down Expand Up @@ -85,21 +86,21 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
}()
if err != nil {
mu.Lock()
if failures[app.DisplayName] < int(app.Healthcheck.Threshold) {
if failures[app.ID] < int(app.Healthcheck.Threshold) {
// increment the failure count and keep status the same.
// we will change it when we hit the threshold.
failures[app.DisplayName]++
failures[app.ID]++
} else {
// set to unhealthy if we hit the failure threshold.
// we stop incrementing at the threshold to prevent the failure value from increasing forever.
health[app.DisplayName] = codersdk.WorkspaceAppHealthUnhealthy
health[app.ID] = codersdk.WorkspaceAppHealthUnhealthy
}
mu.Unlock()
} else {
mu.Lock()
// we only need one successful health check to be considered healthy.
health[app.DisplayName] = codersdk.WorkspaceAppHealthHealthy
failures[app.DisplayName] = 0
health[app.ID] = codersdk.WorkspaceAppHealthHealthy
failures[app.ID] = 0
mu.Unlock()
}

Expand Down Expand Up @@ -155,7 +156,7 @@ func shouldStartTicker(app codersdk.WorkspaceApp) bool {
return app.Healthcheck.URL != "" && app.Healthcheck.Interval > 0 && app.Healthcheck.Threshold > 0
}

func healthChanged(old map[string]codersdk.WorkspaceAppHealth, new map[string]codersdk.WorkspaceAppHealth) bool {
func healthChanged(old map[uuid.UUID]codersdk.WorkspaceAppHealth, new map[uuid.UUID]codersdk.WorkspaceAppHealth) bool {
for name, newValue := range new {
oldValue, found := old[name]
if !found {
Expand All @@ -169,8 +170,8 @@ func healthChanged(old map[string]codersdk.WorkspaceAppHealth, new map[string]co
return false
}

func copyHealth(h1 map[string]codersdk.WorkspaceAppHealth) map[string]codersdk.WorkspaceAppHealth {
h2 := make(map[string]codersdk.WorkspaceAppHealth, 0)
func copyHealth(h1 map[uuid.UUID]codersdk.WorkspaceAppHealth) map[uuid.UUID]codersdk.WorkspaceAppHealth {
h2 := make(map[uuid.UUID]codersdk.WorkspaceAppHealth, 0)
for k, v := range h1 {
h2[k] = v
}
Expand Down
14 changes: 7 additions & 7 deletions agent/apphealth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ func TestAppHealth(t *testing.T) {
defer cancel()
apps := []codersdk.WorkspaceApp{
{
DisplayName: "app1",
Slug: "app1",
Healthcheck: codersdk.Healthcheck{},
Health: codersdk.WorkspaceAppHealthDisabled,
},
{
DisplayName: "app2",
Slug: "app2",
Healthcheck: codersdk.Healthcheck{
// URL: We don't set the URL for this test because the setup will
// create a httptest server for us and set it for us.
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestAppHealth(t *testing.T) {
defer cancel()
apps := []codersdk.WorkspaceApp{
{
DisplayName: "app2",
Slug: "app2",
Healthcheck: codersdk.Healthcheck{
// URL: We don't set the URL for this test because the setup will
// create a httptest server for us and set it for us.
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestAppHealth(t *testing.T) {
defer cancel()
apps := []codersdk.WorkspaceApp{
{
DisplayName: "app2",
Slug: "app2",
Healthcheck: codersdk.Healthcheck{
// URL: We don't set the URL for this test because the setup will
// create a httptest server for us and set it for us.
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestAppHealth(t *testing.T) {
defer cancel()
apps := []codersdk.WorkspaceApp{
{
DisplayName: "app2",
Slug: "app2",
Healthcheck: codersdk.Healthcheck{
// URL: We don't set the URL for this test because the setup will
// create a httptest server for us and set it for us.
Expand Down Expand Up @@ -185,9 +185,9 @@ func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.Workspa
}
postWorkspaceAgentAppHealth := func(_ context.Context, req codersdk.PostWorkspaceAppHealthsRequest) error {
mu.Lock()
for name, health := range req.Healths {
for id, health := range req.Healths {
for i, app := range apps {
if app.DisplayName != name {
if app.ID != id {
continue
}
app.Health = health
Expand Down
8 changes: 4 additions & 4 deletions coderd/workspaceagents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,10 +913,10 @@ func (api *API) postWorkspaceAppHealth(rw http.ResponseWriter, r *http.Request)
}

var newApps []database.WorkspaceApp
for name, newHealth := range req.Healths {
for id, newHealth := range req.Healths {
old := func() *database.WorkspaceApp {
for _, app := range apps {
if app.DisplayName == name {
if app.ID == id {
return &app
}
}
Expand All @@ -926,15 +926,15 @@ func (api *API) postWorkspaceAppHealth(rw http.ResponseWriter, r *http.Request)
if old == nil {
httpapi.Write(r.Context(), rw, http.StatusNotFound, codersdk.Response{
Message: "Error setting workspace app health",
Detail: xerrors.Errorf("workspace app name %s not found", name).Error(),
Detail: xerrors.Errorf("workspace app name %s not found", id).Error(),
})
return
}

if old.HealthcheckUrl == "" {
httpapi.Write(r.Context(), rw, http.StatusNotFound, codersdk.Response{
Message: "Error setting workspace app health",
Detail: xerrors.Errorf("health checking is disabled for workspace app %s", name).Error(),
Detail: xerrors.Errorf("health checking is disabled for workspace app %s", id).Error(),
})
return
}
Expand Down
39 changes: 15 additions & 24 deletions coderd/workspaceagents_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,8 @@ func TestWorkspaceAgentListeningPorts(t *testing.T) {
// should not exist in the response.
_, appLPort := generateUnfilteredPort(t)
app := &proto.App{
Slug: "test-app",
DisplayName: "test-app",
Url: fmt.Sprintf("http://localhost:%d", appLPort),
Slug: "test-app",
Url: fmt.Sprintf("http://localhost:%d", appLPort),
}

// Generate a filtered port that should not exist in the response.
Expand Down Expand Up @@ -624,11 +623,10 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
authToken := uuid.NewString()
apps := []*proto.App{
{
Slug: "code-server",
DisplayName: "code-server",
Command: "some-command",
Url: "http://localhost:3000",
Icon: "/code.svg",
Slug: "code-server",
Command: "some-command",
Url: "http://localhost:3000",
Icon: "/code.svg",
},
{
Slug: "code-server-2",
Expand Down Expand Up @@ -683,31 +681,24 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
// empty
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{})
require.Error(t, err)
// invalid name
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
Healths: map[string]codersdk.WorkspaceAppHealth{
"bad-name": codersdk.WorkspaceAppHealthDisabled,
},
})
require.Error(t, err)
// healcheck disabled
// healthcheck disabled
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
Healths: map[string]codersdk.WorkspaceAppHealth{
"code-server": codersdk.WorkspaceAppHealthInitializing,
Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
metadata.Apps[0].ID: codersdk.WorkspaceAppHealthInitializing,
},
})
require.Error(t, err)
// invalid value
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
Healths: map[string]codersdk.WorkspaceAppHealth{
"code-server-2": codersdk.WorkspaceAppHealth("bad-value"),
Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
metadata.Apps[1].ID: codersdk.WorkspaceAppHealth("bad-value"),
},
})
require.Error(t, err)
// update to healthy
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
Healths: map[string]codersdk.WorkspaceAppHealth{
"code-server-2": codersdk.WorkspaceAppHealthHealthy,
Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
metadata.Apps[1].ID: codersdk.WorkspaceAppHealthHealthy,
},
})
require.NoError(t, err)
Expand All @@ -716,8 +707,8 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
require.EqualValues(t, codersdk.WorkspaceAppHealthHealthy, metadata.Apps[1].Health)
// update to unhealthy
err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
Healths: map[string]codersdk.WorkspaceAppHealth{
"code-server-2": codersdk.WorkspaceAppHealthUnhealthy,
Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
metadata.Apps[1].ID: codersdk.WorkspaceAppHealthUnhealthy,
},
})
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion codersdk/workspaceapps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ type Healthcheck struct {
// @typescript-ignore PostWorkspaceAppHealthsRequest
type PostWorkspaceAppHealthsRequest struct {
// Healths is a map of the workspace app name and the health of the app.
Healths map[string]WorkspaceAppHealth
Healths map[uuid.UUID]WorkspaceAppHealth
}