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

fix: Bump MSSQL image version, remove Azure SQL Edge and Papercut module #1265

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

HofmeisterAn
Copy link
Collaborator

@HofmeisterAn HofmeisterAn commented Sep 19, 2024

What does this PR do?

With the recent VM image update (Ubuntu-22.04) for GitHub-hosted runners and Microsoft-hosted agents, we encounter issues where the MSSQL and Azure SQL Edge modules crash during container startup. We experienced similar issues with Ubuntu-24.04.

We need to update the MSSQL version to one that is compatible with the new VM image: mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04. Usually, we try to avoid updating module image versions to prevent breaking changes, and we recommend that developers pin the image version. If you encounter this issue, please use the container builder API (WithImage(string)) to update the image accordingly:

WithImage("mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04")

Unfortunately, we also need to remove the Azure SQL Edge module and mark the NuGet dependency as deprecated. There is no new version available, and the image is EOL anyway.

@TechLiam I decided to remove the Papercut module as well. The pipeline frequently fails when sending or receiving SMTP messages. We can add the module again if it becomes stable and someone takes responsibility for addressing the issues/module.

I am really sorry for the inconvenience, especially since this is the second issue with the MSSQL module in such a short time. I want to stress that both issues were caused by upstream changes, and we are working to address them and provide fixes/workaround quickly.

🚩 FYI: We saw other images failing too, but these are the two we are using in Testcontainers for .NET.

Why is it important?

Our CI pipeline keeps failing. We need to address the issue to get it running again.

Related issues

@HofmeisterAn HofmeisterAn added bug Something isn't working breaking change Causing compatibility issues labels Sep 19, 2024
Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 48b85e6
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/66ec829c733adf0008aa3783
😎 Deploy Preview https://deploy-preview-1265--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@HofmeisterAn HofmeisterAn merged commit 7174786 into develop Sep 19, 2024
14 checks passed
@HofmeisterAn HofmeisterAn deleted the bugfix/debug-failing-containers-on-ci branch September 19, 2024 20:17
@TechLiam
Copy link
Contributor

TechLiam commented Sep 19, 2024

That is fair enough on removing the Papercut module I do use it so would be happy to support and take ownership of it can you point me to the logs for the problem to take a look @HofmeisterAn
Or is it to do with this if so then ok I understand #1136

@HofmeisterAn
Copy link
Collaborator Author

@TechLiam Initially, it failed with the following error:

System.Net.Mail.SmtpException: The operation has timed out.

I tried several changes to stabilize it, but unfortunately, nothing worked. I switched to SendAsync, which no longer threw the SmtpException, but then receiving the message timed out.

So basically, what I tried was this:

diff --git a/tests/Testcontainers.Papercut.Tests/PapercutContainerTest.cs b/tests/Testcontainers.Papercut.Tests/PapercutContainerTest.cs
index 6feba9c..5ed6add 100644
--- a/tests/Testcontainers.Papercut.Tests/PapercutContainerTest.cs
+++ b/tests/Testcontainers.Papercut.Tests/PapercutContainerTest.cs
@@ -23,17 +23,19 @@ public sealed class PapercutContainerTest : IAsyncLifetime

         Message[] messages;

+        using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(30));
+
         using var httpClient = new HttpClient();
         httpClient.BaseAddress = new Uri(_papercutContainer.GetBaseAddress());

         using var smtpClient = new SmtpClient(_papercutContainer.Hostname, _papercutContainer.SmtpPort);

         // When
-        smtpClient.Send("[email protected]", "[email protected]", subject, "A test message");
+        smtpClient.SendAsync("[email protected]", "[email protected]", subject, "A test message", null);

         do
         {
-            var messagesJson = await httpClient.GetStringAsync("/api/messages")
+            var messagesJson = await httpClient.GetStringAsync("/api/messages", timeoutCts.Token)
                 .ConfigureAwait(true);

             var jsonDocument = JsonDocument.Parse(messagesJson);

The pipeline failed after 30 seconds with the following error:

System.Threading.Tasks.TaskCanceledException: The operation was canceled.

Then I reduced the HTTP client timeout to 1 second and retried getting the message, but this didn’t work either (receiving timeout). Right now, I believe there is an issue where Papercut is not receiving or processing the message correctly. Maybe the wait strategy indicates readiness too early, and we send the message too soon—I'm not sure.

@TechLiam
Copy link
Contributor

Issued a PR to reintroduce the module #1268
I've added the wait strategy I think that needed for the test to not be flakey

@philipnguyxn
Copy link

Encountered this issue again on Azure DevOps pipeline with a self-hosted agent image even when using the suggested WithImage("mcr.microsoft.com/mssql/server:2022-latest") / WithImage("mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04") workaround. Tested locally on a Mac M1 Pro didn't encounter the error.

Image info:

OS: linux-x64
Dotnet SDK: ^8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Causing compatibility issues bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants