wernerdv commented on code in PR #13540:
URL: https://github.com/apache/ignite/pull/13540#discussion_r4025312136


##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/testframework/testcontainers/IgniteClusterContainer.java:
##########
@@ -30,24 +31,66 @@ public class IgniteClusterContainer implements Startable {
     private final List<IgniteContainer> containers;
 
     /** Network. */
-    private final Network net = Network.newNetwork();
+    protected final Network net = Network.newNetwork();
+
+    /** Image name. */
+    protected final String imageName;
+
+    /** Consistent ID's. */
+    protected final List<String> consistentIds;
+
+    /** Whether the cluster has been started, guarding against a second {@link 
#start()}. */
+    private boolean started;
 
     /**
      * @param imageName Image name.
      * @param consistentIds Consistent ID's.
      */
-    public IgniteClusterContainer(String imageName, List<String> 
consistentIds) throws Exception {
+    public IgniteClusterContainer(String imageName, List<String> 
consistentIds) {
+        this.imageName = imageName;
+        this.consistentIds = consistentIds;
+
         containers = new ArrayList<>(consistentIds.size());
+    }
+
+    /**
+     * Factory hook for the node container. Overrides only receive {@code 
idx}; the image name, network and
+     * consistent IDs are instance fields (see {@link #imageName}, {@link 
#net}, {@link #consistentIds}).
+     *
+     * @param idx Node index.
+     * @return The node container.
+     */
+    protected IgniteContainer container(int idx) throws Exception {
+        return new IgniteContainer(imageName, net, "node" + (1 + idx), 
consistentIds.get(idx), idx);
+    }
 
+    /** Builds the node containers. */
+    protected void initContainers() throws Exception {
         for (int i = 0; i < consistentIds.size(); i++)
-            containers.add(new IgniteContainer(imageName, net, "node" + (1 + 
i), consistentIds.get(i), i));
+            containers.add(container(i));
     }
 
     /** {@inheritDoc} */
     @Override public void start() {
+        // Idempotent: either the cluster already started successfully, or 
container creation succeeded
+        // but startup (deepStart/activateCluster) failed on a previous 
attempt — in both cases the
+        // containers list is already populated and must not be built a second 
time (duplicate hostnames,
+        // consistent IDs and fixed host ports would make the baseline 
unreachable).
+        if (started || !containers.isEmpty())
+            return;

Review Comment:
   Good catch, fixed.
   
   Now idempotency only applies to a fully started cluster, and a half-started 
cluster fails loudly instead of being silently skipped.



-- 
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]

Reply via email to