Skip to content

feat: Create v2 API without k8s-dqlite datastore#45

Merged
bschimke95 merged 1 commit into
mainfrom
KU-5043/remove-k8s-dqlite
Jan 26, 2026
Merged

feat: Create v2 API without k8s-dqlite datastore#45
bschimke95 merged 1 commit into
mainfrom
KU-5043/remove-k8s-dqlite

Conversation

@bschimke95

@bschimke95 bschimke95 commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

This pull request removes the k8s-dqlite datastore options from the API.

See ADR 002: k8s-dqlite removal for details.

@bschimke95 bschimke95 marked this pull request as ready for review January 21, 2026 08:25
@bschimke95 bschimke95 requested a review from a team as a code owner January 21, 2026 08:25
@bschimke95 bschimke95 marked this pull request as draft January 21, 2026 11:44
@bschimke95 bschimke95 marked this pull request as ready for review January 22, 2026 11:23

@berkayoz berkayoz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, I have some feelings about possibly doing some struct grouping and embedding to reduce duplications but not anything urgent.

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot @bschimke95! Looks good in general, tho I have some comments:
Shouldn't we mark v1.BootstrapConfig struct as deprecated as a whole? + the v1.BootstrapClusterRequest/Response structs?
Also, I noticed that we've added v2.ControlPlaneJoinConfig but it's not used anywhere. Shouldn't we also add v2.JoinClusterRequest/Response with the new config, like what we have for v1 here?

Comment thread api/v1/type_bootstrap_config.go Outdated
@bschimke95 bschimke95 force-pushed the KU-5043/remove-k8s-dqlite branch from 2440981 to 4e03d3f Compare January 26, 2026 11:46
The k8s-dqlite datastore option has been deprecated and will be removed
in this commit.
@bschimke95 bschimke95 force-pushed the KU-5043/remove-k8s-dqlite branch from 4e03d3f to afee7f6 Compare January 26, 2026 12:43
@bschimke95

Copy link
Copy Markdown
Contributor Author

Thanks for the review @HomayoonAlimohammadi

I've refactored this PR now with the new approach as per https://github.com/canonical/k8s-snap-api/blob/main/docs/adr/002-v2-api-k8s-dqlite-removal.md
The diff is now way cleaner and the concern you raised is resolved.

@berkayoz berkayoz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, pretty clean!

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing, looks great! Thanks a lot @bschimke95

@bschimke95 bschimke95 merged commit cd9b5c3 into main Jan 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants