Skip to content
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

MySQL Support #2837

Open
wants to merge 77 commits into
base: main
Choose a base branch
from
Open

MySQL Support #2837

wants to merge 77 commits into from

Conversation

ismail0234
Copy link
Contributor

@ismail0234 ismail0234 commented Nov 4, 2024

Describe your changes

MySQL support for Netbird. Still in draft stage. I have never used the “GO” language before, so if I have any mistakes you can help me to correct them.

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

@CLAassistant
Copy link

CLAassistant commented Nov 4, 2024

CLA assistant check
All committers have signed the CLA.

@ismail0234
Copy link
Contributor Author

I'm working on it, and from what I've seen, adding mysql support seems to be pretty straightforward. But I haven't tested it yet and I'm not sure if it will work, so it's still in draft stage. Can someone tell me how to compile and test on the server side?

@mlsmaycon
Copy link
Collaborator

Thanks for the submission @ismail0234. Can you enable github action tests for MySQL as well?

See the current ones here:

@mlsmaycon
Copy link
Collaborator

@bcmmbaga can you share how to add a testcontainer for MySQL?

@bcmmbaga
Copy link
Contributor

bcmmbaga commented Nov 4, 2024

@ismail0234 To add a MySQL test container, you will need to add MySQL support in the NewTestStoreFromSQL

func NewTestStoreFromSQL(ctx context.Context, filename string, dataDir string) (Store, func(), error) {

To spin up a new MySQL test container, you can follow the same approach we used for PostgreSQL

func CreatePGDB() (func(), error) {

@ismail0234
Copy link
Contributor Author

The “security/snyk” step seems to have failed but I can't see what caused it.

@bcmmbaga
Copy link
Contributor

bcmmbaga commented Nov 4, 2024

The “security/snyk” step seems to have failed but I can't see what caused it.

Go modules seem out of sync with the codebase. Could you run go mod tidy and push the changes?

@ismail0234
Copy link
Contributor Author

hmm, the result still seems to be the same.

@ismail0234
Copy link
Contributor Author

ismail0234 commented Nov 5, 2024

Is this part used for testing? Do I need to add mysql values?

store: [ 'sqlite', 'postgres' ]

if [ "${{ matrix.store }}" == "postgres" ]; then
echo "NETBIRD_STORE_ENGINE_POSTGRES_DSN=host=$(hostname -I | awk '{print $1}') user=netbird password=postgres dbname=netbird port=5432" >> $GITHUB_ENV
else
echo "NETBIRD_STORE_ENGINE_POSTGRES_DSN==" >> $GITHUB_ENV
fi

@ismail0234
Copy link
Contributor Author

TestContainers have been optimized. Instead of creating a new test container each time, we create 1 for mysql and postgres and cache it.

Test time for Postgres decreased from 350+ seconds to 140 seconds.
Test time for MySQL decreased from 1800+ seconds to 150 seconds.

@ismail0234
Copy link
Contributor Author

The MySQL test seems to have been successful. 🥳

https://github.com/ismail0234/netbird/actions/runs/11777556741

@ismail0234
Copy link
Contributor Author

Hi @mlsmaycon ,

How can I use this in a new server?

@ismail0234 ismail0234 mentioned this pull request Nov 11, 2024
6 tasks
@ismail0234
Copy link
Contributor Author

I tested it in production environment. Mysql connection is established and working without any problem.

But when I use 127.0.0.1 in the mysql connection string, the connection is not established. When I enter the ip address of the server, the connection can be established. I am not sure if this is a docker related issue. Mysql and netbird are running on the same server.

image

@ismail0234
Copy link
Contributor Author

Some tests failed. Is it a general problem?

@ismail0234
Copy link
Contributor Author

Everything is complete. I've tested it in a production environment and so far I haven't had any problems. The peers can connect to each other and we can use the panel features. It looks like there's nothing else I can do. @bcmmbaga

@ismail0234
Copy link
Contributor Author

Using MySQL database made the api calls very fast. When I use sqlite, api calls take 1.5-4 seconds when there are 500 peers on the network, while it takes 0.2 seconds when we use mysql.

@ismail0234
Copy link
Contributor Author

@mlsmaycon @bcmmbaga @pascal-fischer Can you check the Pull Request to merge?

Copy link

sonarcloud bot commented Nov 28, 2024

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.

5 participants