-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
new IP allocator and add postgres to integration tests. #1756
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kradalby
force-pushed
the
postgres-tests
branch
3 times, most recently
from
February 15, 2024 15:59
6296b05
to
da45a20
Compare
kradalby
changed the title
rollback gorm, add postgres to integration tests.
add postgres to integration tests.
Feb 16, 2024
kradalby
force-pushed
the
postgres-tests
branch
from
February 17, 2024 13:09
6f2deb1
to
6f17ca3
Compare
This new allocator keeps the used space in memory and loads the database only on startup. If it allocates but something errors before it gets saved to the database, then thats fine because it will be cleared on next restart. This should resolve races between ip allocation with postgres and as a side effect should make it easier to implement random allocation instead of serial if we so desire. Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby
force-pushed
the
postgres-tests
branch
from
February 17, 2024 13:11
6f17ca3
to
9f91990
Compare
kradalby
changed the title
add postgres to integration tests.
new IP allocator and add postgres to integration tests.
Feb 17, 2024
Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby
force-pushed
the
postgres-tests
branch
from
February 17, 2024 13:54
9f91990
to
93104b2
Compare
Signed-off-by: Kristoffer Dalby <[email protected]>
This commit removes the old IP allocation logic which means that we no longer need to pass IP Prefixes all around the place, this allowed us to clean up some quite racy database code and addresses are now allocated before interacting with the database and fetched before registering the machines. Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
Signed-off-by: Kristoffer Dalby <[email protected]>
kradalby
force-pushed
the
postgres-tests
branch
from
February 17, 2024 14:14
93104b2
to
f59db05
Compare
juanfont
reviewed
Feb 18, 2024
@@ -98,6 +99,10 @@ func (api headscaleV1APIServer) ListUsers( | |||
response[index] = user.Proto() | |||
} | |||
|
|||
sort.Slice(response, func(i, j int) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good lord, yes.
juanfont
approved these changes
Feb 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new IP allocator which should be more efficient, stable and less racy. This was necessary to deal with a discovered race condition in the allocation after adding postgresql integration tests.
I believe this is replacing the first "real" code I contributed to the project, and man, that was ver much needed 😆.
In addition it adds a new option to the integration tests to run them against postgresql instead of sqlite to ensure we do not have this regression again.
The last commit is the generation of new gh actions jobs and can be ignored as generated.
Fixes #1755
Signed-off-by: Kristoffer Dalby [email protected]