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

commit 7c4070ca48a0a8d5a42431d42bc04fde9bda65f3
Author: Dmytro Sen <d...@apache.org>
AuthorDate: Thu Feb 22 17:31:15 2018 +0200

    Changes according to the review + general improvements
---
 mpack-instance-manager/pom.xml                     |   2 +-
 .../python/instance_manager/instance_manager.py    | 233 ++++++++++++++++-----
 .../instance_manager/mpack-instance-manager.py     |  51 ++++-
 .../instance_manager/test_instance_manager.py      | 175 +++++++++++++++-
 4 files changed, 389 insertions(+), 72 deletions(-)

diff --git a/mpack-instance-manager/pom.xml b/mpack-instance-manager/pom.xml
index 3ca6dd4..8df08fa 100644
--- a/mpack-instance-manager/pom.xml
+++ b/mpack-instance-manager/pom.xml
@@ -29,7 +29,7 @@
     <artifactId>mpack-instance-manager</artifactId>
     <version>2.0.0.0-SNAPSHOT</version>
     <name>Mpack instance manager</name>
-    <description>Mpack instance manager</description>
+    <description>Mpack Instance Manager</description>
     <properties>
         
<instanceManagerInstallDir>/usr/lib/mpack-instance-manager</instanceManagerInstallDir>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
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 6f27ebc..6682234 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
@@ -27,16 +27,19 @@ MPACK_JSON_FILE_NAME = 'mpack.json'
 CURRENT_SOFTLINK_NAME = 'current'
 CONFIGS_DIRECTORY_NAME = 'conf'
 
-ROOT_FOLDER_PATH = "/opt/odp"
+ROOT_FOLDER_PATH = "/usr/hwx/"
 
 INSTANCES_FOLDER_NAME = "instances"
 MODULES_FOLDER_NAME = "modules"
 MPACKS_FOLDER_NAME = "mpacks"
 DEFAULT_COMPONENT_INSTANCE_NAME = 'default'
 DEFAULT_MPACK_INSTANCE_NAME = 'default'
+DEFAULT_SUBGROUP_NAME = 'default'
+CLIENT_CATEGORY = "CLIENT"
+SERVER_CATEGORY = "SERVER"
 
 
-def create_mpack(mpack_name, mpack_version, mpack_instance, subgroup_name, 
module_name,
+def create_mpack(mpack_name, mpack_version, mpack_instance, 
subgroup_name=DEFAULT_SUBGROUP_NAME, module_name=None,
                  components=None, components_map=None):
   """
   Use case 1: Creates an instance of mpack with a new subgroup, new module and 
one or more component(s)
@@ -48,11 +51,16 @@ def create_mpack(mpack_name, mpack_version, mpack_instance, 
subgroup_name, modul
   OR
   list of 'components_instances_name' to be created. (default) OR '*' for all 
components
   """
+  mpack_name = mpack_name.lower()
+  module_name = module_name.lower()
+
+  validate_mpack_for_creation_or_changing(mpack_name, mpack_version, 
module_name, components, components_map)
+
   MpackInstance.create_mpack_instance(mpack_name, mpack_version, 
mpack_instance, subgroup_name, module_name,
                                       components, components_map)
 
 
-def set_mpack_instance(mpack, mpack_version, mpack_instance, subgroup_name, 
module_name,
+def set_mpack_instance(mpack, mpack_version, mpack_instance, 
subgroup_name=DEFAULT_SUBGROUP_NAME, module_name=None,
                        components=None, components_map=None):
   """
   Use case: move a given component instances from one version to the next 
version by modifying the soft links.
@@ -63,64 +71,92 @@ def set_mpack_instance(mpack, mpack_version, 
mpack_instance, subgroup_name, modu
   OR
   list of 'components_instances_name' to be created. (default) OR '*' for all 
components
   """
+  mpack = mpack.lower()
+  module_name = module_name.lower()
+
   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:
-    print("\nFound no instances for the given filters.")
-    sys.exit(0)
+    raise ValueError("Found no instances for the given filters.")
+
+  validate_mpack_for_creation_or_changing(mpack, mpack_version, module_name, 
components, components_map)
 
   for mpack in instances:
     for mpack_instance_name in instances[mpack]:
       instances[mpack][mpack_instance_name].set_new_version(mpack, 
mpack_version)
 
 
-def get_conf_dir(mpack=None, mpack_instance=None, subgroup_name='default', 
module_name=None, components_map=None):
+def get_conf_dir(mpack=None, mpack_instance=None, 
subgroup_name=DEFAULT_SUBGROUP_NAME, module_name=None,
+                 components_map=None):
   """
   Use case: retrieve conf directory paths for a given component instances 
based on the granularity specified
             ranging from: mpack, mpack-instance, subgroup-name, module-name 
and map of component instance
             AND with a filtering on each level
 
   Granularity works only while names for all consecutive levels are specified.
+  Note that subgroup has default value of 'default'
   Levels: mpack/instance/subgroup/module
   E.g If only mpack and subgroup names are specified, the granularity will 
work only on mpack level,
-      though the subgroup filer will be applied. But if the instance name is 
specified also, than only granular output
+      though the subgroup fitler will be applied. But if the instance name is 
specified also, than only granular output
       of subgroup will be returned.
 
   Components are provided as map with key as 'component type' and value as 
'list of individual component instances
   names' OR empty map for all component instances present
   """
-  return parse_mpack_instances_with_filtering(mpack, mpack_instance, 
subgroup_name, module_name, components_map,
-                                              output_conf_dir=True)
+  return build_granular_json_with_filtering(mpack, mpack_instance, 
subgroup_name, module_name, components_map,
+                                            output_conf_dir=True)
 
 
-def list_instances(mpack=None, mpack_instance=None, subgroup_name='default', 
module_name=None, components_map=None):
+def list_instances(mpack=None, mpack_instance=None, 
subgroup_name=DEFAULT_SUBGROUP_NAME, module_name=None,
+                   components_map=None):
   """
   Use case: figure out the versions a given component instances based on the 
granularity specified
             ranging from: mpack, mpack-instance, subgroup-name, module-name 
and map of component instance
             AND with a filtering on each level
 
   Granularity works only while names for all consecutive levels are specified.
+  Note that subgroup has default value of 'default'
   Levels: mpack/instance/subgroup/module
   E.g If only mpack and subgroup names are specified, the granularity will 
work only on mpack level,
-      though the subgroup filer will be applied. But if the instance name is 
specified also, than only granular output
+      though the subgroup fitler will be applied. But if the instance name is 
specified also, than only granular output
       of subgroup will be returned.
 
   Components are provided as map with key as 'component type' and value as 
'list of individual component instances
   names' OR empty map for all component instances present
   """
-  return parse_mpack_instances_with_filtering(mpack, mpack_instance, 
subgroup_name, module_name, components_map,
-                                              output_path=True)
+  return build_granular_json_with_filtering(mpack, mpack_instance, 
subgroup_name, module_name, components_map,
+                                            output_path=True)
+
+
+def build_granular_json_with_filtering(mpack_name_filter, 
instance_name_filter, subgroup_name_filter,
+                                       module_name_filter, 
components_name_filter_map, output_conf_dir=False,
+                                       output_path=False):
+  """
+  Builds the json that contains all instances specified in filters or all 
instances if filters are not specified.
+  The level of granularity depends on the consecutive levels of specified 
filters
+  Levels: mpack/instance/subgroup/module
+  E.g If only mpack and subgroup names are specified, the granularity will 
work only on mpack level,
+      though the subgroup fitler will be applied. But if the instance name is 
specified also, than only granular output
+      of subgroup will be returned.
+
+  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()
 
-def parse_mpack_instances_with_filtering(mpack_name_filter, 
instance_name_filter, subgroup_name_filter,
-                                         module_name_filter, 
components_name_filter_map, output_conf_dir=False,
-                                         output_path=False):
   instances = 
MpackInstance.parse_instances_with_filtering(os.path.join(ROOT_FOLDER_PATH, 
INSTANCES_FOLDER_NAME),
                                                            mpack_name_filter,
                                                            
instance_name_filter, subgroup_name_filter,
                                                            module_name_filter,
+                                                           None,
                                                            
components_name_filter_map)
+  if not instances:
+    raise ValueError("Found no instances for the given filters.")
+
   full_json_output = build_json_output(instances, 
output_conf_dir=output_conf_dir, output_path=output_path)
 
   granular_json_output = build_granular_output(full_json_output, 
mpack_name_filter, instance_name_filter,
@@ -132,6 +168,14 @@ def 
parse_mpack_instances_with_filtering(mpack_name_filter, instance_name_filter
 
 def build_granular_output(json_output, mpack_name_filter, 
instance_name_filter, subgroup_name_filter,
                           module_name_filter):
+  """
+  Returns the part of original json using the granularity filters
+
+  The level of granularity depends on the consecutive levels of specified 
filters
+  Levels: mpack/instance/subgroup/module
+  E.g If only mpack and subgroup names are specified, the granularity will 
work only on mpack level,
+      But if the instance name is specified also, than only granular output of 
subgroup will be returned.
+  """
   if mpack_name_filter:
     json_output = json_output['mpacks'][mpack_name_filter]
     if instance_name_filter:
@@ -144,6 +188,10 @@ def build_granular_output(json_output, mpack_name_filter, 
instance_name_filter,
 
 
 def build_json_output_from_instances_dict(instances_dict, plural_name, 
output_conf_dir, output_path):
+  """
+  Build the json from the dictionary of Instance objects.
+  The plural_name is used to form the upper level of the json output.
+  """
   result = {}
   for instance_name in instances_dict:
     result[instance_name] = 
instances_dict[instance_name].build_json_output(output_conf_dir, output_path)
@@ -151,12 +199,26 @@ def build_json_output_from_instances_dict(instances_dict, 
plural_name, output_co
   return {plural_name: result}
 
 
-# FIXME use mpack.json to determine category
-def find_module_category(path, module_name):
-  walk = os.walk(os.path.join(path, module_name))
-  if CONFIGS_DIRECTORY_NAME in next(walk)[1]:
-    return "CLIENT"
-  return "SERVER"
+# Better use it with component instance path.
+# Need to be careful with this method as it will return only the single meta 
mpack whilst the module component instances
+# may be from different mpack versions and point to different mpack.json's
+def find_link_to_current_in_path_recursive(root_path):
+  if not os.path.lexists(os.path.join(root_path, CURRENT_SOFTLINK_NAME)):
+    walk = os.walk(root_path)
+    for folder_name in next(walk)[1]:
+      folder_search_result = 
find_link_to_current_in_path_recursive(os.path.join(root_path, folder_name))
+      if folder_search_result:
+        return folder_search_result
+      else:
+        return None
+  else:
+    return os.path.join(root_path, CURRENT_SOFTLINK_NAME)
+
+
+def get_module_meta_mpack(path, module_name):
+  current_link_location = 
find_link_to_current_in_path_recursive(os.path.join(path, module_name))
+  current_target = os.readlink(current_link_location)
+  return MetaMpack.parse_mpack(os.path.dirname(current_target))
 
 
 def build_json_output(instances, output_conf_dir=False, output_path=False):
@@ -167,6 +229,41 @@ def build_json_output(instances, output_conf_dir=False, 
output_path=False):
   return {'mpacks': result}
 
 
+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):
+    raise ValueError("Mpack {0} doesn't exist, please check mpack 
name.".format(mpack_name))
+
+  mpack_version_path = os.path.join(mpack_root_path, mpack_version)
+  if not os.path.exists(mpack_version_path):
+    raise ValueError(
+      "Mpack version {0} doesn't exist for mpack {1}, please check mpack name 
and version".format(mpack_version,
+                                                                               
                   mpack_name))
+
+  meta_mpack = MetaMpack.parse_mpack(mpack_version_path)
+
+  if not module_name in meta_mpack.module_component_types_map:
+    raise ValueError(
+      "There is no module {0} for mpack {1} with version {2}. Please check 
mpack name, version and module name".format(
+        module_name, mpack_name, mpack_version))
+
+  if components and components != "*":
+    for component in components:
+      if not meta_mpack.get_component_category(component):
+        raise ValueError(
+          "There is no component {0} in module {1} for mpack {2} with version 
{3}."
+          " Please check mpack name, version, module name and component 
name".format(
+            component, module_name, mpack_name, mpack_version))
+
+  if components_map:
+    for component in components_map:
+      if component not in meta_mpack.module_component_types_map[module_name]:
+        raise ValueError(
+          "There is no component {0} in module {1} for mpack {2} with version 
{3}."
+          " Please check mpack name, version, module name and component 
name".format(
+            component, module_name, mpack_name, mpack_version))
+
+
 class MetaMpack:
   def __init__(self, name, version, component_module_map, mpack_json):
     self.name = name
@@ -203,17 +300,33 @@ class MetaMpack:
     return None
 
   @staticmethod
-  def parse_mpack(path, name, version):
+  def parse_mpack(path):
     # build components map from soft links
     component_module_map = {}
     for filename in os.listdir(path):
       if os.path.islink(os.path.join(path, filename)):
         component_module_map[filename] = os.path.realpath(os.path.join(path, 
filename))
 
-    return MetaMpack(name=name,
-                     version=version,
+    mpack_json_path = os.path.join(path, MPACK_JSON_FILE_NAME)
+    if not os.path.exists(mpack_json_path):
+      raise ValueError(
+        "{0} file is missing. The exact location should be 
{1}".format(MPACK_JSON_FILE_NAME, mpack_json_path))
+
+    with open(mpack_json_path, "r") as json_file:
+      json_file_content = json_file.read()
+
+    try:
+      mpack_json = json.loads(json_file_content)
+    except ValueError as e:
+      raise ValueError("The {0} is invalid. Location: {1}. Error message: 
{2}".format(
+        MPACK_JSON_FILE_NAME, mpack_json_path, e.message))
+
+    mpack_version = os.path.basename(path)
+    mpack_name = os.path.basename(os.path.dirname(path))
+    return MetaMpack(name=mpack_name,
+                     version=mpack_version,
                      component_module_map=component_module_map,
-                     mpack_json=json.load(open(os.path.join(path, 
MPACK_JSON_FILE_NAME))))
+                     mpack_json=mpack_json)
 
   @staticmethod
   def parse_mpacks(path):
@@ -228,9 +341,7 @@ class MetaMpack:
     result = {}
     walk = os.walk(path)
     for mpack_version in next(walk)[1]:
-      result[mpack_version] = MetaMpack.parse_mpack(path=os.path.join(path, 
mpack_version),
-                                                    
name=os.path.basename(path),
-                                                    version=mpack_version)
+      result[mpack_version] = MetaMpack.parse_mpack(path=os.path.join(path, 
mpack_version))
     return result
 
 
@@ -293,6 +404,8 @@ class MpackInstance(Instance):
   @staticmethod
   def parse_instances_with_filtering(path, mpack_name_filter=None, 
instance_name_filter=None, subgroup_name_filter=None,
                                      module_name_filter=None, 
components_filter=None, components_name_filter_map=None):
+    if not os.path.exists(path):
+      raise ValueError("There are no created instances. Use 
create-mpack-instance command to add them.")
     result = {}
     walk = os.walk(path)
     for mpack_name in next(walk)[1]:
@@ -344,22 +457,35 @@ class ModuleInstance(Instance):
   def parse_into_module_instance_dict(path, module_name_filter, 
components_filter, components_name_filter_map):
     result = {}
     walk = os.walk(path)
-    for module_name in next(walk)[1]:
-      if not module_name_filter or module_name_filter == module_name:
-
-        module_category = find_module_category(path, module_name)
-
-        if module_category == "CLIENT":
-          components_map = 
ComponentInstance(name=DEFAULT_COMPONENT_INSTANCE_NAME,
-                                             component_path=os.path.join(path, 
module_name),
-                                             path_exec=os.path.realpath(
-                                               os.path.join(path, module_name, 
CURRENT_SOFTLINK_NAME)), is_client=True)
+    # The problem here is that for client component this folder name means the 
component type, while for server modules
+    # it's module name
+    for folder_name in next(walk)[1]:
+      components_map = None
+
+      # find out the module category and name
+      meta_mpack = get_module_meta_mpack(path, folder_name)
+      if folder_name in meta_mpack.module_categoty_map:
+        module_category = meta_mpack.module_categoty_map[folder_name]
+        module_name = folder_name
+      else:
+        module_category = meta_mpack.get_component_category(folder_name)
+        module_name = 
os.path.basename(os.path.dirname(meta_mpack.component_module_map[folder_name]))
+
+      if not module_name_filter or module_name == module_name_filter:
+        if module_category == CLIENT_CATEGORY:
+          if ((not components_filter and not components_name_filter_map) or
+                (components_filter and (components_filter == '*' or 
folder_name in components_filter)) or
+                (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(
+                                                 os.path.join(path, 
folder_name, CURRENT_SOFTLINK_NAME)))
         else:
-          components_map = 
ComponentInstance.parse_into_components_dict(os.path.join(path, module_name),
+          components_map = 
ComponentInstance.parse_into_components_dict(os.path.join(path, folder_name),
                                                                         
components_filter,
                                                                         
components_name_filter_map)
         if components_map:
-          result[module_name] = ModuleInstance(module_name, components_map, 
module_category)
+          result[folder_name] = ModuleInstance(folder_name, components_map, 
module_category)
     return result
 
   @staticmethod
@@ -367,11 +493,9 @@ class ModuleInstance(Instance):
                              components,
                              components_map):
     meta_mpack = MetaMpack.parse_mpack(
-      path=os.path.join(ROOT_FOLDER_PATH, MPACKS_FOLDER_NAME, mpack_name, 
mpack_version),
-      name=mpack_name,
-      version=mpack_version)
+      path=os.path.join(ROOT_FOLDER_PATH, MPACKS_FOLDER_NAME, mpack_name, 
mpack_version))
 
-    is_client_module = meta_mpack.module_categoty_map[module_name] == "CLIENT"
+    is_client_module = meta_mpack.module_categoty_map[module_name] == 
CLIENT_CATEGORY
     if components:
       if components == '*':
         components = meta_mpack.module_component_types_map[module_name]
@@ -387,7 +511,7 @@ class ModuleInstance(Instance):
                                                       is_client_module)
 
   def set_new_version(self, mpack_name, mpack_version):
-    if self.category == 'CLIENT':
+    if self.category == CLIENT_CATEGORY:
       component_instance = self.components_map
       print("\nSetting new version for component : " + 
component_instance.component_path)
       component_instance.set_new_version(mpack_name, mpack_version, 
self.module_name)
@@ -400,7 +524,7 @@ class ModuleInstance(Instance):
 
   def build_json_output(self, output_conf_dir, output_path):
     result = {}
-    if self.category == 'CLIENT':
+    if self.category == CLIENT_CATEGORY:
       result['component_instances'] = {'default': 
self.components_map.build_json_output(output_conf_dir, output_path)}
     else:
       for component_type in self.components_map.keys():
@@ -417,18 +541,19 @@ class ModuleInstance(Instance):
 class ComponentInstance(Instance):
   plural_name = "component-instances"
 
-  def __init__(self, name, component_path, path_exec, is_client=False):
+  def __init__(self, name, component_path, path_exec):
     self.name = name
     self.component_path = component_path
     self.path_exec = path_exec
-    self.is_client = is_client
 
   def set_new_version(self, mpack_name, mpack_version, component_type):
     mpack_path = os.path.join(ROOT_FOLDER_PATH, MPACKS_FOLDER_NAME, 
mpack_name, mpack_version, component_type)
     target_link = os.path.join(self.component_path, CURRENT_SOFTLINK_NAME)
     if os.path.lexists(target_link):
+      if os.readlink(target_link) == mpack_path:
+        print "\n{0} already points to {1}. Skipping.".format(target_link, 
mpack_path)
+        return
       os.remove(target_link)
-      print "\nRemoved old link " + target_link
 
     os.symlink(mpack_path, target_link)
     print "\nCreated new link " + target_link + " -> " + mpack_path
@@ -455,9 +580,9 @@ class ComponentInstance(Instance):
         if components_filter == '*':
           result[component_type] = 
ComponentInstance.parse_into_component_instance_dict(
             os.path.join(path, component_type))
-        else:
+        elif component_type in components_filter:
           component_instance_dict = 
ComponentInstance.parse_into_component_instance_dict(
-            os.path.join(path, component_type), components_filter)
+            os.path.join(path, component_type))
           if component_instance_dict:
             result[component_type] = component_instance_dict
       elif not components_name_filter_map:
@@ -481,6 +606,10 @@ class ComponentInstance(Instance):
                                     module_name, component_type, 
component_instance_name)
     mpack_path = os.path.join(ROOT_FOLDER_PATH, MPACKS_FOLDER_NAME, 
mpack_name, mpack_version, component_type)
 
+    if os.path.exists(component_path):
+      raise ValueError(
+        "The instance {0} already exist. To change the version use 
set-mpack-instance command".format(component_path))
+
     if not os.path.lexists(mpack_path):
       raise ValueError("Path doesn't exist: " + mpack_path)
 
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 81c180d..f5c9451 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
@@ -39,7 +39,24 @@ def init_action_parser(action, parser):
   try:
     action_parser_map[action](parser)
   except KeyError:
-    parser.error("Invalid action: " + action)
+    parser.error(
+      "Invalid action {0}. Possible options are: 
create-mpack-instance|set-mpack-instance|get-conf-dir|list-instances".format(
+        action))
+
+
+def check_required_options(options, action, parser):
+  missing_options = []
+  if action == CREATE_MPACK_INSTANCE_ACTION or action == 
SET_MPACK_INSTANCE_ACTION:
+    if not options.mpack:
+      missing_options.append("mpack")
+    if not options.mpack_version:
+      missing_options.append("mpack-version")
+    if not options.mpack_instance:
+      missing_options.append("mpack-instance")
+    if not options.module_name:
+      missing_options.append("module-name")
+
+  parser.error("Missing following required command options: 
{0}".format(missing_options))
 
 
 def init_create_parser_options(parser):
@@ -95,44 +112,60 @@ def init_get_parser_options(parser):
 
 def main(options, args):
   action = sys.argv[1]
+
+  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 options.components_map:
+      parsed_components_map = ast.literal_eval(options.components_map)
+  except ValueError:
+    raise ValueError("Components or components-map format is invalid.")
+
   if action == CREATE_MPACK_INSTANCE_ACTION:
     create_mpack(mpack_name=options.mpack, mpack_version=options.mpack_version,
                  mpack_instance=options.mpack_instance,
                  subgroup_name=options.subgroup_name, 
module_name=options.module_name,
-                 components=options.components,
-                 components_map=ast.literal_eval(options.components_map))
+                 components=parsed_components,
+                 components_map=parsed_components_map)
 
   elif action == SET_MPACK_INSTANCE_ACTION:
     set_mpack_instance(mpack=options.mpack, 
mpack_version=options.mpack_version,
                        mpack_instance=options.mpack_instance,
                        subgroup_name=options.subgroup_name, 
module_name=options.module_name,
-                       components=options.components,
-                       components_map=ast.literal_eval(options.components_map))
+                       components=parsed_components,
+                       components_map=parsed_components_map)
 
   elif action == GET_CONF_DIR_ACTION:
     print get_conf_dir(mpack=options.mpack, 
mpack_instance=options.mpack_instance,
                        subgroup_name=options.subgroup_name, 
module_name=options.module_name,
-                       components_map=ast.literal_eval(options.components_map))
+                       components_map=parsed_components_map)
 
   elif action == LIST_INSTANCES_ACTION:
     print list_instances(mpack=options.mpack, 
mpack_instance=options.mpack_instance,
                          subgroup_name=options.subgroup_name, 
module_name=options.module_name,
-                         
components_map=ast.literal_eval(options.components_map))
+                         components_map=parsed_components_map)
 
 
 if __name__ == "__main__":
   if len(sys.argv) < 2:
     print(
-      "Missing the command. Possible options are: 
{create-mpack-instance|set-mpack-instance|get-conf-dir|list-instances}")
+      "Missing the command. Possible options are: 
create-mpack-instance|set-mpack-instance|get-conf-dir|list-instances")
     sys.exit(1)
 
-  parser = optparse.OptionParser()
+  usage = "Usage: %prog 
create-mpack-instance|set-mpack-instance|get-conf-dir|list-instances [options]"
+  parser = optparse.OptionParser(usage=usage)
   action = sys.argv[1]
   init_action_parser(action, parser)
   (options, args) = parser.parse_args()
+  check_required_options(options, action, parser)
 
   try:
     main(options, args)
   except (KeyboardInterrupt, EOFError):
     print("\nAborting ... Keyboard Interrupt.")
     sys.exit(1)
+  except ValueError as e:
+    print "ERROR: " + e.message
+    sys.exit(1)
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 88870e3..5315094 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
@@ -29,9 +29,11 @@ TMP_ROOT_FOLDER = "/tmp/instance_manager_test"
 instance_manager.ROOT_FOLDER_PATH = TMP_ROOT_FOLDER
 
 MPACK_NAME = 'hdpcore'
+MPACK_NAME2 = 'edw'
 MPACK_VERSION_1 = '1.0.0-b1'
 MPACK_VERSION_2 = '1.5.0-b1'
 INSTANCE_NAME_1 = 'Production'
+INSTANCE_NAME_2 = 'eCommerce'
 SUBGROUP_NAME = 'default'
 CLIENT_MODULE_NAME = 'hdfs-clients'
 CLIENT_COMPONENT_NAME = 'hdfs_client'
@@ -101,9 +103,11 @@ class TestInstanceManager(TestCase):
                      os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MPACKS_FOLDER_NAME, MPACK_NAME,
                                   MPACK_VERSION_1, SERVER_COMPONENT_NAME))
 
-  def test_set_version_asterisk(self):
+  def test_set_version_server_module_asterisk(self):
     create_mpack_with_defaults()
 
+    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,
                                         SERVER_MODULE_NAME, '*')
 
@@ -116,9 +120,27 @@ class TestInstanceManager(TestCase):
                      os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MPACKS_FOLDER_NAME, MPACK_NAME,
                                   MPACK_VERSION_2, SERVER_COMPONENT_NAME))
 
+  def test_set_version_client_module_asterisk(self):
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME)
+
+    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, '*')
+
+    current_link = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.INSTANCES_FOLDER_NAME, MPACK_NAME,
+                                INSTANCE_NAME_1, SUBGROUP_NAME, 
CLIENT_COMPONENT_NAME,
+                                instance_manager.CURRENT_SOFTLINK_NAME)
+
+    self.assertEqual(os.readlink(current_link),
+                     os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MPACKS_FOLDER_NAME, MPACK_NAME,
+                                  MPACK_VERSION_2, CLIENT_COMPONENT_NAME))
+
   def test_set_version_for_one_of_two_component_instances(self):
     create_mpack_with_defaults(components=None, 
components_map={SERVER_COMPONENT_NAME: ['server1', 'server2']})
 
+    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,
                                         SERVER_MODULE_NAME, None, 
{SERVER_COMPONENT_NAME: ['server2']})
 
@@ -237,6 +259,132 @@ class TestInstanceManager(TestCase):
     }
     self.assertEqual(conf_dir_json, expected_json)
 
+  def test_granularity(self):
+    create_mpack_with_defaults()
+
+    full_conf_dir_json = instance_manager.get_conf_dir()
+    self.assertTrue('mpacks' in full_conf_dir_json)
+
+    mpack_conf_dir_json = instance_manager.get_conf_dir(mpack=MPACK_NAME)
+    self.assertTrue('mpack-instances' in mpack_conf_dir_json)
+
+    instance_conf_dir_json = instance_manager.get_conf_dir(mpack=MPACK_NAME, 
mpack_instance=INSTANCE_NAME_1,
+                                                           subgroup_name=None)
+    self.assertTrue('subgroups' in instance_conf_dir_json)
+
+    subgroup_conf_dir_json = instance_manager.get_conf_dir(mpack=MPACK_NAME, 
mpack_instance=INSTANCE_NAME_1,
+                                                           
subgroup_name=SUBGROUP_NAME)
+    self.assertTrue('modules' in subgroup_conf_dir_json)
+
+    module_conf_dir_json = instance_manager.get_conf_dir(mpack=MPACK_NAME, 
mpack_instance=INSTANCE_NAME_1,
+                                                         
subgroup_name=SUBGROUP_NAME, module_name=SERVER_MODULE_NAME)
+    self.assertTrue('components' in module_conf_dir_json)
+
+    # The mpack level filter not specified
+    full_conf_dir_json = 
instance_manager.get_conf_dir(mpack_instance=INSTANCE_NAME_1, 
subgroup_name=SUBGROUP_NAME,
+                                                       
module_name=SERVER_MODULE_NAME)
+    self.assertTrue('mpacks' in full_conf_dir_json)
+
+    # The instance level filter not specified
+    mpack_conf_dir_json = instance_manager.get_conf_dir(mpack=MPACK_NAME, 
subgroup_name=SUBGROUP_NAME,
+                                                        
module_name=SERVER_MODULE_NAME)
+    self.assertTrue('mpack-instances' in mpack_conf_dir_json)
+
+  def test_filtering(self):
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME)
+    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME, components=None,
+                               components_map={})
+
+    create_mpack_with_defaults(module_name=CLIENT_MODULE_NAME, 
mpack_instance=INSTANCE_NAME_2)
+    create_mpack_with_defaults(module_name=SERVER_MODULE_NAME, 
mpack_instance=INSTANCE_NAME_2, components=None,
+                               components_map={SERVER_COMPONENT_NAME: 
['server1']})
+
+    build_rpm_structure(mpack_name=MPACK_NAME2, remove_old_content=False, 
create_modules=False)
+    create_mpack_with_defaults(mpack_name=MPACK_NAME2, 
mpack_instance=INSTANCE_NAME_2,
+                               module_name=CLIENT_MODULE_NAME)
+    create_mpack_with_defaults(mpack_name=MPACK_NAME2, 
mpack_instance=INSTANCE_NAME_2,
+                               module_name=SERVER_MODULE_NAME, components=None,
+                               components_map={SERVER_COMPONENT_NAME: 
['server2']})
+
+    filter_by_module_json = 
instance_manager.list_instances(module_name=SERVER_MODULE_NAME)
+    self.assertTrue(MPACK_NAME in filter_by_module_json['mpacks'])
+    self.assertTrue(MPACK_NAME2 in filter_by_module_json['mpacks'])
+    self.assertTrue(INSTANCE_NAME_2 in 
filter_by_module_json['mpacks'][MPACK_NAME]['mpack-instances'])
+    self.assertTrue(INSTANCE_NAME_1 not in 
filter_by_module_json['mpacks'][MPACK_NAME]['mpack-instances'])
+
+    filter_by_component_instance_name_json = instance_manager.list_instances(
+      components_map={SERVER_COMPONENT_NAME: ['server2']})
+    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'}}}}}}}}}}}}}
+    self.assertEquals(expected_filter_result, 
filter_by_component_instance_name_json)
+
+  def test_validation(self):
+    try:
+      create_mpack_with_defaults(mpack_name=MPACK_NAME2)
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message, "Mpack {0} doesn't exist, please check 
mpack name.".format(MPACK_NAME2))
+
+    try:
+      create_mpack_with_defaults(mpack_version=MPACK_VERSION_2)
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "Mpack version {0} doesn't exist for mpack {1}, please 
check mpack name and version".format(
+                          MPACK_VERSION_2, MPACK_NAME))
+
+    try:
+      create_mpack_with_defaults(module_name=SERVER_MODULE_NAME + "broken")
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "There is no module {0} for mpack {1} with version 
{2}."
+                        " Please check mpack name, version and module 
name".format(
+                          SERVER_MODULE_NAME + "broken", MPACK_NAME, 
MPACK_VERSION_1))
+
+    try:
+      create_mpack_with_defaults(components_map={SERVER_COMPONENT_NAME: 
"comp1"}, module_name=CLIENT_MODULE_NAME)
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "There is no component {0} in module {1} for mpack {2} 
with version {3}."
+                        " Please check mpack name, version, module name and 
component name".format(
+                          SERVER_COMPONENT_NAME, CLIENT_MODULE_NAME, 
MPACK_NAME, MPACK_VERSION_1))
+
+  def test_creating_existing_component_instance(self):
+    create_mpack_with_defaults()
+    try:
+      create_mpack_with_defaults()
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "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_set_non_existing_instance(self):
+    try:
+      instance_manager.set_mpack_instance(mpack=MPACK_NAME, 
mpack_version=MPACK_VERSION_1,
+                                          mpack_instance=INSTANCE_NAME_1,
+                                          subgroup_name=SUBGROUP_NAME, 
module_name=SERVER_MODULE_NAME,
+                                          components_map={})
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "There are no created instances. Use 
create-mpack-instance command to add them.")
+
+    create_mpack_with_defaults()
+    try:
+      instance_manager.set_mpack_instance(mpack=MPACK_NAME, 
mpack_version=MPACK_VERSION_1,
+                                          mpack_instance=INSTANCE_NAME_1,
+                                          subgroup_name=SUBGROUP_NAME, 
module_name=SERVER_MODULE_NAME,
+                                          
components_map={SERVER_COMPONENT_NAME: ["non-existing-instance"]})
+      raise AssertionError("The previous call should have thrown exception")
+    except ValueError as e:
+      self.assertEquals(e.message,
+                        "Found no instances for the given filters.")
+
 
 def create_mpack_with_defaults(mpack_name=MPACK_NAME, 
mpack_version=MPACK_VERSION_1, mpack_instance=INSTANCE_NAME_1,
                                subgroup_name=SUBGROUP_NAME, 
module_name=SERVER_MODULE_NAME, components='*',
@@ -245,19 +393,26 @@ def create_mpack_with_defaults(mpack_name=MPACK_NAME, 
mpack_version=MPACK_VERSIO
                                 subgroup_name, module_name, components, 
components_map)
 
 
-def build_rpm_structure():
-  remove_rpm_structure()
+def build_rpm_structure(mpack_name=MPACK_NAME, mpack_version=MPACK_VERSION_1, 
mpack_json=MPACK_JSON,
+                        module_version_mapping=MODULE_VERSION_MAPPING,
+                        module_component_mapping=MODULE_COMPONENT_MAPPING,
+                        remove_old_content=True,
+                        create_modules=True):
+  if remove_old_content:
+    remove_rpm_structure()
 
-  mpack_path = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MPACKS_FOLDER_NAME, MPACK_NAME, MPACK_VERSION_1)
+  mpack_path = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MPACKS_FOLDER_NAME, mpack_name, mpack_version)
   os.makedirs(mpack_path)
-  with open(os.path.join(mpack_path, "mpack.json"), "w") as mpack_json:
-    json.dump(MPACK_JSON, mpack_json)
+  with open(os.path.join(mpack_path, "mpack.json"), "w") as mpack_json_file:
+    json.dump(mpack_json, mpack_json_file)
 
   modules_path = os.path.join(TMP_ROOT_FOLDER, 
instance_manager.MODULES_FOLDER_NAME)
-  for module_name in MODULE_VERSION_MAPPING:
-    os.makedirs(os.path.join(modules_path, module_name, 
MODULE_VERSION_MAPPING[module_name], 'bin'))
-    os.symlink(os.path.join(modules_path, module_name, 
MODULE_VERSION_MAPPING[module_name]),
-               os.path.join(mpack_path, MODULE_COMPONENT_MAPPING[module_name]))
+  for module_name in module_version_mapping:
+    if create_modules:
+      os.makedirs(os.path.join(modules_path, module_name, 
module_version_mapping[module_name], 'bin'))
+
+    os.symlink(os.path.join(modules_path, module_name, 
module_version_mapping[module_name]),
+               os.path.join(mpack_path, module_component_mapping[module_name]))
 
 
 def remove_rpm_structure():

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

Reply via email to