Skip to content

feat(coderd): store workspace proxy version in the database #10790

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

Jump to bottom
Merged
merged 7 commits into from
Nov 21, 2023
Merged

feat(coderd): store workspace proxy version in the database #10790

merged 7 commits into from
Nov 21, 2023

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Nov 20, 2023 •
edited
Loading

Part of #9558

When workspace proxies come online, they hit the /register endpoint with a payload.
Part of this payload is the proxy version (buildinfo.Version).
Right now the /register handler performs a check that the major and minor versions match before allowing the registration to complete.

This change just adds a database column and updates the stored version with the value reported by workspace proxies.

@johnstcn johnstcn self-assigned this Nov 20, 2023
@johnstcn johnstcn requested review from mtojek and Emyrk November 20, 2023 14:54
johnstcn
johnstcn commented Nov 20, 2023
View reviewed changes
coderd/database/migrations/000170_workspaceproxy_version.down.sql
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a 169 in the pipeline already, so renaming preemptively

mtojek
mtojek approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

In case of future improvements, we don't expect another version column? Let's say, protocol version, agent version, foobar version, etc.

@johnstcn
Copy link
Member Author

In case of future improvements, we don't expect another version column? Let's say, protocol version, agent version, foobar version, etc.

workspace_proxies.proxy_version is really cumbersome. I initially considered api_version but felt that would be easily confused with the corresponding workspace_agents.api_version.

@mtojek
Copy link
Member

mtojek commented Nov 20, 2023 •
edited
Loading

release_version, app_version ?

EDIT:

I'm good with version too!

Emyrk
Emyrk previously requested changes Nov 20, 2023
View reviewed changes
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

We need the version field on the codersdk type too right?

coder/codersdk/workspaceproxy.go

Line 61 in 7c4043d

Deleted bool `json:"deleted" table:"deleted"`

@Emyrk
Copy link
Member

Emyrk commented Nov 20, 2023

And the convert

coder/enterprise/coderd/workspaceproxy.go

Line 948 in 7c4043d

return codersdk.WorkspaceProxy{

@johnstcn johnstcn requested a review from Emyrk November 20, 2023 16:47
@johnstcn johnstcn dismissed Emyrk’s stale review November 20, 2023 17:45

addressed comments

@johnstcn johnstcn force-pushed the cj/workspaceproxy-version branch from c526c37 to 2794d5c Compare November 21, 2023 10:57
@johnstcn johnstcn merged commit abafc08 into main Nov 21, 2023
@johnstcn johnstcn deleted the cj/workspaceproxy-version branch November 21, 2023 11:21
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Emyrk Emyrk Emyrk approved these changes

@mtojek mtojek mtojek approved these changes

Assignees

@johnstcn johnstcn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@johnstcn @mtojek @Emyrk