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

Migrate Bitnami images to the official ones #277

Merged
merged 5 commits into from
Apr 25, 2022
Merged

Migrate Bitnami images to the official ones #277

merged 5 commits into from
Apr 25, 2022

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Apr 21, 2022

Today Bitnami migrated their images to Dockerhub Bitnami. The migration caused several test errors in beefy-scenarios, more specifically:

  • 014-quarkus-panache-with-transactions-xa failed due to missing quay.io/bitnami/mysql:5.7.32
  • 019-quarkus-consul failed due to missing quay.io/bitnami/consul:1.9.3
  • 302-quarkus-vertx-jwt failed due to missing quay.io/bitnami/redis:6.0
  • 002-quarkus-all-extensions failed due to missing quay.io/bitnami/mongodb:4.0.23

while I didn't find connect between migration and following failures (so far, there might be correlation between 302-quarkus-vertx-jwt and other vertx failures):

  • 013-quarkus-oidc-restclient
  • 300-quarkus-vertx-webclient
  • 301-quarkus-vertx-kafka

This PR migrates Bitnami images to the official ones:
quay.io/bitnami/mongodb:4.0.23 ----> mongo:4.0.23
quay.io/bitnami/mysql:5.7.32 -------> mysql:5.7 (change suggested here)
quay.io/bitnami/postgresql:13.4.0 ----> postgres:13.6 (mirroring changes made here)
quay.io/bitnami/mysql:8.0 -----> mysql:8.0
quay.io/bitnami/consul:1.9.3 ----> 1.9.3
quay.io/bitnami/redis:6.0 ----> redis:6.0

@pjgg pjgg self-requested a review April 22, 2022 07:20
@pjgg
Copy link
Contributor

pjgg commented Apr 22, 2022

please review CI failures!

Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

Please use long names with docker.io/ prefix

e.g. docker.io/consul:1.9.3

@rsvoboda
Copy link
Member

013-quarkus-oidc-restclient fails because of recent change in Quarkus main - quarkusio/quarkus#24666

Test needs to be adjusted to check for enhanced details

@michalvavrik
Copy link
Member Author

michalvavrik commented Apr 22, 2022

PR now uses long docker image names (please see a separate commit).

013-quarkus-oidc-restclient test is fixed by setting a same OpenApi Info title for both JVM and Native build (as mentioned above, there are recent changes that changed the behavior)

@michalvavrik michalvavrik marked this pull request as draft April 22, 2022 11:05
@rsvoboda
Copy link
Member

ah, so you are dsiabling the tests in 300 and 301

I have prepare #278 to fix those modules

So maybe the best way would be to cherry-pick my commit and use it in this PR

@michalvavrik
Copy link
Member Author

I review your PR and looks good, therefore I got rid of the commit that disabled tests. thanks!

@michalvavrik michalvavrik marked this pull request as ready for review April 22, 2022 12:43
@michalvavrik michalvavrik marked this pull request as draft April 22, 2022 14:31
@rsvoboda
Copy link
Member

To fix 303 and 304 you need to add these changes:

303-quarkus-vertx-sql

--- a/303-quarkus-vertx-sql/pom.xml
+++ b/303-quarkus-vertx-sql/pom.xml
@@ -44,6 +44,10 @@
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-flyway</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.flywaydb</groupId>
+            <artifactId>flyway-mysql</artifactId>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-smallrye-openapi</artifactId>

Details in https://github.com/quarkusio/quarkus/wiki/Migration-Guide-2.7#flyway

304-quarkus-vertx-routes

diff --git a/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationAssertions.java b/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationAssertions.java
index 1bef9e6..2dcbed6 100644
--- a/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationAssertions.java
+++ b/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationAssertions.java
@@ -17,7 +17,7 @@ public class ValidationAssertions {
     }

     public static final void assertValidationErrorDetails(ValidationErrorResponse response) {
-        assertEquals("validation constraint violations", response.getDetails());
+        assertEquals("validation constraint violations", response.getDetail());
     }

     public static final void assertValidationErrorStatus(ValidationErrorResponse response, int expected) {
diff --git a/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationErrorResponse.java b/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationErrorResponse.java
index 2d7ef42..86c8d38 100644
--- a/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationErrorResponse.java
+++ b/304-quarkus-vertx-routes/src/test/java/io/quarkus/qe/validation/utils/ValidationErrorResponse.java
@@ -2,7 +2,7 @@ package io.quarkus.qe.validation.utils;

 public class ValidationErrorResponse {
     private String title;
-    private String details;
+    private String detail;
     private int status;
     private ValidationError[] violations;

@@ -14,12 +14,12 @@ public class ValidationErrorResponse {
         this.title = title;
     }

-    public String getDetails() {
-        return details;
+    public String getDetail() {
+        return detail;
     }

-    public void setDetails(String details) {
-        this.details = details;
+    public void setDetail(String details) {
+        this.detail = details;
     }

     public int getStatus() {

Response changed (was fixed, details => detail), example:

{
    "title": "Constraint Violation",
    "detail": "validation constraint violations",
    "status": 400,
    "violations": [
        {
            "field": "validateRequestBody.param.custom",
            "message": "Value must be uppercase"
        }
    ]
}

@rsvoboda rsvoboda marked this pull request as ready for review April 22, 2022 19:26
@rsvoboda
Copy link
Member

Vanilla MySQL needs more specific log entry check, it prints log with ready for connections in the DB setup phase. Bitnami has different logs in DB setup phase.

  • docker run --name some-mysql -e MYSQL_ROOT_PASSWORD=my-secret-pw mysql:8.0
  • docker run --name some-mysql-bit -e MYSQL_ROOT_PASSWORD=my-secret-pw quay.io/bitnami/mysql:8.0

@michalvavrik
Copy link
Member Author

michalvavrik commented Apr 22, 2022

@rsvoboda thank you, I've tried that moments before with .*/usr/sbin/mysqld: ready for connections.* which didn't fix, but your log message works.

@michalvavrik
Copy link
Member Author

michalvavrik commented Apr 23, 2022

Build currently fails on 301-quarkus-vertx-kafka#ConfluentKafkaTest, I think the problem was introduced with this commit, but it's not related to this PR, so maybe we could merge this commit and resolve the problem in a separate issue? Thank you

@rsvoboda rsvoboda merged commit 01dfcd7 into quarkus-qe:main Apr 25, 2022
@rsvoboda
Copy link
Member

Please continue with 301-quarkus-vertx-kafka#ConfluentKafkaTest in separate PR

@michalvavrik michalvavrik deleted the feature/migrate-docker-img-registry branch April 25, 2022 07:28
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.

3 participants