OLUEK-5005 oci-metadata (oci-utils) value-only flag broken

Author: guido tijskens

bin/oci-iscsi-config

@@ -69,8 +69,14 @@ do
                               break
                               ;;
         # end of parsing , everything getopt did not understood is after that, we do not care in our case
-        --) shift; break ;;
-        *)                    COMMAND="usage"                                       ; break   ;;
+        --)
+                              if [ -z ${COMMAND} ]
+                              then
+                                COMMAND="usage"
+                              fi
+                              shift; break
+                              ;;
+        *)                    COMMAND="usage"                                                ; break   ;;
     esac
 done
 

buildrpm/oci-utils.spec

@@ -1,6 +1,6 @@
 Name: oci-utils
-Version: 0.12.4
-Release: 4%{?dist}
+Version: 0.12.5
+Release: 2%{?dist}
 Url: http://cloud.oracle.com/iaas
 Summary: Oracle Cloud Infrastructure utilities
 License: UPL
@@ -178,6 +178,15 @@ rm -rf %{buildroot}
 /opt/oci-utils/tests/__init__*
 
 %changelog
+* Mon Jun 28 Guido Tijskens <[email protected]> -- 0.12.5-2
+- OLUEK-5005 oci-metadata (oci-utils) value-only flag broken
+
+* Wed Jun 23 2021 Guido Tijskens <[email protected]> -- 0.12.5-1
+- modified oci-iscsi-config output in compat mode
+- corrected oci-iscsi-config behaviour on invalid compat syntax
+- some small changes
+- changed log and error messages
+
 * Thu Jun 17 2021 Guido Tijskens <[email protected]> -- 0.12.4-4
 - LINUX-11136 compatibility: oci-network-config --(de)configure does not show results
 - LINUX-11163 compatibility: oci-network-config differences in output
@@ -237,7 +246,7 @@ rm -rf %{buildroot}
 
 
 * Tue Dec 1 2020 Guido Tijskens <[email protected]> -- 0.12.0-1
- - update migrate
+- update migrate
 
 * Tue Nov 10 2020 Emmanuel Jannetti <[email protected]> --0.12.0
  - LINUX-9546 - oci-image-cleanup --dry-run do not print the plan directly

lib/oci_utils/cache.py

@@ -164,8 +164,7 @@ def write_cache(cache_content, cache_fname, fallback_fname=None, mode=None):
         if not os.path.exists(cachedir):
             os.makedirs(cachedir)
         if mode is not None:
-            cache_fd = os.open(cache_fname, os.O_WRONLY | os.O_CREAT,
-                               mode)
+            cache_fd = os.open(cache_fname, os.O_WRONLY | os.O_CREAT, mode)
         else:
             cache_fd = os.open(cache_fname, os.O_WRONLY | os.O_CREAT)
         cache_file = os.fdopen(cache_fd, 'w')
@@ -178,11 +177,9 @@ def write_cache(cache_content, cache_fname, fallback_fname=None, mode=None):
             if not os.path.exists(cachedir):
                 os.makedirs(cachedir)
             if mode is not None:
-                cache_fd = os.open(fallback_fname,
-                                   os.O_WRONLY | os.O_CREAT, mode)
+                cache_fd = os.open(fallback_fname, os.O_WRONLY | os.O_CREAT, mode)
             else:
-                cache_fd = os.open(fallback_fname,
-                                   os.O_WRONLY | os.O_CREAT)
+                cache_fd = os.open(fallback_fname, os.O_WRONLY | os.O_CREAT)
             cache_file = os.fdopen(cache_fd, 'w')
             fname = fallback_fname
         except (OSError, IOError):

lib/oci_utils/impl/__init__.py

@@ -12,8 +12,15 @@
 import logging.handlers
 
 
-__all__ = ['read_config', 'SUDO_CMD',
-           'CAT_CMD', 'SH_CMD', 'CP_CMD', 'TOUCH_CMD', 'CHMOD_CMD', 'LSBLK_CMD', 'MKDIR_CMD']
+__all__ = ['read_config',
+           'SUDO_CMD',
+           'CAT_CMD',
+           'SH_CMD',
+           'CP_CMD',
+           'TOUCH_CMD',
+           'CHMOD_CMD',
+           'LSBLK_CMD',
+           'MKDIR_CMD']
 
 
 CAT_CMD = '/usr/bin/cat'

lib/oci_utils/impl/oci-iscsi-config-main.py

@@ -90,8 +90,7 @@ def get_args_parser():
     -------
         The commandline argparse namespace.
     """
-    parser = argparse.ArgumentParser(description='Utility for listing or configuring iSCSI devices on an OCI '
-                                                 'instance.')
+    parser = argparse.ArgumentParser(description='Utility for listing or configuring iSCSI devices on an OCI instance.')
     subparser = parser.add_subparsers(dest='command')
     #
     # sync
@@ -428,7 +427,7 @@ def display_attached_volumes(oci_sess, iscsiadm_session, disks, output_mode, det
        No return value.
     """
     #
-    # todo: handle the None ocisession more elegantly.
+    # todo: handle the None oci_sess more elegantly.
     oci_vols = list()
     try:
         if bool(oci_sess):
@@ -498,8 +497,13 @@ def display_attached_volumes(oci_sess, iscsiadm_session, disks, output_mode, det
             iscsi_dev_printer.printRow(_item)
             if output_mode == 'compat':
                 if 'partitions' not in disks[_item['dev']]:
-                    iscsi_dev_printer.printKeyValue('File system type', disks[_item['dev']]['fstype'])
-                    iscsi_dev_printer.printKeyValue('Mountpoint', disks[_item['dev']]['mountpoint'])
+                    #
+                    # iscsi_dev_printer.printKeyValue('File system type', disks[_item['dev']]['fstype'])
+                    # iscsi_dev_printer.printKeyValue('Mountpoint', disks[_item['dev']]['mountpoint'])
+                    fstype = disks[_item['dev']]['fstype'] if bool(disks[_item['dev']]['fstype']) else 'Unknown'
+                    iscsi_dev_printer.printKeyValue('File system type', fstype)
+                    mntpoint = disks[_item['dev']]['mountpoint'] if bool(disks[_item['dev']]['mountpoint']) else 'Not mounted'
+                    iscsi_dev_printer.printKeyValue('Mountpoint', mntpoint)
                 else:
                     partitions = disks[device]['partitions']
                     partitionPrinter.printHeader()
@@ -530,7 +534,7 @@ def display_detached_iscsi_device(iqn, targets, attach_failed=()):
     """
     devicePrinter = get_row_printer_impl('table')(title="Target %s" % iqn,
                                                   text_truncate=False,
-                                                  columns=(['Portal', 12, 'portal'], ['State', 12, 'state']))
+                                                  columns=(['Portal', 20, 'portal'], ['State', 65, 'state']))
     devicePrinter.printHeader()
     _item = {}
     for ipaddr in list(targets.keys()):
@@ -577,6 +581,7 @@ def _do_iscsiadm_attach(iqn, targets, user=None, passwd=None, iscsi_portal_ip=No
             if iqn in targets[ipaddr]:
                 portal_ip = ipaddr
         if portal_ip is None:
+            #
             # this shouldn't really happen, but just in case
             raise Exception("Can't find portal IP address")
     else:
@@ -1062,7 +1067,7 @@ def main():
         if bool(oci_sess.this_instance()):
             _this_instance_ocid = oci_sess.this_instance().get_ocid()
     else:
-        _this_instance_ocid = get_instance_ocid
+        _this_instance_ocid = get_instance_ocid()
 
     if 'compat' in args and args.compat is True:
         # Display information as version 0.11 for compatibility reasons for few settings.
@@ -1092,7 +1097,7 @@ def main():
 
     ocid_cache = load_cache(iscsiadm.ISCSIADM_CACHE, max_age=timedelta(minutes=2))[1]
     if ocid_cache is None:
-        _logger.debug('updating the cache')
+        _logger.debug('Updating the cache')
         # run ocid once, to update the cache
         ocid_refresh(wait=True)
         # now try to load again
@@ -1273,7 +1278,6 @@ def main():
             return 1
 
         retval = 0
-
         for iqn in args.iqns:
             _iqn_to_use = iqn
             _save_chap_cred = False

lib/oci_utils/impl/oci-metadata-main.py

@@ -74,14 +74,14 @@
     'publicIp',
     'macAddr',
     'subnetCidrBlock',
-    'virtualRouterIp', ]
+    'virtualRouterIp',
+    'definedTags']
 
 human_readable_type = {
     str: 'string type',
     dict: 'json format'}
 
 oci_metadata_ignores = [
-    'definedTags',
     'freeformTags',
     'launchMode',
     'ipxeScript',
@@ -436,7 +436,7 @@ def get_values(key, metadata):
             elif v is not None:
                 values.append(v)
         return values
-    if isinstance(metadata, dict):
+    if not isinstance(metadata, dict):
         return None
     if key in metadata:
         return metadata[key]
@@ -481,7 +481,11 @@ def get_trimed_key_values(keys, metadata):
         if len(ks) > 1:
             # path key
             newkey_list = []
-            _get_path_keys(metadata, ks[1:], newkey_list)
+            try:
+                _get_path_keys(metadata, ks[1:], newkey_list)
+            except Exception as e:
+                _logger.error('%s', str(e))
+                continue
             for _key in newkey_list:
                 v = _get_by_path(metadata, _key)
                 if v:
@@ -640,6 +644,7 @@ def main():
         return 1
 
     if args.setkeys:
+        # set
         if args.keys:
             _logger.error("-g or --get option conflicts with -u or --update.")
             return 1
@@ -652,6 +657,11 @@ def main():
             # meta = oci_utils.oci_api.OCISession().update_instance_metadata(
             # instance_id=inst_id, **k_v)
             meta = OCISession().update_instance_metadata(instance_id=inst_id, **k_v)
+            if meta is None:
+                #
+                # if meta is None, the session failed to update the metadata; the session is writing the error message;
+                # this should change...
+                return 1
             metadata = meta.filter(list(k_v.keys()))
         except Exception as e:
             _logger.error("%s", str(e), exc_info=True)
@@ -660,7 +670,7 @@ def main():
         # get
         if args.value_only:
             if len(args.keys) != 1:
-                sys.stderr.write("Error: --value-only option works only with one -g or --get option.")
+                _logger.error("Error: --value-only option works only with one -g or --get option.")
                 return 1
 
         try:

lib/oci_utils/impl/oci-network-config-main.py

@@ -530,7 +530,9 @@ def _get_subnet(_, interface):
                 ['IFACE', 15, 'IFACE'],
                 ['VLTAG', 5, 'VLTAG'],
                 ['VLAN', 11, 'VLAN'],
-                ['STATE', 5, 'STATE'], ['MAC', 17, 'MAC'], ['VNIC', 90, 'VNIC'])
+                ['STATE', 5, 'STATE'],
+                ['MAC', 17, 'MAC'],
+                ['VNIC ID', 90, 'VNIC'])
     printer = TablePrinter(title=_title, columns=_columns, column_separator='', text_truncate=False)
 
     printer.printHeader()

lib/oci_utils/impl/ocid-main.py

@@ -153,7 +153,7 @@ def request_stop(self):
         trigger a stop phase of the daemon
         set the event triggering the stop
         """
-        self.thr_logger.debug('been requested to stop')
+        self.thr_logger.debug('Recieved a request to stop.')
         self.active = False
         self.blocking_evt.set()
 
@@ -179,7 +179,7 @@ def run(self):
             if not self.active:
                 # shutting down.
                 self.thread_lock.release()
-                self.thr_logger.debug('shutting down')
+                self.thr_logger.debug('Shutting down.')
                 break
             try:
                 self.context = self.ocidfunc(self.context, self.thr_logger)
@@ -284,11 +284,9 @@ def iscsi_func(context, func_logger):
 
     # Load the saved passwords
     chap_passwords = context['chap_pws']
-    if context['chap_pw_ts'] == 0 or \
-            get_timestamp(oci_utils.__chap_password_file) > context['chap_pw_ts']:
+    if context['chap_pw_ts'] == 0 or get_timestamp(oci_utils.__chap_password_file) > context['chap_pw_ts']:
         # the password file has changed or was never loaded
-        context['chap_pw_ts'], chap_passwords = \
-            load_cache(oci_utils.__chap_password_file)
+        context['chap_pw_ts'], chap_passwords = load_cache(oci_utils.__chap_password_file)
     if chap_passwords is None:
         chap_passwords = {}
     # save for the next iteration
@@ -302,10 +300,11 @@ def iscsi_func(context, func_logger):
     # volumes connected to this instance
     inst_volumes = []
     if context['oci_sess'] is not None:
+        #
         # get a list of volumes attached to the instance
         instance = context['oci_sess'].this_instance()
         if instance is None:
-            func_logger.debug('Cannot get current instance')
+            func_logger.debug('Cannot get current instance.')
         else:
             volumes = instance.all_volumes()
             for v in volumes:
@@ -319,7 +318,9 @@ def iscsi_func(context, func_logger):
                 else:
                     all_iqns[v.get_portal_ip()] = [v.get_iqn()]
     else:
+        #
         # fall back to scanning
+        func_logger.debug('Scan for volumes.')
         for r in range(context['max_volumes'] + 1):
             ipaddr = "169.254.2.%d" % (r + 1)
             iqns = oci_utils.iscsiadm.discovery(ipaddr)
@@ -334,28 +335,28 @@ def iscsi_func(context, func_logger):
                     vol['user'] = chap_passwords[iqn][0]
                     vol['password'] = chap_passwords[iqn][1]
                 inst_volumes.append(vol)
-
+    #
     # Load the list of volumes that were detached using oci-iscsi-config.
     # ocid shouldn't attach these automatically.
     ignore_iqns = context['ignore_iqns']
-    if context['ignore_file_ts'] == 0 or \
-            get_timestamp(oci_utils.__ignore_file) > context['ignore_file_ts']:
+    if context['ignore_file_ts'] == 0 or get_timestamp(oci_utils.__ignore_file) > context['ignore_file_ts']:
+        #
         # the list of detached volumes changed since last reading the file
-        context['ignore_file_ts'], ignore_iqns = \
-            load_cache(oci_utils.__ignore_file)
+        context['ignore_file_ts'], ignore_iqns = load_cache(oci_utils.__ignore_file)
     if ignore_iqns is None:
         ignore_iqns = []
+    #
     # save for next iteration
     context['ignore_iqns'] = ignore_iqns
-
+    #
     # volumes that failed to attach in an earlier iteration
     attach_failed = context['attach_failed']
-
+    #
     # do we need to cache files?
     cache_changed = False
     ign_changed = False
     chap_changed = False
-
+    #
     # check if all discovered iscsi devices are configured and attached
     for vol in inst_volumes:
         if vol['iqn'] in ignore_iqns: