fix(perf): validate metric arguments and missing instances

Author: c-kr

checkvsphere/vcmd/datastores.py

@@ -70,6 +70,12 @@ def range_in_bytes(r: Range, uom):
         ":" + ('' if end == float('+inf') else str(end))
 
 args = None
+UNIT_METRICS = ('B', 'kB', 'MB', 'GB')
+METRIC_CHOICES = (
+    ['usage']
+    + [metric for metric in ('free', 'used', 'capacity')]
+    + [f"{metric}_{unit}" for metric in ('free', 'used', 'capacity') for unit in UNIT_METRICS]
+)
 
 def run():
     global args
@@ -85,6 +91,7 @@ def run():
         'options': {
             'action': 'store',
             'default': 'usage',
+            'choices': METRIC_CHOICES,
             'help': 'The metric to apply the thresholds on, defaults to `usage`, can be: '
                     'usage (in percent), free and used. '
                     'free and used are measured in bytes. You can one of these suffixes: '

checkvsphere/vcmd/perf.py

@@ -80,17 +80,18 @@ def run():
     check = Check()
     check.set_threshold(warning=args.warning, critical=args.critical)
 
-    args._si = service_instance.connect(args)
-
     try:
         vimtype = getattr(vim, args.vimtype)
     except:
         raise Exception(f"vim.{args.vimtype} is not known")
 
-    try:
-        args.perfcounter.split(":", 2)
-    except:
-        raise Exception("perfcounter must be composed as groupName:perfName:rollupType")
+    parts = args.perfcounter.split(":")
+    if len(parts) != 3 or any(not part for part in parts):
+        raise CheckVsphereException(
+            "perfcounter must be composed as groupName:perfName:rollupType"
+        )
+
+    args._si = service_instance.connect(args)
 
     (counter, metricId) = get_metric(
         args._si.content.perfManager, args.perfcounter, args.perfinstance
@@ -152,15 +153,24 @@ def run():
                     code=check.check_threshold(val),
                     message=f'Counter {args.perfcounter} on {args.vimtype}:{args.vimname or props["name"]} reported {val:.8g} {counterInfo["unit"]}',
                 )
+        check.exit(
+            code=Status.UNKNOWN,
+            message=(
+                f"Counter {args.perfcounter} on {args.vimtype}:{args.vimname or props['name']} "
+                "did not return an aggregate instance; maybe --perfinstance='*' helps"
+            ),
+        )
     else:
         metrics_count = len(values.value)
+        found_instance = False
         for instance in values.value:
             if metrics_count == 1 and instance.id.instance == '':
                 raise CheckVsphereException("The perfcounter only returns an aggregate, maybe you want to use --perfinstance '' instead")
             if instance.id.instance == '':
                 # ignore the aggregate if we query a specific or all instances
                 continue
             if args.perfinstance == '*' or args.perfinstance == instance.id.instance:
+                found_instance = True
                 val = instance.value[0] * counterInfo['factor']
                 check.add_perfdata(
                     label=f'{instance.id.instance} {args.perfcounter}',
@@ -173,6 +183,16 @@ def run():
                     f"'{instance.id.instance} {args.perfcounter}' has value {val:.8g} {counterInfo['unit']}",
                 )
 
+        if not found_instance:
+            check.exit(
+                code=Status.UNKNOWN,
+                message=(
+                    f"Counter {args.perfcounter} on {args.vimtype}:{args.vimname or props['name']} "
+                    f"did not return the requested instance '{args.perfinstance}'; "
+                    "maybe --perfinstance='*' helps to examine the available instances"
+                ),
+            )
+
         (code, message) = check.check_messages(separator='\n  ')
         check.exit(code=code, message=message)
 
@@ -230,7 +250,7 @@ def get_argparser():
             'options': {
                 'action': 'store',
                 'default': '',
-                'help': 'the instance of of the metric to monitor. defaults to empty string, '
+                'help': 'the instance of the metric to monitor. defaults to empty string, '
                 'which is not always available but means an aggregated value over all instances',
             },
         }

checkvsphere/vcmd/vmguestfs.py

@@ -30,7 +30,7 @@
     isallowed,
     isbanned,
 )
-from checkvsphere.vcmd.datastores import Space, range_in_bytes
+from checkvsphere.vcmd.datastores import METRIC_CHOICES, Space, range_in_bytes
 
 args = None
 
@@ -49,6 +49,7 @@ def run():
             "options": {
                 "action": "store",
                 "default": "usage",
+                "choices": METRIC_CHOICES,
                 "help": "The metric to apply the thresholds on, defaults to `usage`, can be: "
                 "usage (in percent), free and used. "
                 "free and used are measured in bytes. You can use one of these suffixes: "