PR Comments and Fixes

Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>

bundle-workflow/src/perf_test.py

@@ -6,8 +6,8 @@
 from git.git_repository import GitRepository
 from manifests.bundle_manifest import BundleManifest
 from system.working_directory import WorkingDirectory
-from test_workflow.perf_test_cluster import PerformanceTestCluster
-from test_workflow.perf_test_suite import PerformanceTestSuite
+from test_workflow.perf_test_cluster import Cluster
+from test_workflow.perf_test_suite import PerfTestSuite
 
 """
     Entry point for Performance Test with bundle manifest, config file containing the required arguments for running
@@ -39,8 +39,8 @@ def get_infra_repo_url():
         security = True
 
 with WorkingDirectory(current_workspace) as curdir:
-    with PerformanceTestCluster(manifest, config, args.stack, security).cluster() as test_cluster_endpoint:
+    with Cluster.create(manifest, config, args.stack, security) as test_cluster_endpoint:
 
         os.chdir(current_workspace)
-        perf_test_suite = PerformanceTestSuite(manifest, test_cluster_endpoint, security, current_workspace)
+        perf_test_suite = PerfTestSuite(manifest, test_cluster_endpoint, security, current_workspace)
         perf_test_suite.execute()

bundle-workflow/src/system/working_directory.py

@@ -7,12 +7,11 @@
 import os
 from contextlib import contextmanager
 
-saved_path = os.getcwd()
-
 
 @contextmanager
 def WorkingDirectory(path):
     try:
+        saved_path = os.getcwd()
         yield os.chdir(path)
     finally:
         os.chdir(saved_path)

bundle-workflow/src/test_workflow/perf_test_cluster.py

@@ -6,7 +6,18 @@
 from test_workflow.test_cluster import TestCluster
 
 
-class PerformanceTestCluster(TestCluster):
+class Cluster:
+    @contextmanager
+    def create(manifest, config, stack_name, security):
+        perf_test_cluster = PerfTestCluster(manifest, config, stack_name, security)
+        try:
+            perf_test_cluster.create()
+            yield perf_test_cluster.endpoint()
+        finally:
+            perf_test_cluster.destroy()
+
+
+class PerfTestCluster(TestCluster):
     """
     Represents a performance test cluster. This class deploys the opensearch bundle with CDK and returns the private IP.
     """
@@ -25,18 +36,22 @@ def __init__(self, bundle_manifest, config, stack_name, security):
         self.output_file = 'output.json'
         self.ip_address = None
         self.security = 'enable' if security else 'disable'
-        self.params = f'-c url={self.manifest.build.location} -c security_group_id={self.security_id} -c vpc_id={self.vpc_id}'\
-                      f' -c account_id={self.account_id} -c region={self.region} -c stack_name={self.stack_name} -c security={self.security}'\
-                      f' -c architecture={self.manifest.build.architecture} --require-approval=never --plugin cdk-assume-role-credential-plugin'\
-                      f' -c assume-role-credentials:writeIamRoleName={self.role} -c assume-role-credentials:readIamRoleName={self.role}'
-
-    @contextmanager
-    def cluster(self):
-        try:
-            self.create()
-            yield self.endpoint()
-        finally:
-            self.destroy()
+        self.params_dict = {
+            'url': self.manifest.build.location,
+            'security_group_id': self.security_id,
+            'vpc_id': self.vpc_id,
+            'account_id': self.account_id,
+            'region': self.region,
+            'stack_name': self.stack_name,
+            'security': self.security,
+            'architecture': self.manifest.build.architecture,
+        }
+        self.params_list = []
+        for key, value in self.params_dict.items():
+            self.params_list.append(f' -c {key}={value}')
+        self.role_params = f' --require-approval=never --plugin cdk-assume-role-credential-plugin'\
+                           f' -c assume-role-credentials:writeIamRoleName={self.role} -c assume-role-credentials:readIamRoleName={self.role} '
+        self.params = ''.join(self.params_list) + self.role_params
 
     def create(self):
         os.chdir(self.work_dir)

bundle-workflow/src/test_workflow/perf_test_suite.py

@@ -1,8 +1,10 @@
 import os
 import subprocess
 
+from system.working_directory import WorkingDirectory
 
-class PerformanceTestSuite:
+
+class PerfTestSuite:
     """
     Represents a performance test suite. This class runs rally test on the deployed cluster with the provided IP.
     """
@@ -18,13 +20,14 @@ def __init__(self, bundle_manifest, endpoint, security, current_workspace):
 
     def execute(self):
         try:
-            os.chdir(self.work_dir)
-            dir = os.getcwd()
-            subprocess.check_call('python3 -m pipenv install', cwd=dir, shell=True)
-            subprocess.check_call('pipenv install', cwd=dir, shell=True)
-            if self.security:
-                subprocess.check_call(f'{self.command} -s', cwd=dir, shell=True)
-            else:
-                subprocess.check_call(f'{self.command}', cwd=dir, shell=True)
+            with WorkingDirectory(self.work_dir):
+                dir = os.getcwd()
+                subprocess.check_call('python3 -m pipenv install', cwd=dir, shell=True)
+                subprocess.check_call('pipenv install', cwd=dir, shell=True)
+
+                if self.security:
+                    subprocess.check_call(f'{self.command} -s', cwd=dir, shell=True)
+                else:
+                    subprocess.check_call(f'{self.command}', cwd=dir, shell=True)
         finally:
             os.chdir(self.current_workspace)

bundle-workflow/tests/test_perf_workflow/test_perf_cluster.py

@@ -9,13 +9,18 @@
 from unittest.mock import MagicMock, patch
 
 from manifests.bundle_manifest import BundleManifest
-from test_workflow.perf_test_cluster import PerformanceTestCluster
+from test_workflow.perf_test_cluster import PerfTestCluster
 
 
-class TestPerformanceCluster(unittest.TestCase):
+class TestPerfCluster(unittest.TestCase):
     def setUp(self):
-        os.chdir(os.path.dirname(__file__))
-        self.manifest = BundleManifest.from_path("data/test_manifest.yaml")
+        self.data_path = os.path.realpath(
+            os.path.join(os.path.dirname(__file__), "data")
+        )
+        self.manifest_filename = os.path.join(
+            self.data_path, "test_manifest.yaml"
+        )
+        self.manifest = BundleManifest.from_path(self.manifest_filename)
         self.stack_name = 'stack'
         self.security = 'disable'
         config = {
@@ -27,7 +32,7 @@ def setUp(self):
                 'Role': 'role-arn'
             }
         }
-        self.perf_test_cluster = PerformanceTestCluster(
+        self.perf_test_cluster = PerfTestCluster(
             bundle_manifest=self.manifest, config=config, stack_name=self.stack_name, security=self.security
         )
 

bundle-workflow/tests/test_perf_workflow/test_perf_suite.py

@@ -9,15 +9,15 @@
 from unittest.mock import patch
 
 from manifests.bundle_manifest import BundleManifest
-from test_workflow.perf_test_suite import PerformanceTestSuite
+from test_workflow.perf_test_suite import PerfTestSuite
 
 
-class TestPerformanceSuite(unittest.TestCase):
+class TestPerfSuite(unittest.TestCase):
     def setUp(self):
         os.chdir(os.path.dirname(__file__))
         self.manifest = BundleManifest.from_path("data/test_manifest.yaml")
         self.endpoint = None
-        self.perf_test_suite = PerformanceTestSuite(
+        self.perf_test_suite = PerfTestSuite(
             bundle_manifest=self.manifest, endpoint=None, security=False, current_workspace='current_workspace'
         )