Remove iscsi deploy interface support for Xena+

This is inline with upstream removal of the iscsi deploy
interface in OpenStack Xena. [1]

Additional changes include:
* Switch to sorted(list) as list.sort() returns None
* Switch to deepcopy(PACKAGES) to prevent changing the constant

[1] 929907d684

Closes-Bug: #1952140
Depends-On: https://review.opendev.org/c/openstack/charms.openstack/+/822039
Change-Id: If7fdf6083cf298ee3932ee84a9e9d0bcd053686a

osci.yaml

@@ -2,18 +2,8 @@
     templates:
       - charm-unit-jobs-py38
       - charm-unit-jobs-py39
-      # Non-voting due to:
-      # https://bugs.launchpad.net/charm-ironic-conductor/+bug/1952140
-      # - charm-xena-functional-jobs
+      - charm-xena-functional-jobs
       - charm-yoga-functional-jobs
-    check:
-      jobs:
-        # Non-voting due to:
-        # https://bugs.launchpad.net/charm-ironic-conductor/+bug/1952140
-        - focal-xena:
-            voting: false
-        - impish-xena:
-            voting: false
     vars:
       needs_charm_build: true
       charm_build_name: ironic-conductor

src/README.md

@@ -27,7 +27,7 @@ There are a number of configuration parameters that greatly influence how the Ir
 The Ironic conductor charm currently supports two deployment interfaces:
 
   * direct
-  * iscsi
+  * iscsi (prior to OpenStack Xena)
 
 ### The iSCSI deployment interface
 
@@ -51,6 +51,11 @@ Cons:
     * Applying any post-write configuration
   * Requires more Ironic conductor units as the number of bare metal nodes increases
 
+Removal:
+
+  * The iSCSI deploy interface is removed from the ironic project in OpenStack Xena. Support for the iscsi deploy interface is therefore removed from the charm for OpenStack Xena
+  and later releases.
+
 You can set this deployment interface by running the following commands:
 
 ```bash

src/config.yaml

@@ -114,7 +114,7 @@ options:
       list of enabled-network-interfaces.
   enabled-deploy-interfaces:
     type: string
-    default: "direct, iscsi"
+    default: "direct"
     description: |
       Comma separated list of deploy interfaces to use.
       Valid options are:
@@ -132,6 +132,10 @@ options:
       Note: The iscsi deploy mode requires that ironic-conductor be deployed on a VM
       or a bare metal machine. That is because the iscsi kernel module is not namespaced,
       and the ironic-conductor will not be able to log into any iscsi target.
+
+      Note: The iscsi deploy interface is removed from the Ironic project in OpenStack
+      Xena. Support for the iscsi deploy interface is therefore removed from the charm
+      For OpenStack Xena and later releases.
   default-deploy-interface:
     type: string
     default: "direct"

src/lib/charm/openstack/ironic/ironic.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import
 
 import collections
 import os
+from copy import deepcopy
 
 import charms_openstack.charm
 import charms_openstack.adapters
@@ -10,7 +11,10 @@ from charms_openstack.adapters import (
     DatabaseRelationAdapter,
     OpenStackRelationAdapters,
 )
-from charmhelpers.contrib.openstack.utils import os_release
+from charmhelpers.contrib.openstack.utils import (
+    CompareOpenStackReleases,
+    os_release,
+)
 
 import charm.openstack.ironic.controller_utils as controller_utils
 import charms_openstack.adapters as adapters
@@ -182,7 +186,7 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm):
     abstract_class = False
     release = 'train'
     name = 'ironic'
-    packages = PACKAGES
+    packages = deepcopy(PACKAGES)
     python_version = 3
 
     service_type = 'ironic'
@@ -348,6 +352,10 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm):
 
     def _validate_deploy_interfaces(self, interfaces):
         valid_interfaces = VALID_DEPLOY_INTERFACES
+        if (CompareOpenStackReleases(self.release) >= 'xena' and
+                "iscsi" in valid_interfaces):
+            valid_interfaces.remove("iscsi")
+
         has_secret = reactive.is_flag_set("leadership.set.temp_url_secret")
         for interface in interfaces:
             if interface not in valid_interfaces:
@@ -432,3 +440,9 @@ class IronicConductorCharm(charms_openstack.charm.OpenStackCharm):
             return ('blocked', msg)
 
         return (None, None)
+
+
+class IronicConductorXenaCharm(IronicConductorCharm):
+
+    release = 'xena'
+    packages = [p for p in deepcopy(PACKAGES) if p != "open-iscsi"]

unit_tests/test_lib_charm_openstack_ironic.py

@@ -13,13 +13,17 @@
 # limitations under the License.
 
 import mock
+from copy import deepcopy
 
 import charms_openstack.test_utils as test_utils
 import charms.leadership as leadership
 import charmhelpers.core.hookenv as hookenv
 import charms.reactive as reactive
 
-from charmhelpers.contrib.openstack.utils import os_release
+from charmhelpers.contrib.openstack.utils import (
+    CompareOpenStackReleases,
+    os_release,
+)
 
 from charm.openstack.ironic import ironic
 from charm.openstack.ironic import controller_utils as ctrl_util
@@ -272,7 +276,7 @@ class TestIronicCharm(test_utils.PatchHelper):
 
         target = ironic.IronicConductorCharm()
         target._setup_pxe_config(self.mocked_pxe_cfg)
-        expected_pkgs = ironic.PACKAGES + ["fakepkg1", "fakepkg2"]
+        expected_pkgs = deepcopy(ironic.PACKAGES) + ["fakepkg1", "fakepkg2"]
 
         expected_cfg = {
             'tftpboot': ctrl_util.PXEBootBase.TFTP_ROOT,
@@ -295,13 +299,23 @@ class TestIronicCharm(test_utils.PatchHelper):
             'default-deploy-interface': 'fake_deploy'}
 
         self.assertEqual(
-            target.packages.sort(),
-            expected_pkgs.sort())
+            sorted(target.packages),
+            sorted(expected_pkgs))
         self.assertEqual(target.config, expected_cfg)
         self.assertEqual(
             target.restart_map.get("fake_config", []), ["fake_svc"])
         self.assertTrue("fakehttpd" in target.services)
 
+    def test_packages_xena(self):
+        reactive.is_flag_set.side_effect = [False, False, False]
+        target = ironic.IronicConductorXenaCharm()
+        expected_pkgs = deepcopy(ironic.PACKAGES) + ["fakepkg1", "fakepkg2"]
+        expected_pkgs.remove("open-iscsi")
+
+        self.assertEqual(
+            sorted(target.packages),
+            sorted(expected_pkgs))
+
     def test_validate_network_interfaces(self):
         target = ironic.IronicConductorCharm()
         with self.assertRaises(ValueError):
@@ -311,6 +325,7 @@ class TestIronicCharm(test_utils.PatchHelper):
 
     def test_validate_deploy_interfaces(self):
         target = ironic.IronicConductorCharm()
+        CompareOpenStackReleases.return_value = 'wallaby'
         with self.assertRaises(ValueError) as err:
             target._validate_deploy_interfaces(["bogus"])
 
@@ -322,11 +337,27 @@ class TestIronicCharm(test_utils.PatchHelper):
             target._validate_deploy_interfaces(["direct"]))
         self.assertEqual(str(err.exception), expected_msg)
 
+    def test_validate_deploy_interfaces_xena(self):
+        reactive.is_flag_set.side_effect = [False, False, False]
+        target = ironic.IronicConductorXenaCharm()
+        CompareOpenStackReleases.return_value = 'xena'
+        with self.assertRaises(ValueError) as err:
+            target._validate_deploy_interfaces(["bogus"])
+
+        expected_msg = (
+            'Deploy interface bogus is not valid.'
+            ' Valid interfaces are: direct')
+
+        self.assertIsNone(
+            target._validate_deploy_interfaces(["direct"]))
+        self.assertEqual(str(err.exception), expected_msg)
+
     def test_validate_deploy_interfaces_tmp_secret(self):
         # leadership.set.temp_url_secret is not set, and "direct"
         # boot method is enabled. Validate will fail, until
         # set-temp-url-secret action is run
         reactive.is_flag_set.side_effect = [False, True]
+        CompareOpenStackReleases.return_value = 'wallaby'
         target = ironic.IronicConductorCharm()
         with self.assertRaises(ValueError) as err:
             target._validate_deploy_interfaces(["direct"])
@@ -381,6 +412,7 @@ class TestIronicCharm(test_utils.PatchHelper):
             "enabled-deploy-interfaces": "direct, iscsi",
             "default-network-interface": "flat",
             "enabled-network-interfaces": "neutron, flat, noop"}
+        CompareOpenStackReleases.return_value = 'wallaby'
         target = ironic.IronicConductorCharm()
         self.assertEqual(target.custom_assess_status_check(), (None, None))
 
@@ -404,6 +436,7 @@ class TestIronicCharm(test_utils.PatchHelper):
             "enabled-deploy-interfaces": "bogus, iscsi",
             "default-network-interface": "flat",
             "enabled-network-interfaces": "neutron, flat, noop"}
+        CompareOpenStackReleases.return_value = 'wallaby'
         target = ironic.IronicConductorCharm()
         expected_status = (
             'blocked',
@@ -433,6 +466,7 @@ class TestIronicCharm(test_utils.PatchHelper):
             "enabled-deploy-interfaces": "direct, iscsi",
             "default-network-interface": "flat",
             "enabled-network-interfaces": "neutron, flat, noop"}
+        CompareOpenStackReleases.return_value = 'wallaby'
         target = ironic.IronicConductorCharm()
         expected_status = (
             'blocked',