sergehuber opened a new pull request, #767: URL: https://github.com/apache/unomi/pull/767
> **Note:** This PR replaces #759, which was accidentally closed when the head branch was renamed from \`UNOMI-itests-es-docker\` to \`UNOMI-921-itests-es-docker\` to align local and remote naming. All commit history, review comments, and fixes from #759 are preserved in this branch — see that PR for the full review thread. --- ### Stacked PR (merge order) | Role | Branch | |------|--------| | **Base (merge into)** | `master` | | **Head (this PR)** | `UNOMI-921-itests-es-docker` | This PR is **stacked**: it is the bottom of a chain. Merge **into `master` first**; downstream branches that build on top will resync onto the new `master` tip. --- **JIRA:** https://issues.apache.org/jira/browse/UNOMI-921 — _Replace elasticsearch-maven-plugin with Docker-based Elasticsearch Instance in integration tests_. ### Why this change The `elasticsearch` profile in `itests/pom.xml` currently uses `com.github.alexcojocaru:elasticsearch-maven-plugin` to download an Elasticsearch tarball and run it as a forked JVM during the Maven build. This approach: - requires downloading Elasticsearch binaries during the build, - creates a `default_template` that overrides user templates on ES 8/9 — currently worked around by `BaseIT.fixDefaultTemplateIfNeeded()`, - is inconsistent with the `opensearch` profile (same `itests/pom.xml`), which already uses Docker, and - has a more complex lifecycle to manage. Aligning Elasticsearch on the same Docker-based approach the `opensearch` profile uses removes the template-override workaround, eliminates the binary download, and makes both search-engine profiles symmetric. ### What changed #### `itests/pom.xml` — `elasticsearch` profile - Add `<elasticsearch.port>9400</elasticsearch.port>` and surface it to Failsafe via `systemPropertyVariables`, so tests resolve the HTTP port from a single property (value unchanged: `9400`). - Replace the `elasticsearch-maven-plugin` execution with an `io.fabric8:docker-maven-plugin` execution running: - Image: `docker.elastic.co/elasticsearch/elasticsearch:${elasticsearch.test.version}` - Port mapping: container `9200` → host `9400` - Heap: `${elasticsearch.heap}` (default `4g`, overridable) — aligned with OpenSearch - Settings: `discovery.type=single-node`, `xpack.ml.enabled=false`, `xpack.security.enabled=false`, `cluster.routing.allocation.disk.threshold_enabled=false`, `path.repo=/tmp/snapshots_repository` - Volume bind: `${project.build.directory}/snapshots_repository` → `/tmp/snapshots_repository` - HTTP wait probe before the integration-test phase - **Container lifecycle matches the OpenSearch profile exactly**: `pre-integration-test` runs `stop` + `remove` (idempotent cleanup, with `ignoreRunningContainers`) then `start` with `showLogs`; `post-integration-test` runs `stop` (skippable via `it.keepContainer`). - Add `chmod -R ugo+rwx` on `target/snapshots_repository` in the antrun `unzip` step. The official ES image runs as UID 1000, so on Linux CI the bind-mounted snapshot repository otherwise hits `access_denied` during `repository verify` operations. #### `itests/pom.xml` — `opensearch` profile - Add symmetric `stop-opensearch` `post-integration-test` execution (was missing). - Add `ignoreRunningContainers` to the pre-start cleanup execution. - Remove `-Dcluster.default.index.settings.number_of_replicas=0` from `OPENSEARCH_JAVA_OPTS` (JVM property, had no effect on OpenSearch settings). #### `itests/pom.xml` — both profiles - Add `it.keepContainer` property (default `false`). When `true`, the post-test `stop` execution is skipped so the container stays alive for post-failure inspection. #### `pom.xml` (root) - Declare `<docker-maven-plugin.version>0.48.0</docker-maven-plugin.version>` and add the corresponding `<pluginManagement>` entry so the `elasticsearch` profile inherits a single version. #### `build.sh` - Add `--keep-container` flag wired to `-Dit.keepContainer=true`, so the container can be kept alive for inspection directly from the build script. #### `itests/src/test/java` — test fixes - `GraphQLListIT`: restore the `catch` block and null-guard in the `keepTrying` supplier that were inadvertently removed — without them, a transient HTTP error aborts the poll loop immediately instead of retrying. - `PropertiesUpdateActionIT`: tighten the assert message on the `changes > 0` check. ### Review history (from #759) The following Copilot review comments were raised on #759 and addressed in this branch: - **`-Dcluster.default.index.settings.number_of_replicas=0` passed as a JVM `-D` property** — removed from both `ES_JAVA_OPTS` and `OPENSEARCH_JAVA_OPTS`; Unomi creates its own indices with explicit settings so a cluster-level default isn't needed. - **ES heap hardcoded at `-Xms8g -Xmx8g`** — made configurable via `${elasticsearch.heap}` (default `4g`), aligned with the OpenSearch profile. ### Verification - `./build.sh --integration-tests` (ES): container starts on `9400`, all integration tests pass, container stops after the run. - `./build.sh --integration-tests --use-opensearch` (OpenSearch): same, container stops after the run. - `./build.sh --integration-tests --keep-container`: container remains running after tests for inspection. ### Follow-ups (tracked under the same JIRA) - Remove `BaseIT.fixDefaultTemplateIfNeeded()` and its call in `checkSearchEngine()` (now dead code under Docker). - `Migrate16xToCurrentVersionIT`: replace the hardcoded `ES_BASE_URL = "http://localhost:9400"` with the dynamic `getSearchPort()` resolution. - Drop the comments referring to the `elasticsearch-maven-plugin` template-override workaround. - Remove the explicit `<version>` pin from the OpenSearch profile's `docker-maven-plugin` declaration (should inherit from `pluginManagement`). - [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
