This is an automated email from the ASF dual-hosted git repository.

dsen pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/branch-feature-AMBARI-14714 by 
this push:
     new d7bcc61  [AMBARI-23121] Fix few instance manager issues found during 
ZK deploment (dsen) (#518)
d7bcc61 is described below

commit d7bcc61b68539612c4936669b305223b91623ad4
Author: Dmitry Sen <d...@apache.org>
AuthorDate: Fri Mar 2 15:30:14 2018 +0200

    [AMBARI-23121] Fix few instance manager issues found during ZK deploment 
(dsen) (#518)
    
    * [AMBARI-23121] Fix few instance manager issues found during ZK deployment 
(dsen)
    
    * [AMBARI-23121] Fix few instance manager issues found during ZK deployment 
(dsen) - changes according to review
---
 .../python/instance_manager/instance_manager.py    | 45 +++++++++++++++-------
 .../instance_manager/mpack-instance-manager.py     | 17 ++++++--
 .../instance_manager/test_instance_manager.py      | 43 ++++++++++++++-------
 3 files changed, 74 insertions(+), 31 deletions(-)

diff --git 
a/mpack-instance-manager/src/main/python/instance_manager/instance_manager.py 
b/mpack-instance-manager/src/main/python/instance_manager/instance_manager.py
index 6682234..c9f610b 100644
--- 
a/mpack-instance-manager/src/main/python/instance_manager/instance_manager.py
+++ 
b/mpack-instance-manager/src/main/python/instance_manager/instance_manager.py
@@ -51,8 +51,8 @@ def create_mpack(mpack_name, mpack_version, mpack_instance, 
subgroup_name=DEFAUL
   OR
   list of 'components_instances_name' to be created. (default) OR '*' for all 
components
   """
-  mpack_name = mpack_name.lower()
-  module_name = module_name.lower()
+  mpack_name, module_name, components, components_map = normalize_parameters(
+    mpack_name, module_name, components, components_map)
 
   validate_mpack_for_creation_or_changing(mpack_name, mpack_version, 
module_name, components, components_map)
 
@@ -71,14 +71,17 @@ def set_mpack_instance(mpack, mpack_version, 
mpack_instance, subgroup_name=DEFAU
   OR
   list of 'components_instances_name' to be created. (default) OR '*' for all 
components
   """
-  mpack = mpack.lower()
-  module_name = module_name.lower()
+  mpack, module_name, components, components_map = normalize_parameters(
+    mpack, module_name, components, components_map)
 
   instances = 
MpackInstance.parse_instances_with_filtering(os.path.join(ROOT_FOLDER_PATH, 
INSTANCES_FOLDER_NAME), mpack,
                                                            mpack_instance, 
subgroup_name, module_name,
                                                            components, 
components_map)
   if not instances:
-    raise ValueError("Found no instances for the given filters.")
+    raise ValueError("Found no instances for the given filters: 
mpack_name:{0}, instance_name:{1},"
+                     " subgroup_name:{2}, module_name:{3}, components:{4}, 
components_map:{5}".
+                     format(mpack, mpack_instance, subgroup_name, module_name,
+                            components, components_map))
 
   validate_mpack_for_creation_or_changing(mpack, mpack_version, module_name, 
components, components_map)
 
@@ -142,11 +145,8 @@ def build_granular_json_with_filtering(mpack_name_filter, 
instance_name_filter,
 
   The output_conf_dir or output_path for each component instance will be 
included in json depending on given parameters.
   """
-
-  if mpack_name_filter:
-    mpack_name_filter = mpack_name_filter.lower()
-  if module_name_filter:
-    module_name_filter = module_name_filter.lower()
+  mpack_name_filter, module_name_filter, skipped, components_name_filter_map = 
normalize_parameters(
+    mpack_name_filter, module_name_filter, [], components_name_filter_map)
 
   instances = 
MpackInstance.parse_instances_with_filtering(os.path.join(ROOT_FOLDER_PATH, 
INSTANCES_FOLDER_NAME),
                                                            mpack_name_filter,
@@ -155,7 +155,10 @@ def build_granular_json_with_filtering(mpack_name_filter, 
instance_name_filter,
                                                            None,
                                                            
components_name_filter_map)
   if not instances:
-    raise ValueError("Found no instances for the given filters.")
+    raise ValueError("Found no instances for the given filters: 
mpack_name:{0}, instance_name:{1},"
+                     " subgroup_name:{2}, module_name:{3}, 
components_name_map:{4}".
+                     format(mpack_name_filter, instance_name_filter, 
subgroup_name_filter,
+                            module_name_filter, components_name_filter_map))
 
   full_json_output = build_json_output(instances, 
output_conf_dir=output_conf_dir, output_path=output_path)
 
@@ -229,6 +232,22 @@ def build_json_output(instances, output_conf_dir=False, 
output_path=False):
   return {'mpacks': result}
 
 
+def normalize_parameters(mpack_name, module_name, components, components_map):
+  """
+  normalize parameters to be lowercase, for components_map dictionary only 
keys are normalized
+  """
+  if mpack_name:
+    mpack_name = mpack_name.lower()
+  if module_name:
+    module_name = module_name.lower()
+  if components and components != "*":
+    components = [component.lower() for component in components]
+  if components_map:
+    components_map = dict((k.lower(), v) for k, v in 
components_map.iteritems())
+
+  return mpack_name, module_name, components, components_map
+
+
 def validate_mpack_for_creation_or_changing(mpack_name, mpack_version, 
module_name, components, components_map):
   mpack_root_path = os.path.join(ROOT_FOLDER_PATH, MPACKS_FOLDER_NAME, 
mpack_name)
   if not os.path.exists(mpack_root_path):
@@ -478,7 +497,7 @@ class ModuleInstance(Instance):
                 (components_name_filter_map and folder_name in 
components_name_filter_map)):
             components_map = 
ComponentInstance(name=DEFAULT_COMPONENT_INSTANCE_NAME,
                                                
component_path=os.path.join(path, folder_name),
-                                               path_exec=os.path.realpath(
+                                               path_exec=os.readlink(
                                                  os.path.join(path, 
folder_name, CURRENT_SOFTLINK_NAME)))
         else:
           components_map = 
ComponentInstance.parse_into_components_dict(os.path.join(path, folder_name),
@@ -566,7 +585,7 @@ class ComponentInstance(Instance):
       if not component_names_filter or component_instance_name in 
component_names_filter:
         result[component_instance_name] = 
ComponentInstance(name=component_instance_name,
                                                             
component_path=os.path.join(path, component_instance_name),
-                                                            
path_exec=os.path.realpath(
+                                                            
path_exec=os.readlink(
                                                               
os.path.join(path, component_instance_name,
                                                                            
CURRENT_SOFTLINK_NAME)))
     return result
diff --git 
a/mpack-instance-manager/src/main/python/instance_manager/mpack-instance-manager.py
 
b/mpack-instance-manager/src/main/python/instance_manager/mpack-instance-manager.py
index 38c29f8..f9458b1 100644
--- 
a/mpack-instance-manager/src/main/python/instance_manager/mpack-instance-manager.py
+++ 
b/mpack-instance-manager/src/main/python/instance_manager/mpack-instance-manager.py
@@ -56,8 +56,14 @@ def check_required_options(options, action, parser):
     if not options.module_name:
       missing_options.append("module-name")
 
-  if missing_options:
-    parser.error("Missing following required command options: 
{0}".format(missing_options))
+    if missing_options:
+      parser.error("Missing following required command options: 
{0}".format(missing_options))
+
+    if not options.components_map and not options.components:
+      parser.error("Either components or components-map option must be 
specified.")
+
+    if options.components_map and options.components:
+      parser.error("Only components or components-map option could be 
specified. Can't use both.")
 
 
 def init_create_parser_options(parser):
@@ -117,8 +123,11 @@ def main(options, args):
   parsed_components = None
   parsed_components_map = None
   try:
-    if hasattr(options, 'components') and options.components and 
options.components != '*':
-      parsed_components = ast.literal_eval(options.components)
+    if hasattr(options, 'components') and options.components:
+      if options.components == '*':
+        parsed_components = '*'
+      else:
+        parsed_components = ast.literal_eval(options.components)
     if options.components_map:
       parsed_components_map = ast.literal_eval(options.components_map)
   except ValueError:
diff --git 
a/mpack-instance-manager/src/test/python/instance_manager/test_instance_manager.py
 
b/mpack-instance-manager/src/test/python/instance_manager/test_instance_manager.py
index 5315094..0d834cc 100644
--- 
a/mpack-instance-manager/src/test/python/instance_manager/test_instance_manager.py
+++ 
b/mpack-instance-manager/src/test/python/instance_manager/test_instance_manager.py
@@ -84,7 +84,7 @@ class TestInstanceManager(TestCase):
                                                 
instance_manager.DEFAULT_MPACK_INSTANCE_NAME)))
 
   def test_create_mpack_server_module_with_multiple_component_instances(self):
-    create_mpack_with_defaults(components=None, 
components_map={SERVER_COMPONENT_NAME: ['server1', 'server2']})
+    create_mpack_with_defaults(components=None, 
components_map={SERVER_COMPONENT_NAME.upper(): ['server1', 'server2']})
 
     current_link_1 = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.INSTANCES_FOLDER_NAME, MPACK_NAME,
                                   INSTANCE_NAME_1, SUBGROUP_NAME, 
SERVER_MODULE_NAME, SERVER_COMPONENT_NAME,
@@ -121,12 +121,12 @@ class TestInstanceManager(TestCase):
                                   MPACK_VERSION_2, SERVER_COMPONENT_NAME))
 
   def test_set_version_client_module_asterisk(self):
-    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME)
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME.upper())
 
     build_rpm_structure(mpack_version=MPACK_VERSION_2, 
remove_old_content=False, create_modules=False)
 
-    instance_manager.set_mpack_instance(MPACK_NAME, MPACK_VERSION_2, 
INSTANCE_NAME_1, SUBGROUP_NAME,
-                                        CLIENT_MODULE_NAME, '*')
+    instance_manager.set_mpack_instance(MPACK_NAME.upper(), MPACK_VERSION_2, 
INSTANCE_NAME_1, SUBGROUP_NAME,
+                                        CLIENT_MODULE_NAME.upper(), '*')
 
     current_link = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.INSTANCES_FOLDER_NAME, MPACK_NAME,
                                 INSTANCE_NAME_1, SUBGROUP_NAME, 
CLIENT_COMPONENT_NAME,
@@ -160,9 +160,9 @@ class TestInstanceManager(TestCase):
                                   MPACK_VERSION_2, SERVER_COMPONENT_NAME))
 
   def test_get_conf_dir_all(self):
-    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME)
-    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME, components=None,
-                               components_map={SERVER_COMPONENT_NAME: 
['server1']})
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME.upper())
+    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME.upper(), 
components=None,
+                               components_map={SERVER_COMPONENT_NAME.upper(): 
['server1']})
 
     conf_dir_json = instance_manager.get_conf_dir()
 
@@ -210,9 +210,9 @@ class TestInstanceManager(TestCase):
     self.assertEqual(conf_dir_json, expected_json)
 
   def test_list_instances_all(self):
-    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME)
-    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME, components=None,
-                               components_map={SERVER_COMPONENT_NAME: 
['server1']})
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME.upper())
+    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME.upper(), 
components=None,
+                               components_map={SERVER_COMPONENT_NAME.upper(): 
['server1']})
 
     conf_dir_json = instance_manager.list_instances()
 
@@ -232,7 +232,7 @@ class TestInstanceManager(TestCase):
                         "hdfs_server": {
                           "component-instances": {
                             "server1": {
-                              "path": 
"/tmp/instance_manager_test/modules/hdfs/3.1.0.0-b1",
+                              "path": 
"/tmp/instance_manager_test/mpacks/hdpcore/1.0.0-b1/hdfs_server",
                               "name": "server1"
                             }
                           }
@@ -243,7 +243,7 @@ class TestInstanceManager(TestCase):
                       "category": "CLIENT",
                       "component_instances": {
                         "default": {
-                          "path": 
"/tmp/instance_manager_test/modules/hdfs-clients/3.1.0.0-b1",
+                          "path": 
"/tmp/instance_manager_test/mpacks/hdpcore/1.0.0-b1/hdfs_client",
                           "name": "default"
                         }
                       },
@@ -317,7 +317,7 @@ class TestInstanceManager(TestCase):
     expected_filter_result = {'mpacks': {'edw': {'mpack-instances': 
{'eCommerce': {'name': 'eCommerce', 'subgroups': {
       'default': {'modules': {'hdfs': {'category': 'SERVER', 'name': 'hdfs', 
'components': {'hdfs_server': {
         'component-instances': {
-          'server2': {'path': 
'/tmp/instance_manager_test/modules/hdfs/3.1.0.0-b1', 'name': 
'server2'}}}}}}}}}}}}}
+          'server2': {'path': 
'/tmp/instance_manager_test/mpacks/edw/1.0.0-b1/hdfs_server', 'name': 
'server2'}}}}}}}}}}}}}
     self.assertEquals(expected_filter_result, 
filter_by_component_instance_name_json)
 
   def test_validation(self):
@@ -363,6 +363,19 @@ class TestInstanceManager(TestCase):
                         "The instance 
/tmp/instance_manager_test/instances/hdpcore/Production/default/hdfs/"
                         "hdfs_server/default already exist. To change the 
version use set-mpack-instance command")
 
+  def test_normalize_parameters(self):
+    mpack_name = MPACK_NAME.upper()
+    module_name = SERVER_MODULE_NAME.upper()
+    components = [SERVER_COMPONENT_NAME.upper()]
+    components_map = {SERVER_COMPONENT_NAME.upper(): ["DEFAULT"]}
+    mpack_name, module_name, components, components_map = 
instance_manager.normalize_parameters(
+      mpack_name, module_name, components, components_map)
+
+    self.assertEquals(mpack_name, MPACK_NAME.lower())
+    self.assertEquals(module_name, SERVER_MODULE_NAME.lower())
+    self.assertEquals(components, [SERVER_COMPONENT_NAME.lower()])
+    self.assertEquals(components_map, {SERVER_COMPONENT_NAME.lower(): 
["DEFAULT"]})
+
   def test_set_non_existing_instance(self):
     try:
       instance_manager.set_mpack_instance(mpack=MPACK_NAME, 
mpack_version=MPACK_VERSION_1,
@@ -383,7 +396,9 @@ class TestInstanceManager(TestCase):
       raise AssertionError("The previous call should have thrown exception")
     except ValueError as e:
       self.assertEquals(e.message,
-                        "Found no instances for the given filters.")
+                        "Found no instances for the given filters: 
mpack_name:hdpcore, instance_name:Production,"
+                        " subgroup_name:default, module_name:hdfs, 
components:None,"
+                        " components_map:{'hdfs_server': 
['non-existing-instance']}")
 
 
 def create_mpack_with_defaults(mpack_name=MPACK_NAME, 
mpack_version=MPACK_VERSION_1, mpack_instance=INSTANCE_NAME_1,

-- 
To stop receiving notification emails like this one, please contact
d...@apache.org.

Reply via email to