SEZ9 opened a new pull request, #12298: URL: https://github.com/apache/seatunnel/pull/12298
### Purpose of this pull request Closes #12294. When `seatunnel.engine.http.enable-dynamic-port` is `true` and the configured port is taken, `JettyService` binds a different port but never publishes it. `HttpConfig.getPort()` keeps returning the configured value, and `GetNodeHttpPortOperation` — the only way a peer learns another node's REST port — reads exactly that value: ```java response = service.getSeaTunnelConfig().getEngineConfig().getHttpConfig().getPort(); ``` `SeaTunnelServer` holds a single `SeaTunnelConfig` instance and passes it to `new JettyService(nodeEngine, seaTunnelConfig)`, so writing the chosen port back to `HttpConfig` is enough to make it visible to the operation. The fix also hoists `seaTunnelConfig.getEngineConfig().getHttpConfig()` into a local, which the constructor was re-fetching four times. ### Does this PR introduce _any_ user-facing change? Yes, two. **1. `/logs` and `/loggers?scope=cluster` now address the right node.** `LogService#allLogNameList` walks every member, resolves its port, and issues an HTTP GET to it: ```java String url = "http://"; + host + ":" + nodeHttpPort + contextPath; ... final String nodeId = host + ":" + nodeHttpPort; ``` On a cluster where node A binds the configured port and node B falls back to a dynamic one, node B previously reported node A's port. So `GET /logs` enumerated node A twice, emitted two entries with an identical `nodeId`, pointed node B's `href` links at node A, and never listed node B's local log files. After this change each node reports the port it is actually listening on. `LoggerLevelService` resolves peer ports the same way and is fixed by the same change. **2. `HttpConfig.getPort()` now returns the bound port rather than the configured port** once dynamic-port fallback has happened. This is deliberate — it is what makes the port resolvable by peers — but it is a semantic change worth calling out for review. Two notes on it: - Nothing in the tree treats `getPort()` as "the configured value"; every non-test caller (`LogService`, `LoggerLevelService` via `GetNodeHttpPortOperation`, and the local-mode log line in `ClientExecuteCommand`) wants the bound port and is served better by this. - The write-back happens while the node is initialising, so a fan-out issued against a node that has not finished starting can still read the configured value. In practice fan-out is driven by user requests long after startup, and this is not a regression — the value was simply always wrong before. The duplicated startup log line that reported the HTTP port under the label "https port" is removed; `enableHttps` already logs the real HTTPS port at the end of the method. No configuration change, no new option. ### How was this patch tested? `RestApiIT` already builds precisely the affected topology, and the stale value was silently degrading it: - node1 does `setPort(8080)` and starts first, binding 8080; - node2 sets no port, so it inherits `port: 8080` from `seatunnel-e2e/.../test/resources/seatunnel.yaml`, and sets `setEnableDynamicPort(true)`. It therefore always falls back to another port; - `beforeClass` records `ports.put(node2HzPort, node2Config...getHttpConfig().getPort())`, which was the stale `8080`. `ports` maps the Hazelcast member port to the HTTP port. The REST v1 assertions use the key (the member port) and were unaffected, but every REST v2 assertion uses the value — so all ~15 of them addressed node1 on both loop iterations and **node2's Jetty endpoints were never exercised**. `verifyLogLink` passed for the wrong reason: node2's `href` pointed at node1, and node1 is the master, so the expected `Init JobMaster for Job fake_to_file` content was there. That means this fix makes a large amount of existing E2E coverage start doing what it was written to do, which is the real verification here. On top of that I added `testDynamicHttpPortIsResolvableByPeers`, which pins the behaviour so it cannot silently regress: - asserts the two nodes' HTTP ports differ; - asserts `GET /logs?format=JSON` reports two distinct `node` values, and that one of them carries node2's dynamic port. Both nodes run in the same JVM and share a log directory, so the file list is identical between them and the only thing distinguishing the two entries is the port — which is exactly what regressed. I also replaced `buildHttpBaseUrl(httpPorts.get(0))` in `testCheckpointOverviewAndHistoryApi` with node1's port explicitly. That line relied on `HashMap` iteration order, which was harmless while both values were `8080` and becomes load-bearing now that they differ. I do not have a JDK/Maven in my working environment, so I have not run the suite locally — CI is the first execution of these changes. Reviewers should expect this PR to newly exercise node2's REST v2 endpoints for the first time, and I will chase anything that surfaces there. ### Check list * [x] If any new Jar binary package adding in your PR, please add License Notice according [New License Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md) * [x] If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs * [x] If necessary, please update `incompatible-changes.md` to describe the incompatibility caused by this PR. * [x] If you are contributing the connector code, please check that the following files are updated: -- 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]
