Skip to content

fix: allow tag removal in provisioner upsert #14187

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 2 commits into from
Aug 6, 2024
Merged

fix: allow tag removal in provisioner upsert #14187

merged 2 commits into from
Aug 6, 2024

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented Aug 6, 2024 •
edited
Loading

If you start a provisioner with a tag and then restart it at some later time without that tag, the app returns a 500 error. This change fixes the issue by allowing provisioner tags to be added and removed freely.

The included test can be ran against main to demonstrate the issue:

t.go:102: 2024-08-06 15:14:32.459 [erro]  coderd: create provisioner daemon  name=pPxKaqhVKPQjumKlJTA8RgvADF4oZTNODNixI9aHrC7JDSfbd3kJOfBFtpQncGw  provisioners=[echo]  tags={"owner":"","scope":"organization"}  request_id=8e77bf9b-dcd0-4748-b9af-e48b683ffb17  error="sql: no rows in result set"
    t.go:99: 2024-08-06 15:14:32.459 [warn]  coderd: GET  host=localhost:40747  path=/api/v2/organizations/28ccf372-3006-4c4c-b65c-cce806813dd4/provisionerdaemons/serve  proto=HTTP/1.1  remote_addr=127.0.0.1  start="2024-08-06T15:14:32.449511657Z"  took=9.90056ms  status_code=500  latency_ms=9  request_id=8e77bf9b-dcd0-4748-b9af-e48b683ffb17 ...
        response_body= {
                        "message": "Internal error creating provisioner daemon.",
                        "detail": "sql: no rows in result set"
                       }
    provisionerdaemons_test.go:382: 
                Error Trace:    /home/coder/coder/enterprise/coderd/provisionerdaemons_test.go:382
                Error:          Received unexpected error:
                                GET http://localhost:40747/api/v2/organizations/28ccf372-3006-4c4c-b65c-cce806813dd4/provisionerdaemons/serve?id=a8966410-8bb6-4e84-ba0e-e3b144560342&name=pPxKaqhVKPQjumKlJTA8RgvADF4oZTNODNixI9aHrC7JDSfbd3kJOfBFtpQncGw&provisioner=echo&tag=scope%3Dorganization&version=1.1&version=1.1: unexpected status code 500: Internal error creating provisioner daemon.
                                        Error: sql: no rows in result set
                Test:           TestProvisionerDaemonServe/ChangeTags

I've also changed the unique index to include the organization_id so that names between orgs will not collide.

@f0ssel f0ssel requested review from johnstcn and Emyrk August 6, 2024 15:20
Emyrk
Emyrk approved these changes Aug 6, 2024
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.

I don't think this change in behavior is breaking since it allows more functionality (removing tags) then the previous.

@f0ssel f0ssel merged commit fab1960 into main Aug 6, 2024
31 checks passed
@f0ssel f0ssel deleted the f0ssel/allow-tag-removal branch August 6, 2024 15:38
@github-actions github-actions bot locked and limited conversation to collaborators Aug 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@johnstcn johnstcn johnstcn approved these changes

@Emyrk Emyrk Emyrk approved these changes

Assignees

@f0ssel f0ssel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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