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

WIP: chore(deps): update mysql docker tag to v8 #1631

Closed
wants to merge 7 commits into from

Conversation

blumamir
Copy link
Member

@blumamir blumamir commented Aug 11, 2023

This is an attempt to apply the changes in this renovate PR: #1406 along with some necessary changes:

  • also update the image tag in test-utils and package.json that reference the old version

  • As described in details in this StackOverflow post:

MySQL 8 has supports pluggable authentication methods. By default, one of them named caching_sha2_password is used rather than our good old mysql_native_password

Our tests uses native passwords, so obviously they couldn't connect. The new authentication method is not supported by mysql package, and old widely used versions of mysql2. I found that mysql8 can be run with mysql_native_password by using the following flag when running docker: --default-authentication-plugin=mysql_native_password.

This PR is for applying relevant fixes to the renovate PR to use the new image tag.
The current image 5.7 has no docker image for Apple M2 chip on mac, therefore I am not even able to run current tests on my Mac.

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #1631 (72a3e33) into main (8802eae) will decrease coverage by 0.71%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1631      +/-   ##
==========================================
- Coverage   91.75%   91.05%   -0.71%     
==========================================
  Files         137       91      -46     
  Lines        7084     4728    -2356     
  Branches     1426      918     -508     
==========================================
- Hits         6500     4305    -2195     
+ Misses        584      423     -161     

see 47 files with indirect coverage changes

@blumamir
Copy link
Member Author

blumamir commented Aug 11, 2023

the service in github actions is defined like so:

      mysql:
        image: mysql:8.0
        env:
          MYSQL_USER: otel
          MYSQL_PASSWORD: secret
          MYSQL_DATABASE: otel_mysql_database
          MYSQL_ROOT_PASSWORD: rootpw
        ports:
        - 3306:3306
        options: >-
          --health-cmd="mysqladmin ping"
          --health-interval 10s
          --health-timeout 5s
          --health-retries 5

And I need a way to pass the --default-authentication-plugin=mysql_native_password flag to the container. Looking at gh docs, I don't see a way to do that.

Another option is to mount a MySQL config file, as suggested here under "Using a custom MySQL configuration file":

-v /my/custom:/etc/mysql/conf.d

This will require us to add a config file next to github workflows which isn't great.

This option is not exposed as environment variable, as can be seen in MySQL environment variable docs.

By using the command docker run -it --rm mysql:8.0 --verbose --help I am able to see that the default is indeed:

default-authentication-plugin                                caching_sha2_password

While we need mysql_native_password to support old and widely used mysql and mysql2 instrumentations

@github-actions
Copy link
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 16, 2023
Copy link
Contributor

github-actions bot commented Nov 6, 2023

This PR was closed because it has been stale for 14 days with no activity.

@github-actions github-actions bot closed this Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants