You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/01/27 13:30:23 UTC

[GitHub] [ignite] Vladsz83 opened a new pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Vladsz83 opened a new pull request #8714:
URL: https://github.com/apache/ignite/pull/8714


   Discovery ducktape test lacks simulation of partial network failure.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8714:
URL: https://github.com/apache/ignite/pull/8714#discussion_r566756472



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -33,13 +34,22 @@
 from ignitetest.services.utils.ignite_spec import resolve_spec
 from ignitetest.services.utils.jmx_utils import ignite_jmx_mixin
 from ignitetest.services.utils.log_utils import monitor_log
+from ignitetest.utils.enum import constructible
 
 
 # pylint: disable=too-many-public-methods
 class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABCMeta):
     """
     The base class to build services aware of Ignite.
     """
+    @constructible
+    class NetPart(IntEnum):
+        """
+        Network part to emulate failure.
+        """
+        INCOMING = 0
+        OUTCOMING = 1

Review comment:
       Fixed




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8714:
URL: https://github.com/apache/ignite/pull/8714#discussion_r566756898



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -33,13 +34,22 @@
 from ignitetest.services.utils.ignite_spec import resolve_spec
 from ignitetest.services.utils.jmx_utils import ignite_jmx_mixin
 from ignitetest.services.utils.log_utils import monitor_log
+from ignitetest.utils.enum import constructible
 
 
 # pylint: disable=too-many-public-methods
 class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABCMeta):
     """
     The base class to build services aware of Ignite.
     """
+    @constructible
+    class NetPart(IntEnum):
+        """
+        Network part to emulate failure.
+        """
+        INCOMING = 0
+        OUTCOMING = 1
+        ALL = 2

Review comment:
       There is not 'ALL'. And we are not interrested in separated FORWARD.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8714:
URL: https://github.com/apache/ignite/pull/8714#discussion_r566757016



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -318,15 +333,17 @@ def drop_network(self, nodes=None):
         dsc_ports = str(dsc_spi.port) if not hasattr(dsc_spi, 'port_range') or dsc_spi.port_range < 1 else str(
             dsc_spi.port) + ':' + str(dsc_spi.port + dsc_spi.port_range)
 
-        cmd = f"sudo iptables -I %s 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP"
+        if net_part in (IgniteAwareService.NetPart.ALL, IgniteAwareService.NetPart.INCOMING):
+            node.account.ssh_client.exec_command(
+                f"sudo iptables -I INPUT 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP")
+            node.account.ssh_client.exec_command(
+                f"sudo iptables -I FORWARD 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP")

Review comment:
       Fixed




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8714:
URL: https://github.com/apache/ignite/pull/8714#discussion_r566745746



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -169,6 +177,9 @@ def _perform_node_fail_scenario(self, test_config):
             if LATEST_2_7 < test_config.version <= V_2_9_0:
                 discovery_spi.so_linger = 0
 
+            if test_config.disable_conn_recovery:
+                discovery_spi.connRecoveryTimeout = 0

Review comment:
       any guarantee it's not null before? :)
   should we use explicit connRecoveryTimeout instead?

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -33,13 +34,22 @@
 from ignitetest.services.utils.ignite_spec import resolve_spec
 from ignitetest.services.utils.jmx_utils import ignite_jmx_mixin
 from ignitetest.services.utils.log_utils import monitor_log
+from ignitetest.utils.enum import constructible
 
 
 # pylint: disable=too-many-public-methods
 class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABCMeta):
     """
     The base class to build services aware of Ignite.
     """
+    @constructible
+    class NetPart(IntEnum):
+        """
+        Network part to emulate failure.
+        """
+        INCOMING = 0
+        OUTCOMING = 1
+        ALL = 2

Review comment:
       Better case is just to use iptables set (input, forward, output) I think.

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -33,13 +34,22 @@
 from ignitetest.services.utils.ignite_spec import resolve_spec
 from ignitetest.services.utils.jmx_utils import ignite_jmx_mixin
 from ignitetest.services.utils.log_utils import monitor_log
+from ignitetest.utils.enum import constructible
 
 
 # pylint: disable=too-many-public-methods
 class IgniteAwareService(BackgroundThreadService, IgnitePathAware, metaclass=ABCMeta):
     """
     The base class to build services aware of Ignite.
     """
+    @constructible
+    class NetPart(IntEnum):
+        """
+        Network part to emulate failure.
+        """
+        INCOMING = 0
+        OUTCOMING = 1

Review comment:
       OUTGOING?

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -318,15 +333,17 @@ def drop_network(self, nodes=None):
         dsc_ports = str(dsc_spi.port) if not hasattr(dsc_spi, 'port_range') or dsc_spi.port_range < 1 else str(
             dsc_spi.port) + ':' + str(dsc_spi.port + dsc_spi.port_range)
 
-        cmd = f"sudo iptables -I %s 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP"
+        if net_part in (IgniteAwareService.NetPart.ALL, IgniteAwareService.NetPart.INCOMING):
+            node.account.ssh_client.exec_command(
+                f"sudo iptables -I INPUT 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP")
+            node.account.ssh_client.exec_command(
+                f"sudo iptables -I FORWARD 1 -p tcp -m multiport --dport {dsc_ports},{cm_ports} -j DROP")

Review comment:
       Why forward is incoming while it's outcoming as well?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] anton-vinogradov merged pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
anton-vinogradov merged pull request #8714:
URL: https://github.com/apache/ignite/pull/8714


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Vladsz83 commented on a change in pull request #8714: IGNITE-14054 : Improve discovery ducktest: add partial network drop.

Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #8714:
URL: https://github.com/apache/ignite/pull/8714#discussion_r566757766



##########
File path: modules/ducktests/tests/ignitetest/tests/discovery_test.py
##########
@@ -169,6 +177,9 @@ def _perform_node_fail_scenario(self, test_config):
             if LATEST_2_7 < test_config.version <= V_2_9_0:
                 discovery_spi.so_linger = 0
 
+            if test_config.disable_conn_recovery:
+                discovery_spi.connRecoveryTimeout = 0

Review comment:
       null is checked in the template. pulint doesn't like many params. Gotta be disabled every time. Think it is ok for simple config.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org