lordgamez commented on a change in pull request #995:
URL: https://github.com/apache/nifi-minifi-cpp/pull/995#discussion_r572893281



##########
File path: docker/test/integration/MiNiFi_integration_test_driver.py
##########
@@ -0,0 +1,208 @@
+from subprocess import Popen, PIPE, STDOUT
+
+import docker
+import logging
+import os
+import shutil
+import threading
+import time
+import uuid
+
+from pydoc import locate
+
+from minifi.core.InputPort import InputPort
+
+from minifi.core.DockerTestCluster import DockerTestCluster
+from minifi.core.SingleNodeDockerCluster import SingleNodeDockerCluster
+from minifi.core.DockerTestDirectoryBindings import DockerTestDirectoryBindings
+
+from minifi.validators.EmptyFilesOutPutValidator import 
EmptyFilesOutPutValidator
+from minifi.validators.NoFileOutPutValidator import NoFileOutPutValidator
+from minifi.validators.SingleFileOutputValidator import 
SingleFileOutputValidator
+
+class MiNiFi_integration_test():
+    def __init__(self, context):
+        logging.info("MiNiFi_integration_test init")
+        self.test_id = str(uuid.uuid4())
+        self.clusters = {}
+
+        self.connectable_nodes = []
+        # Remote process groups are not connectables
+        self.remote_process_groups = []
+        self.file_system_observer = None
+
+        self.docker_network = None
+
+        self.docker_directory_bindings = DockerTestDirectoryBindings()
+        
self.docker_directory_bindings.create_new_data_directories(self.test_id)
+
+    def __del__(self):
+        logging.info("MiNiFi_integration_test cleanup")
+
+        # Clean up network, for some reason only this order of events work for 
cleanup
+        if self.docker_network is not None:
+            logging.info('Cleaning up network network: %s', 
self.docker_network.name)
+            while len(self.docker_network.containers) != 0:
+                for container in self.docker_network.containers:
+                    self.docker_network.disconnect(container, force=True)
+                self.docker_network.reload()
+            self.docker_network.remove()
+
+        container_ids = []
+        for cluster in self.clusters.values():
+            for container in cluster.containers.values():
+                container_ids.append(container.id)
+            del cluster
+
+        # Backup for cleaning up containers as the cluster deleter is not 
reliable
+        docker_client = docker.from_env()
+        for container_id in container_ids:    
+            wait_start_time = time.perf_counter()
+            while (time.perf_counter() - wait_start_time) < 35:
+                # There is no clean way to check for container existence
+                try:
+                    container = docker_client.containers.get(container_id)
+                    logging.error("Failure when trying to clean up containers. 
Attempting secondary cleanup.")
+                    container.kill()
+                    time.sleep(5)
+                    container.remove(v=True, force=True)
+                    time.sleep(5)
+                except docker.errors.NotFound:
+                    break
+            try:
+                container = docker_client.containers.get(container_id)
+                logging.error("All attempts to clean up docker containers were 
unsuccessful.")
+            except docker.errors.NotFound:
+                logging.info("Docker container secondary cleanup successful.")
+                pass

Review comment:
       I think it is better to encapsulate it, with the retries as well. It 
seems that the only thing we do with the result is write a log entry about it. 
That could be merged in the function, and we do not need a return value either. 
The first log entry could be changed to a warn as it does not show an error 
here. I changed the message a bit as we do not talk about the cleanup of all 
the containers but a single container.
   
   ```python
   def delete_docker_container_by_id(container_id):
       try:
           container = docker_client.containers.get(container_id)
           container.remove(v=True, force=True)
       except docker.errors.NotFound:
           logging.warn("Contaner '%s' is already cleaned up before.", 
container_id)
           return
       while (time.perf_counter() - wait_start_time) < 35:
           try:
               docker_client.containers.get(container_id)
               logging.error("Docker container '%s' still exists after remove. 
Waiting for docker daemon...", container_id)
               time.sleep(5)
           except docker.errors.NotFound:
               logging.info("Docker container cleanup successful for '%s'.", 
container_id)
               return
       logging.error("Failed to clean up docker container '%s'.", container_id)
   ```




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to