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

ci: update mssql server image #2446

Merged

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Sep 23, 2024

Tests are currently failing as the Microsoft SQL Server 2017 container fails to initialize, see #2445.
This seems to be caused by microsoft/mssql-docker#899

Since that image is already way outdated anyway, this PR updates the image to Microsoft SQL Server 2022.

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (dfb2dff) to head (9e329e6).
Report is 243 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2446      +/-   ##
==========================================
- Coverage   90.97%   90.75%   -0.23%     
==========================================
  Files         146      156      +10     
  Lines        7492     7700     +208     
  Branches     1502     1583      +81     
==========================================
+ Hits         6816     6988     +172     
- Misses        676      712      +36     

see 76 files with indirect coverage changes

env:
SA_PASSWORD: mssql_passw0rd
ACCEPT_EULA: Y
ports:
- 1433:1433
options: >-
--health-cmd "/opt/mssql-tools/bin/sqlcmd -U sa -P $SA_PASSWORD -Q 'select 1' -b -o /dev/null"
--health-cmd "/opt/mssql-tools18/bin/sqlcmd -S localhost -U sa -P $SA_PASSWORD -C -Q 'select 1' -b -o /dev/null"
Copy link
Member Author

Choose a reason for hiding this comment

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

note for reviewers: that's the path from MSSQL server 2018 and up - as we're using MSSQL server 2022 I assumed there would be a /opt/mssql-tools22/ directory but inspecting the file system of a running container shows that there's only /opt/mssql-tools18/

-C ignores the self-signed certificate

Copy link
Contributor

Choose a reason for hiding this comment

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

The best docs I've found for this image is the "Overview" section here: https://hub.docker.com/r/microsoft/mssql-server

Note Starting with SQL Server 2022 CU 14, we are updating SQL Server 2022 container images to include the new mssql-tools18 package. With the introduction of SQL Server 2022 CU 14, and in all future container images, the previous directory /opt/mssql-tools/bin will be phased out.

Where by "phased out" they mean: just gone. ;)

The version for the "mssql-tools" dir is, at least somewhat, independent of the mssql-server package. I believe that's why it is "mssql-tools18" even for versions of MSSQL server beyond 2018.

@pichlermarc
Copy link
Member Author

pichlermarc commented Sep 23, 2024

Note: I've noticed that pg-pool tests may be flaky. They should not block this PR as the MS SQL Server is only used by the instrumentation-tedious package.

Copy link
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

FWIW, the SA_PASSWORD envvar is (long since) deprecated in favour of MSSQL_SA_PASSWORD.
https://learn.microsoft.com/en-us/sql/linux/sql-server-linux-configure-environment-variables?view=sql-server-2017

However, that could be updated in a separate PR.

packages/opentelemetry-test-utils/src/test-utils.ts Outdated Show resolved Hide resolved
env:
SA_PASSWORD: mssql_passw0rd
ACCEPT_EULA: Y
ports:
- 1433:1433
options: >-
--health-cmd "/opt/mssql-tools/bin/sqlcmd -U sa -P $SA_PASSWORD -Q 'select 1' -b -o /dev/null"
--health-cmd "/opt/mssql-tools18/bin/sqlcmd -S localhost -U sa -P $SA_PASSWORD -C -Q 'select 1' -b -o /dev/null"
Copy link
Contributor

Choose a reason for hiding this comment

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

The best docs I've found for this image is the "Overview" section here: https://hub.docker.com/r/microsoft/mssql-server

Note Starting with SQL Server 2022 CU 14, we are updating SQL Server 2022 container images to include the new mssql-tools18 package. With the introduction of SQL Server 2022 CU 14, and in all future container images, the previous directory /opt/mssql-tools/bin will be phased out.

Where by "phased out" they mean: just gone. ;)

The version for the "mssql-tools" dir is, at least somewhat, independent of the mssql-server package. I believe that's why it is "mssql-tools18" even for versions of MSSQL server beyond 2018.

…' helper to same 2022-latest tag


THis isn't critical. This startDocker() helper is only used for local dev testing.
@trentm trentm merged commit 5507c36 into open-telemetry:main Sep 23, 2024
19 of 20 checks passed
@pichlermarc pichlermarc deleted the chore/update-mssql-server branch September 24, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment