Skip to content

feat: modify PG Coordinator to work with new v2 Tailnet API #10573

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.

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 1 commit into from
Nov 20, 2023

Conversation

spikecurtis
Copy link
Contributor

re: #10528

Refactors PG Coordinator to work with the Tailnet v2 API, including wrappers for the existing v1 API.

The debug endpoint functions, but doesn't return sensible data, that will be in another stacked PR.

Copy link
Contributor Author

spikecurtis commented Nov 8, 2023

@spikecurtis spikecurtis force-pushed the spike/10528-sql-queries branch from 0d829dd to 504d82c Compare November 8, 2023 09:43
@spikecurtis spikecurtis force-pushed the spike/10528-pg-coord-refactor branch from 0e96fa3 to e74a0f4 Compare November 8, 2023 09:43
@spikecurtis spikecurtis requested a review from coadler November 8, 2023 09:44
@spikecurtis spikecurtis force-pushed the spike/10528-pg-coord-refactor branch from e74a0f4 to 0cbde53 Compare November 8, 2023 09:46
@spikecurtis spikecurtis force-pushed the spike/10528-sql-queries branch 3 times, most recently from 06746f6 to e9ec10d Compare November 13, 2023 12:08
@spikecurtis spikecurtis force-pushed the spike/10528-pg-coord-refactor branch from 0cbde53 to c36befe Compare November 13, 2023 12:08
@spikecurtis spikecurtis force-pushed the spike/10528-sql-queries branch from e9ec10d to b0c48ca Compare November 14, 2023 06:55
@spikecurtis spikecurtis force-pushed the spike/10528-pg-coord-refactor branch from c36befe to 2a2e2db Compare November 14, 2023 06:55
Base automatically changed from spike/10528-sql-queries to main November 15, 2023 06:13
// Node returns an in-memory node by ID.
Node(id uuid.UUID) *Node
Close() error
Coordinate(ctx context.Context, id uuid.UUID, name string, a TunnelAuth) (chan<- *proto.CoordinateRequest, <-chan *proto.CoordinateResponse)
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious on your thoughts: on public interfaces I normally like returning a wrapper with functions to access the channels instead of returning raw channels. Does that feel too abstracty for this? I could feel it being slightly verbose needing to always store two channels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good instinct, and I think it depends on how we expect it to be used. In this case, I'm imagining that we will have an object that "glues" the Coordinate() call to the drpc Stream.

e.g.

type coordinateStreamer struct {
    stream proto.DRPCStream_Coordinate
    reqs chan<- *proto.CoordinateRequests
    resps <-chan *proto.CoordinateResponse
}

And this thing will have very simple methods that copy from the channels into the stream and vice versa.

Thus, I'm kinda already planning to wrap them up in a struct as soon as they are returned, so putting them together in a wrapper just means I have to unwrap.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense 👍

@spikecurtis spikecurtis force-pushed the spike/10528-pg-coord-refactor branch from 2a2e2db to 63becc5 Compare November 17, 2023 12:37
@spikecurtis spikecurtis merged commit 5c48cb4 into main Nov 20, 2023
@spikecurtis spikecurtis deleted the spike/10528-pg-coord-refactor branch November 20, 2023 10:31
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants