KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape #4195

cmccabe · 2017-11-08T18:32:29Z

For ducktape: add Kibosh to the testing Dockerfile.
Create files_unreadable_fault_spec.py.

For trogdor: create FilesUnreadableFaultSpec.java.
Add a unit test of using the Kibosh service.

asfgit · 2017-11-08T20:01:21Z

SUCCESS
8057 tests run, 5 skipped, 0 failed.
--none--

asfgit · 2017-11-08T20:11:40Z

SUCCESS
8057 tests run, 5 skipped, 0 failed.
--none--

asfgit · 2017-11-08T20:25:14Z

FAILURE
7949 tests run, 5 skipped, 1 failed.
--none--

rajinisivaram

The new system tests failed for me. I will check the results to see if I have missed some step. But just wanted to check if they worked for you. Apart from that I have just left some minor comments.

rajinisivaram · 2017-11-14T17:29:39Z

tests/kafkatest/services/trogdor/kibosh.py

+        :param nodes:               The nodes to put the Kibosh FS on.  Kibosh allocates no
+                                    nodes of its own.
+        :param target:              The target directory, which Kibosh exports a view of.
+        :param mirror:              The mirror direcotry, where Kibosh injects faults.


typo: directory

rajinisivaram · 2017-11-14T17:35:30Z

tests/kafkatest/services/trogdor/kibosh.py

+            util.wait_until(lambda: self.kibosh_process_running(node), 20, backoff_sec=.1,
+                            err_msg="Timed out waiting for kibosh to stop on %s" % node.account.hostname)
+        except TimeoutError:
+            # If the prcess won't terminate, use kill -9 to shut it down.


type in comment: process

rajinisivaram · 2017-11-14T17:37:13Z

tests/kafkatest/tests/tools/kibosh_test.py

+        node = self.nodes[0]
+
+        def check(self, node):
+            self.logger.info("WATERMELON")


Is the log entry as marker that something checks?

Oops, this was supposed to be removed.

rajinisivaram · 2017-11-14T17:37:48Z

tests/kafkatest/tests/tools/kibosh_test.py

+            return fault_json == expected_json
+
+        self.kibosh.set_faults(node, [spec])
+        #time.sleep(30 * 60 * 1000)


Was the sleep temporary?

Yes, let me remove that

rajinisivaram · 2017-11-14T17:39:04Z

tests/kafkatest/tests/tools/trogdor_test.py

@@ -96,3 +96,32 @@ def verify_nodes_partitioned():
            raise RuntimeError("Node 2 must be reachable from node 1.")
        if not node_is_reachable(self.agent_nodes[2], self.agent_nodes[1]):
            raise RuntimeError("Node 1 must be reachable from node 2.")
+
+    @cluster(num_nodes=4)
+    def test_files_unreadable_fault(self):


This test seems to be testing network partition rathter than disk error?

rajinisivaram · 2017-11-14T17:42:21Z

tools/src/main/java/org/apache/kafka/trogdor/fault/Kibosh.java

+import java.util.Objects;
+import java.util.TreeMap;
+
+public enum Kibosh {


Why is this an enum?

It's a singleton implemented as a Java enum. http://keaplogik.blogspot.com/2013/12/the-java-enum-singleton-pattern.html

Hmm... Maybe it should be called KiboshSingleton if it really needs to be an enum. Personally, it feels like an odd use of enum to me. Another opinion would help - @ijuma ?

I changed it to a regular class

rajinisivaram · 2017-11-14T17:45:40Z

tools/src/main/java/org/apache/kafka/trogdor/fault/Kibosh.java

+
+        synchronized void removeFault(KiboshFaultSpec toRemove) throws IOException {
+            KiboshControlFile file = KiboshControlFile.read(controlPath);
+            List<KiboshFaultSpec> newFaults = new ArrayList<>();


Not really new faults?

Hmm, I can just call it "faults." That's probably clearer

asfgit · 2017-11-14T18:42:11Z

FAILURE
4020 tests run, 1 skipped, 1 failed.
--none--

asfgit · 2017-11-14T19:38:26Z

FAILURE
8068 tests run, 5 skipped, 2 failed.
--none--

asfgit · 2017-11-14T20:05:10Z

FAILURE
7960 tests run, 5 skipped, 1 failed.
--none--

rajinisivaram · 2017-11-15T18:37:39Z

tests/kafkatest/services/trogdor/kibosh.py

+
+    def pids(self, node):
+        return [pid for pid in node.account.ssh_capture("test -e '%s' && test -e /proc/$(cat '%s')" %
+                                                        (self.pidfile_path, self.pidfile_path), allow_fail=True)]


Is the support for multiple pids only for clean up? Since the code checks for a fixed control file, I was thinking we expect only one process at a time.

You are right that we expect only one process here. The reason it's plural is that there's a pattern in the ducktape code of having a pids function for Service subclasses which returns an array of ints, or an empty array if there are no processes. For example, zookeeper.py, kafka.py, mirror_maker.py, streams.py, etc. all follow this pattern.

asfgit · 2017-11-15T19:58:39Z

FAILURE
8068 tests run, 5 skipped, 2 failed.
--none--

asfgit · 2017-11-15T20:07:04Z

FAILURE
8068 tests run, 5 skipped, 2 failed.
--none--

rajinisivaram · 2017-11-15T19:58:00Z

tests/kafkatest/services/trogdor/files_unreadable_fault_spec.py

+    def message(self):
+        return {
+            "class": "org.apache.kafka.trogdor.fault.FilesUnreadableFaultSpec",
+            "startMs": self.start_ms,


durationMs not specified?

asfgit · 2017-11-15T20:15:48Z

FAILURE
7960 tests run, 5 skipped, 1 failed.
--none--

For ducktape: add Kibosh to the testing Dockerfile. Create files_unreadable_fault_spec.py. For trogdor: create FilesUnreadableFaultSpec.java. Add a unit test of using the Kibosh service.

rajinisivaram

LGTM

asfgit · 2017-11-16T18:35:25Z

SUCCESS
8072 tests run, 5 skipped, 0 failed.
--none--

asfgit · 2017-11-16T19:01:01Z

FAILURE
7964 tests run, 5 skipped, 1 failed.
--none--

asfgit · 2017-11-16T19:12:56Z

SUCCESS
8072 tests run, 5 skipped, 0 failed.
--none--

dkuritsyn · 2019-06-18T14:42:56Z

tools/src/main/java/org/apache/kafka/trogdor/fault/Kibosh.java

+        }
+
+        @JsonProperty
+        public int errorCode() {


As I can see here https://github.com/confluentinc/kibosh/blob/b4288fa98d637f3765dc2abe848d18fce5afa897/fault.c#L41
the parameter name should be 'code', not 'errorCode'. Do you have this fault actually working?

rajinisivaram reviewed Nov 14, 2017

View reviewed changes

cmccabe force-pushed the KAFKA-5811 branch from 52b8f6d to cc48feb Compare November 14, 2017 18:33

rajinisivaram reviewed Nov 15, 2017

View reviewed changes

cmccabe force-pushed the KAFKA-5811 branch from cc48feb to 8871bc0 Compare November 15, 2017 18:52

rajinisivaram reviewed Nov 15, 2017

View reviewed changes

KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape

dfdd671

For ducktape: add Kibosh to the testing Dockerfile. Create files_unreadable_fault_spec.py. For trogdor: create FilesUnreadableFaultSpec.java. Add a unit test of using the Kibosh service.

cmccabe force-pushed the KAFKA-5811 branch from 8871bc0 to dfdd671 Compare November 16, 2017 17:29

rajinisivaram approved these changes Nov 16, 2017

View reviewed changes

asfgit closed this in d9cbc6b Nov 16, 2017

cmccabe deleted the KAFKA-5811 branch May 20, 2019 19:07

dkuritsyn reviewed Jun 18, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape #4195

KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape #4195

cmccabe commented Nov 8, 2017

asfgit commented Nov 8, 2017

asfgit commented Nov 8, 2017

asfgit commented Nov 8, 2017

rajinisivaram left a comment

rajinisivaram Nov 14, 2017

rajinisivaram Nov 14, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 14, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 14, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 14, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 14, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 15, 2017

rajinisivaram Nov 14, 2017

cmccabe Nov 14, 2017

asfgit commented Nov 14, 2017

asfgit commented Nov 14, 2017

asfgit commented Nov 14, 2017

rajinisivaram Nov 15, 2017

cmccabe Nov 15, 2017

asfgit commented Nov 15, 2017

asfgit commented Nov 15, 2017

rajinisivaram Nov 15, 2017

asfgit commented Nov 15, 2017

rajinisivaram left a comment

asfgit commented Nov 16, 2017

asfgit commented Nov 16, 2017

asfgit commented Nov 16, 2017

dkuritsyn Jun 18, 2019 •

edited

KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape #4195

KAFKA-5811: Add Kibosh integration for Trogdor and Ducktape #4195

Conversation

cmccabe commented Nov 8, 2017

asfgit commented Nov 8, 2017

asfgit commented Nov 8, 2017

asfgit commented Nov 8, 2017

rajinisivaram left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Nov 14, 2017

asfgit commented Nov 14, 2017

asfgit commented Nov 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Nov 15, 2017

asfgit commented Nov 15, 2017

Choose a reason for hiding this comment

asfgit commented Nov 15, 2017

rajinisivaram left a comment

Choose a reason for hiding this comment

asfgit commented Nov 16, 2017

asfgit commented Nov 16, 2017

asfgit commented Nov 16, 2017

dkuritsyn Jun 18, 2019 • edited

Choose a reason for hiding this comment

dkuritsyn Jun 18, 2019 •

edited