rabbitmq - integration test SendMessageRabbitmqIT is broken #143
Comments
|
Also maybe we should make the test rely on the LDAP content (e.g. check if the user has been created) instead of parsing container's stdout. |
pmauduit
added a commit
that referenced
this issue
Sep 11, 2024
Check the account creation onto the LDAP instead of parsing the console output.
pmauduit
added a commit
to georchestra/georchestra
that referenced
this issue
Sep 11, 2024
1. Fixing version mismatch between dependencies in the pom Some weeks ago, a version bump of `spring-amqp` broke the rabbitmq integration (see CICD of the gateway and georchestra/georchestra-gateway#143 for the details). While digging into the issue, it turned out that PR#4295 upgraded the dependency to a version non compatible with `spring-rabbitmq`, which generated a docker image for the console which won't be viable in regards to the rabbitmq integration. spring-rabbitmq already depends on spring-amqp, which by the way should be forced into a specific version somewhere else. Sticking to 1.7.12.RELEASE of spring-rabbit, so that we keep a compatible version with amqp. 2. Rework console message consumption When a user account from a third-party oauth2 identity provider is saved into the LDAP (at first connection through the gateway), a message is sent via rabbitMq and consumed by the console. In order to avoid consumption of the same message several times, a `Set<String>` was created as a static member of the `RabbitmqEventsListener` class. But if an exception popped before reaching the call to add the message uid to the list, then the `onMessage` method was then called once again, in a kind of infinite loop. We had some issues with one of the external service recently on one of our infrastructure, and we suspect that this behaviour generated a huge amount of unecessary logs because of this infinite loop. This PR suggests to rewrite the `onMessage()` in the following way: * removing the `Set<String>` (what will it look like in some centuries of console uptime ?) * log the error to the logs * discards the message (by simply returning from the onMessage() without throwing any exception 3. tests Added an IT, which was the reason leading to the 2. above Tests: Added an integration test (along with a dependency against testcontainers). This is a first step to integrate the library and drop the IT dedicated docker composition. Caveat: we are late on the spring-amqp/rabbit version, but at least we will have a test ready when we will go for a version bump.
pmauduit
added a commit
to georchestra/georchestra
that referenced
this issue
Sep 11, 2024
1. Fixing version mismatch between dependencies in the pom Some weeks ago, a version bump of `spring-amqp` broke the rabbitmq integration (see CICD of the gateway and georchestra/georchestra-gateway#143 for the details). While digging into the issue, it turned out that PR#4295 upgraded the dependency to a version non compatible with `spring-rabbitmq`, which generated a docker image for the console which won't be viable in regards to the rabbitmq integration. spring-rabbitmq already depends on spring-amqp, which by the way should be forced into a specific version somewhere else. Sticking to 1.7.12.RELEASE of spring-rabbit, so that we keep a compatible version with amqp. 2. Rework console message consumption When a user account from a third-party oauth2 identity provider is saved into the LDAP (at first connection through the gateway), a message is sent via rabbitMq and consumed by the console. In order to avoid consumption of the same message several times, a `Set<String>` was created as a static member of the `RabbitmqEventsListener` class. But if an exception popped before reaching the call to add the message uid to the list, then the `onMessage` method was then called once again, in a kind of infinite loop. We had some issues with one of the external service recently on one of our infrastructure, and we suspect that this behaviour generated a huge amount of unecessary logs because of this infinite loop. This PR suggests to rewrite the `onMessage()` in the following way: * removing the `Set<String>` (what will it look like in some centuries of console uptime ?) * log the error to the logs * discards the message (by simply returning from the onMessage() without throwing any exception 3. tests Added an IT, which was the reason leading to the 2. above Tests: Added an integration test (along with a dependency against testcontainers). This is a first step to integrate the library and drop the IT dedicated docker composition. Caveat: we are late on the spring-amqp/rabbit version, but at least we will have a test ready when we will go for a version bump.
pmauduit
added a commit
to georchestra/georchestra
that referenced
this issue
Sep 11, 2024
1. Fixing version mismatch between dependencies in the pom Some weeks ago, a version bump of `spring-amqp` broke the rabbitmq integration (see CICD of the gateway and georchestra/georchestra-gateway#143 for the details). While digging into the issue, it turned out that PR#4295 upgraded the dependency to a version non compatible with `spring-rabbitmq`, which generated a docker image for the console which won't be viable in regards to the rabbitmq integration. spring-rabbitmq already depends on spring-amqp, which by the way should be forced into a specific version somewhere else. Sticking to 1.7.12.RELEASE of spring-rabbit, so that we keep a compatible version with amqp. 2. Rework console message consumption When a user account from a third-party oauth2 identity provider is saved into the LDAP (at first connection through the gateway), a message is sent via rabbitMq and consumed by the console. In order to avoid consumption of the same message several times, a `Set<String>` was created as a static member of the `RabbitmqEventsListener` class. But if an exception popped before reaching the call to add the message uid to the list, then the `onMessage` method was then called once again, in a kind of infinite loop. We had some issues with one of the external service recently on one of our infrastructure, and we suspect that this behaviour generated a huge amount of unecessary logs because of this infinite loop. This PR suggests to rewrite the `onMessage()` in the following way: * removing the `Set<String>` (what will it look like in some centuries of console uptime ?) * log the error to the logs * discards the message (by simply returning from the onMessage() without throwing any exception 3. tests Added an IT, which was the reason leading to the 2. above Tests: Added an integration test (along with a dependency against testcontainers). This is a first step to integrate the library and drop the IT dedicated docker composition. Caveat: we are late on the spring-amqp/rabbit version, but at least we will have a test ready when we will go for a version bump.
pmauduit
added a commit
to georchestra/georchestra
that referenced
this issue
Sep 11, 2024
1. Fixing version mismatch between dependencies in the pom Some weeks ago, a version bump of `spring-amqp` broke the rabbitmq integration (see CICD of the gateway and georchestra/georchestra-gateway#143 for the details). While digging into the issue, it turned out that PR#4295 upgraded the dependency to a version non compatible with `spring-rabbitmq`, which generated a docker image for the console which won't be viable in regards to the rabbitmq integration. spring-rabbitmq already depends on spring-amqp, which by the way should be forced into a specific version somewhere else. Sticking to 1.7.12.RELEASE of spring-rabbit, so that we keep a compatible version with amqp. 2. Rework console message consumption When a user account from a third-party oauth2 identity provider is saved into the LDAP (at first connection through the gateway), a message is sent via rabbitMq and consumed by the console. In order to avoid consumption of the same message several times, a `Set<String>` was created as a static member of the `RabbitmqEventsListener` class. But if an exception popped before reaching the call to add the message uid to the list, then the `onMessage` method was then called once again, in a kind of infinite loop. We had some issues with one of the external service recently on one of our infrastructure, and we suspect that this behaviour generated a huge amount of unecessary logs because of this infinite loop. This PR suggests to rewrite the `onMessage()` in the following way: * removing the `Set<String>` (what will it look like in some centuries of console uptime ?) * log the error to the logs * discards the message (by simply returning from the onMessage() without throwing any exception 3. tests Added an IT, which was the reason leading to the 2. above Tests: Added an integration test (along with a dependency against testcontainers). This is a first step to integrate the library and drop the IT dedicated docker composition. Caveat: we are late on the spring-amqp/rabbit version, but at least we will have a test ready when we will go for a version bump.
github-actions bot
pushed a commit
to georchestra/georchestra
that referenced
this issue
Sep 30, 2024
1. Fixing version mismatch between dependencies in the pom Some weeks ago, a version bump of `spring-amqp` broke the rabbitmq integration (see CICD of the gateway and georchestra/georchestra-gateway#143 for the details). While digging into the issue, it turned out that PR#4295 upgraded the dependency to a version non compatible with `spring-rabbitmq`, which generated a docker image for the console which won't be viable in regards to the rabbitmq integration. spring-rabbitmq already depends on spring-amqp, which by the way should be forced into a specific version somewhere else. Sticking to 1.7.12.RELEASE of spring-rabbit, so that we keep a compatible version with amqp. 2. Rework console message consumption When a user account from a third-party oauth2 identity provider is saved into the LDAP (at first connection through the gateway), a message is sent via rabbitMq and consumed by the console. In order to avoid consumption of the same message several times, a `Set<String>` was created as a static member of the `RabbitmqEventsListener` class. But if an exception popped before reaching the call to add the message uid to the list, then the `onMessage` method was then called once again, in a kind of infinite loop. We had some issues with one of the external service recently on one of our infrastructure, and we suspect that this behaviour generated a huge amount of unecessary logs because of this infinite loop. This PR suggests to rewrite the `onMessage()` in the following way: * removing the `Set<String>` (what will it look like in some centuries of console uptime ?) * log the error to the logs * discards the message (by simply returning from the onMessage() without throwing any exception 3. tests Added an IT, which was the reason leading to the 2. above Tests: Added an integration test (along with a dependency against testcontainers). This is a first step to integrate the library and drop the IT dedicated docker composition. Caveat: we are late on the spring-amqp/rabbit version, but at least we will have a test ready when we will go for a version bump. (cherry picked from commit 26dbbf4)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Noticed here:
#142 (comment)
I first thought it was because console docker logging changed in newer versions of the image, as the test result relies on it, but turned out it might be related to a spring upgrade console-side:
Maybe we are better off deactivating this test for now, as it breaks the build on something which has to be fixed console-side.
The text was updated successfully, but these errors were encountered: