PRO TIP: Migrate inventory plugins: prepare section plugin

In order to migrate an inventory plugin to the new API,
it is best to first migrate the required section.

While not strictly necessary, a dedicated section plugin
decouples the inventory and check plugins.

In this commit:
 * move the parse function
  ** rename "info" -> "string_table" (required)
  ** add a few type hints (optional)
 * register the section
 * add a comment to the legacy plugins (for less puzzlement)

Change-Id: Idc4060f5f7f12371aa8baf1fbd8d0d76ad1d53fb

checks/oracle_instance

@@ -25,100 +25,6 @@ factory_settings["oracle_instance_defaults"] = {
 }
 
 
-def create_oracle_instance(info_line):
-    result = {}
-    result['general_error'] = None
-    # In case of a general error (e.g. authentication failed), the second
-    # column contains the word "FAILURE"
-    if info_line[1] == 'FAILURE':
-        result['general_error'] = " ".join(info_line[2:])
-
-    # lines can have different length
-    line_len = len(info_line)
-    result['invalid_data'] = result['general_error'] is not None or line_len not in [6, 11, 12, 22]
-
-    def getcolumn(column_index, default=None):
-        if (result['general_error'] and column_index != 0) or column_index >= line_len:
-            return default
-        return info_line[column_index]
-
-    # assign columns
-    result['sid'] = getcolumn(0)
-    result['version'] = getcolumn(1)
-    result['openmode'] = getcolumn(2)
-    result['logins'] = getcolumn(3)
-    result['archiver'] = getcolumn(4) if line_len > 6 else None
-    result['up_seconds'] = getcolumn(5) if line_len > 6 else None
-    # line_len > 6
-    result['_dbid'] = getcolumn(6)
-    result['log_mode'] = getcolumn(7)
-    result['database_role'] = getcolumn(8)
-    result['force_logging'] = getcolumn(9)
-    result['name'] = getcolumn(10)
-    # line_len > 11
-    result['db_creation_time'] = getcolumn(11)
-    # line_len > 12
-    result['pluggable'] = getcolumn(12, "FALSE")
-    result['_con_id'] = getcolumn(13)
-    result['pname'] = getcolumn(14)
-    result['_pdbid'] = getcolumn(15)
-    result['popenmode'] = getcolumn(16)
-    result['prestricted'] = getcolumn(17)
-    result['ptotal_size'] = getcolumn(18)
-    result['_precovery_status'] = getcolumn(19)
-    result['pup_seconds'] = getcolumn(20)
-    result['_pblock_size'] = getcolumn(21)
-
-    result['old_agent'] = False
-    result['pdb'] = False
-
-    if not result['general_error']:
-        # Detect old oracle agent plugin output
-        if line_len == 6:
-            result['old_agent'] = True
-
-        # possible multitenant entry?
-        # every pdb has a con_id != 0
-        if line_len > 12 and result['pluggable'] == 'TRUE' and result['_con_id'] != '0':
-            result['pdb'] = True
-
-            if result['prestricted'].lower() == 'no':
-                result['logins'] = 'RESTRICTED'
-            else:
-                result['logins'] = 'ALLOWED'
-
-            result['openmode'] = result['popenmode']
-            result['up_seconds'] = result['pup_seconds']
-
-    return result
-
-
-def parse_oracle_instance(info):
-    parsed = {}
-
-    for line in info:
-        if not line:
-            continue
-
-        # Skip ORA- error messages from broken old oracle agent
-        # <<<oracle_instance:sep(124)>>>
-        # ORA-99999 tnsping failed for +ASM1
-        if line[0].startswith('ORA-') and line[0][4].isdigit() and len(line[0]) < 16:
-            continue
-
-        item_data = create_oracle_instance(line)
-
-        item_name = item_data['sid']
-
-        # Multitenant use DB_NAME.PDB_NAME as Service
-        if item_data['pdb']:
-            item_name = "%s.%s" % (item_data['sid'], item_data['pname'])
-
-        parsed[item_name] = item_data
-
-    return parsed
-
-
 def _transform_oracle_instance_params(p):
     if "ignore_noarchivelog" in p:
         if p["ignore_noarchivelog"]:
@@ -222,7 +128,7 @@ def check_oracle_instance(item, params, item_data):
 
 
 check_info['oracle_instance'] = {
-    "parse_function": parse_oracle_instance,
+    # section is already migrated!
     "check_function": check_oracle_instance,
     "inventory_function": discover(),
     "service_description": "ORA %s Instance",

cmk/base/plugins/agent_based/oracle_instance.py

@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+# Copyright (C) 2019 tribe29 GmbH - License: GNU General Public License v2
+# This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
+# conditions defined in the file COPYING, which is part of this source code package.
+
+from typing import Dict, Optional, Sequence, Mapping, Union
+
+from .agent_based_api.v1.type_defs import StringTable
+from .agent_based_api.v1 import register
+
+Instance = Mapping[str, Union[None, str, bool]]
+
+Section = Mapping[str, Instance]
+
+
+def create_oracle_instance(info_line: Sequence[str]) -> Instance:
+    result: Dict[str, Union[None, str, bool]] = {}
+    result['general_error'] = None
+    # In case of a general error (e.g. authentication failed), the second
+    # column contains the word "FAILURE"
+    if info_line[1] == 'FAILURE':
+        result['general_error'] = " ".join(info_line[2:])
+
+    # lines can have different length
+    line_len = len(info_line)
+    result['invalid_data'] = result['general_error'] is not None or line_len not in [6, 11, 12, 22]
+
+    def getcolumn(column_index: int, default: Optional[str] = None) -> Optional[str]:
+        if (result['general_error'] and column_index != 0) or column_index >= line_len:
+            return default
+        return info_line[column_index]
+
+    # assign columns
+    result['sid'] = getcolumn(0)
+    result['version'] = getcolumn(1)
+    result['openmode'] = getcolumn(2)
+    result['logins'] = getcolumn(3)
+    result['archiver'] = getcolumn(4) if line_len > 6 else None
+    result['up_seconds'] = getcolumn(5) if line_len > 6 else None
+    # line_len > 6
+    result['_dbid'] = getcolumn(6)
+    result['log_mode'] = getcolumn(7)
+    result['database_role'] = getcolumn(8)
+    result['force_logging'] = getcolumn(9)
+    result['name'] = getcolumn(10)
+    # line_len > 11
+    result['db_creation_time'] = getcolumn(11)
+    # line_len > 12
+    result['pluggable'] = getcolumn(12, "FALSE")
+    result['_con_id'] = getcolumn(13)
+    result['pname'] = getcolumn(14)
+    result['_pdbid'] = getcolumn(15)
+    result['popenmode'] = getcolumn(16)
+    result['prestricted'] = getcolumn(17)
+    result['ptotal_size'] = getcolumn(18)
+    result['_precovery_status'] = getcolumn(19)
+    result['pup_seconds'] = getcolumn(20)
+    result['_pblock_size'] = getcolumn(21)
+
+    result['old_agent'] = False
+    result['pdb'] = False
+
+    if not result['general_error']:
+        # Detect old oracle agent plugin output
+        if line_len == 6:
+            result['old_agent'] = True
+
+        # possible multitenant entry?
+        # every pdb has a con_id != 0
+        if line_len > 12 and result['pluggable'] == 'TRUE' and result['_con_id'] != '0':
+            result['pdb'] = True
+
+            if str(result['prestricted']).lower() == 'no':
+                result['logins'] = 'RESTRICTED'
+            else:
+                result['logins'] = 'ALLOWED'
+
+            result['openmode'] = result['popenmode']
+            result['up_seconds'] = result['pup_seconds']
+
+    return result
+
+
+def parse_oracle_instance(string_table: StringTable) -> Section:
+    parsed = {}
+
+    for line in string_table:
+        if not line:
+            continue
+
+        # Skip ORA- error messages from broken old oracle agent
+        # <<<oracle_instance:sep(124)>>>
+        # ORA-99999 tnsping failed for +ASM1
+        if line[0].startswith('ORA-') and line[0][4].isdigit() and len(line[0]) < 16:
+            continue
+
+        item_data = create_oracle_instance(line)
+
+        item_name = str(item_data['sid'])
+
+        # Multitenant use DB_NAME.PDB_NAME as Service
+        if item_data['pdb']:
+            item_name = "%s.%s" % (item_data['sid'], item_data['pname'])
+
+        parsed[item_name] = item_data
+
+    return parsed
+
+
+register.agent_section(
+    name="oracle_instance",
+    parse_function=parse_oracle_instance,
+)

tests/unit/checks/generictests/datasets/oracle_instance_regression.py

@@ -7,11 +7,13 @@
 # yapf: disable
 # type: ignore
 
+from cmk.base.plugins.agent_based.oracle_instance import parse_oracle_instance
+
 checkname = 'oracle_instance'
 
 freeze_time = '2020-01-30 00:00:00'
 
-info = [
+parsed = parse_oracle_instance([
     [
         'TUX2', '12.1.0.1.0', 'OPEN', 'ALLOWED', 'STARTED', '6735',
         '1297771692', 'ARCHIVELOG', 'PRIMARY', 'NO', 'TUX2'
@@ -24,7 +26,7 @@
         '+ASM', 'FAILURE',
         'ORA-99999 tnsping failed for +ASM ERROR: ORA-28002: the password will expire within 1 days'
     ]
-]
+])
 
 discovery = {
     '': [('+ASM', {}), ('TUX2', {}), ('TUX5', {})],

tests/unit/checks/test_oracle_instance.py

@@ -9,31 +9,24 @@
 
 import pytest
 from tests.testlib import Check
+
 from cmk.base.check_api import MKCounterWrapped
+from cmk.base.plugins.agent_based.oracle_instance import parse_oracle_instance
 
 pytestmark = pytest.mark.checks
 
-_broken_info = [
-    [
-        '+ASM', 'FAILURE',
-        'ORA-99999 tnsping failed for +ASM ERROR: ORA-28002: the password will expire within 1 days'
-    ]
-]
-
-@pytest.mark.parametrize('info', [
-    _broken_info,
-])
-def test_oracle_intance_uptime_discovery(info):
-    main_check = Check('oracle_instance')
+PARSED = parse_oracle_instance([[
+    '+ASM', 'FAILURE',
+    'ORA-99999 tnsping failed for +ASM ERROR: ORA-28002: the password will expire within 1 days'
+]])
+
+
+def test_oracle_intance_uptime_discovery():
     check = Check('oracle_instance.uptime')
-    assert list(check.run_discovery(main_check.run_parse(info))) == []
+    assert list(check.run_discovery(PARSED)) == []
 
 
-@pytest.mark.parametrize('info', [
-    _broken_info,
-])
-def test_oracle_instance_uptime_check_error(info):
-    main_check = Check('oracle_instance')
+def test_oracle_instance_uptime_check_error():
     check = Check('oracle_instance.uptime')
     with pytest.raises(MKCounterWrapped):
-        check.run_check("+ASM", {}, main_check.run_parse(info))
+        check.run_check("+ASM", {}, PARSED)