Skip to content

lint duplicate migration numbers before merge and in main #215

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
Open
dannykopping opened this issue Nov 15, 2024 · 6 comments
Open

lint duplicate migration numbers before merge and in main #215

dannykopping opened this issue Nov 15, 2024 · 6 comments

Comments

@dannykopping
Copy link
Collaborator

dannykopping commented Nov 15, 2024 •
edited
Loading

Exhibit A: coder/coder#15530

coder/coder#15448 was merged after coder/coder#15502, and both contained the same migration number. This passed all checks, and when we auto-deployed main to our dogfood environment we encountered a fatal error on startup:

Encountered an error running "coder server", see "coder server --help" for more information
error: connect to postgres:
    github.com/coder/coder/v2/cli.(*RootCmd).Server.func2
        /home/runner/work/coder/coder/cli/server.go:704
  - connect to postgres:                                                                                                                                                              
    github.com/coder/coder/v2/cli.getPostgresDB
        /home/runner/work/coder/coder/cli/server.go:2625
  - migrate up:                                                                                                                                                                       
        /home/runner/work/coder/coder/cli/server.go:2206
  - migrate setup:                                                                                                                                                                    
        /home/runner/work/coder/coder/coderd/database/migrations/migrate.go:116
  - create iofs:                                                                                                                                                                      
        /home/runner/work/coder/coder/coderd/database/migrations/migrate.go:82      
  - failed to init driver with path .: duplicate migration file: 000274_rename_user_link_claims.down.sql
@ethanndickson
Copy link
Member

Looks like this is possible with a merge queue - do we know if there's another way to prevent a PR from merging if a CI job fails during the merge? I assume there are existing reasons we don't use a merge queue.

@ethanndickson ethanndickson transferred this issue from coder/coder Nov 18, 2024
@coder-labeler coder-labeler bot added the invalid This doesn't seem right label Nov 18, 2024
@ethanndickson ethanndickson removed the invalid This doesn't seem right label Nov 18, 2024
@dannykopping
Copy link
Collaborator Author

Sounds like a cool idea; I'm not sure why we don't use merge queues yet.

In any case, we could probably start with just a CI check that runs on merge to main which will notify the PR author on failure.

@ethanndickson
Copy link
Member

ethanndickson commented Nov 18, 2024 •
edited
Loading

Oh, I only mentioned merge queues because duplicate migration numbers already fail a bunch of dbtestutil tests: https://github.com/coder/coder/actions/runs/11853767948/job/33034569968#step:7:594

@dannykopping
Copy link
Collaborator Author

Is that a run from main?

@ethanndickson
Copy link
Member

ethanndickson commented Nov 18, 2024 •
edited
Loading

Yep
Image

@dannykopping
Copy link
Collaborator Author

OK sweet, so at least we have that. I suspect that whoever merged the PR didn't have CI failure notifications from GitHub enabled.

We should probably route a Slack notification to #dev instead of relying on folks needing to a) have the notification enabled and b) care to look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dannykopping @ethanndickson