Merge ~jneo8/charm-openstack-service-checks:bug/1963549 into charm-openstack-service-checks:master

[JamesLin]

Add OS_VOLUME_API_VERSION to nagios.novarc

This merge make volume_api_version as required in os-credentials.

Reference:

https://docs.openstack.org/python-cinderclient/latest/user/shell.html

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Alvaro Uria (aluria)]

Is the nagios_plugin3.py lib needed in this project? Tox will import it from charm-nrpe, which holds the original version.

review: Needs Information

[JamesLin (jneo8)]

Sorry about that, nagios_plugin3.py is a error when git commit.
Removed.

[Alvaro Uria (aluria)]

LGTM. Please provide some info about the check working in non-xena/non-yoga openstack versions as well as any of those versions and it will be +1 from me. Thanks.

Possibly, functests should include one of the versions (xena or yoga).

[Alvaro Uria (aluria)]

Status of the MP marked as "Work in progress". Please change it manually to "Ready for review" when the feedback is addressed. Thanks!

[JamesLin (jneo8)]

Deploy cinder with channel wallaby/edge && ussuri/edge.

Manual execute the python script, and get the response from cinder success.

Do I need to provide the functional testing from those version?

[Eric Chen (eric-chen)]

review: Approve (mr tracking)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Failed to merge change (unable to merge source repository due to conflicts), setting status to needs review.

[Rodrigo Barbieri (rodrigo-barbieri2010)]

Hello,

I reproduced the issue on charmhub latest/stable rev11 version and attempted to test this fix.

There is a trivial merge conflict on master but the master code is broken at the moment due to a "nagios_plugin3" dependency error. I applied the fix on top of 22.04 tag then and tested it successfully.

So right now master needs to be fixed so this fix can be tested against master.

[Rodrigo Barbieri (rodrigo-barbieri2010)]

I tested this fix on top of [1] and the error is resolved. I +1 this.

[1] https://code.launchpad.net/~bootstack-charmers/charm-openstack-service-checks/+git/charm-openstack-service-checks/+merge/426423

review: Approve

[Rodrigo Barbieri (rodrigo-barbieri2010)]

Dependency [1] has merged, this patch needs to be rebased.

@jneo8, could you please rebase this?

[1] https://code.launchpad.net/~bootstack-charmers/charm-openstack-service-checks/+git/charm-openstack-service-checks/+merge/426423

review: Needs Fixing

[JamesLin (jneo8)]

> Dependency [1] has merged, this patch needs to be rebased.
>
> @jneo8, could you please rebase this?
>
> [1] https://code.launchpad.net/~bootstack-charmers/charm-openstack-service-
> checks/+git/charm-openstack-service-checks/+merge/426423

rebased

[Rodrigo Barbieri (rodrigo-barbieri2010)]

Thanks James Lin, I code reviewed, the rebase looks good (thanks for removing that comma btw, I just spotted it) but just a nitpick that "test_09_openstack_check_cinder_service" should now be "test_10_openstack_check_cinder_service". I'm fine with it and approve it anyway, but it would be great if you can fix it.

review: Approve

[Paul Goins (vultaire)]

Please see diff comment; thanks.

review: Needs Fixing

[Rodrigo Barbieri (rodrigo-barbieri2010)]

Hi James Lin, do you have some spare cycles to perform the updates suggested by Paul Goins?

review: Needs Fixing

[JamesLin (jneo8)]

> Please see diff comment; thanks.

Updated.

[JamesLin (jneo8)]

> Hi James Lin, do you have some spare cycles to perform the updates suggested
> by Paul Goins?

Hi Rodrigo, thanks for the patience. I switch to another task last week. I updated it in the last commit.

[Paul Goins (vultaire)]

review: Needs Fixing

[JamesLin (jneo8)]

Updated

[Paul Goins (vultaire)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 5ccf0981e512be8fb0685f3b207c3a6deba116df

[JamesLin (jneo8)]

Thanks @vultaire

diff --git a/src/config.yaml b/src/config.yaml
index 20efe3b..045e0f4 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -94,7 +94,7 @@ options:
       Comma separated OpenStack credentials to be used by nagios.
       It is strongly recommended this be a user with a dedicated role,
       and not a full admin.  Takes the format of
-      username=foo, password=bar, credentials_project=baz, region_name=Region1, auth_url=http://127.0.0.1:35357
+      username=foo, password=bar, credentials_project=baz, region_name=Region1, auth_url=http://127.0.0.1:35357, volume_api_version=3
   nova_warn:
     default: 2
     type: int
diff --git a/src/lib/lib_openstack_service_checks.py b/src/lib/lib_openstack_service_checks.py
index 81db325..2a12a7b 100644
--- a/src/lib/lib_openstack_service_checks.py
+++ b/src/lib/lib_openstack_service_checks.py
@@ -207,7 +207,8 @@ class OSCHelper:
             creds = {}
 
         common_attrs = (
-            "username password region_name auth_url credentials_project"
+            "username password region_name auth_url"
+            " credentials_project volume_api_version"
         ).split()
         all_attrs = common_attrs + extra_attrs
         missing = [k for k in all_attrs if k not in ident_creds]
diff --git a/src/templates/nagios.novarc b/src/templates/nagios.novarc
index 27f1c2a..a6e6be8 100644
--- a/src/templates/nagios.novarc
+++ b/src/templates/nagios.novarc
@@ -4,6 +4,7 @@ export OS_PROJECT_NAME={{ project_name }}
 export OS_PASSWORD={{ password }}
 export OS_REGION_NAME={{ region }}
 export OS_AUTH_URL={{ auth_url }}
+export OS_VOLUME_API_VERSION={{ volume_api_version }}
 {%- if cacert %}
 export OS_CACERT={{ cacert }}
 export REQUESTS_CA_BUNDLE=$OS_CACERT
diff --git a/src/tests/functional/tests/bundles/focal-yoga.yaml b/src/tests/functional/tests/bundles/focal-yoga.yaml
new file mode 100644
index 0000000..57d12d3
--- /dev/null
+++ b/src/tests/functional/tests/bundles/focal-yoga.yaml
@@ -0,0 +1,125 @@
+variables:
+  openstack-origin: &openstack-origin cloud:focal-yoga
+
+series: focal
+
+applications:
+  rabbitmq-server:
+    charm: ch:rabbitmq-server
+    channel: latest/edge
+    num_units: 1
+    options:
+      source: *openstack-origin
+  keystone-mysql-router:
+    charm: ch:mysql-router
+    channel: latest/edge
+  nova-mysql-router:
+    charm: ch:mysql-router
+    channel: latest/edge
+  neutron-mysql-router:
+    charm: ch:mysql-router
+    channel: latest/edge
+  mysql-innodb-cluster:
+    charm: ch:mysql-innodb-cluster
+    num_units: 3
+    channel: latest/edge
+    constraints: mem=3072M
+    options:
+      source: *openstack-origin
+  nova-cloud-controller:
+    charm: ch:nova-cloud-controller
+    channel: latest/edge
+    num_units: 1
+    options:
+      network-manager: Neutron
+      openstack-origin: *openstack-origin
+  neutron-api:
+    charm: ch:neutron-api
+    channel: latest/edge
+    num_units: 1
+    options:
+      manage-neutron-plugin-legacy-mode: True
+      flat-network-providers: physnet1
+      neutron-security-groups: true
+      openstack-origin: *openstack-origin
+  keystone:
+    charm: ch:keystone
+    channel: latest/edge
+    num_units: 1
+    options:
+      openstack-origin: *openstack-origin
+  nagios:
+    charm: ch:nagios
+    num_units: 1
+    series: bionic  # Does not support Focal
+  nrpe:
+    charm: ch:nrpe
+  openstack-service-checks:
+    num_units: 1
+  ceph-radosgw:
+    charm: ch:ceph-radosgw
+    num_units: 1
+    options:
+      namespace-tenants: True
+      source: *openstack-origin
+  cinder:
+    charm: ch:cinder
+    num_units: 1
+    storage:
+      block-devices: '40G'
+    options:
+      openstack-origin: *openstack-origin
+      block-device: None
+      overwrite: "true"
+  cinder-lvm:
+    charm: ch:cinder-lvm
+    options:
+      block-device: "/mnt/cinder-lvm-block|20G"
+      config-flags: "target_helper=lioadm"
+  cinder-mysql-router:
+    charm: ch:mysql-router
+    channel: latest/edge
+
+relations:
+  - - nova-cloud-controller:shared-db
+    - nova-mysql-router:shared-db
+  - - nova-cloud-controller:identity-service
+    - keystone:identity-service
+  - - nova-cloud-controller:amqp
+    - rabbitmq-server:amqp
+  - - keystone:shared-db
+    - keystone-mysql-router:shared-db
+  - - neutron-api:shared-db
+    - neutron-mysql-router:shared-db
+  - - neutron-api:amqp
+    - rabbitmq-server:amqp
+  - - neutron-api:neutron-api
+    - nova-cloud-controller:neutron-api
+  - - neutron-api:identity-service
+    - keystone:identity-service
+  - - nrpe:monitors
+    - nagios:monitors
+  - - nrpe:nrpe-external-master
+    - openstack-service-checks:nrpe-external-master
+  - - openstack-service-checks:identity-credentials
+    - keystone:identity-credentials
+  - - openstack-service-checks:identity-notifications
+    - keystone:identity-notifications
+  - - keystone-mysql-router:db-router
+    - mysql-innodb-cluster:db-router
+  - - neutron-mysql-router:db-router
+    - mysql-innodb-cluster:db-router
+  - - nova-mysql-router:db-router
+    - mysql-innodb-cluster:db-router
+  - - ceph-radosgw:identity-service
+    - keystone:identity-service
+  - - cinder:identity-service
+    - keystone:identity-service
+  - - cinder-mysql-router:shared-db
+    - cinder:shared-db
+  - - rabbitmq-server:amqp
+    - cinder:amqp
+  - - cinder-mysql-router:db-router
+    - mysql-innodb-cluster:db-router
+  - - cinder:storage-backend
+    - cinder-lvm:storage-backend
diff --git a/src/tests/functional/tests/test_deploy.py b/src/tests/functional/tests/test_deploy.py
index a9b610c..827f172 100755
--- a/src/tests/functional/tests/test_deploy.py
+++ b/src/tests/functional/tests/test_deploy.py
@@ -247,3 +247,10 @@ class TestOpenStackServiceChecks(TestBase):
         assert status_msg == expected_msg
         # NOTE (rgildein): This last test will break the keystone unit, and you need to
         # run `juju resolve`.
+
+    def test_10_openstack_check_cinder_service(self):
+        """Verify cinder service.""",
+        model.block_until_all_units_idle()
+        cmd = "python3 /usr/local/lib/nagios/plugins/check_cinder_services.py"
+        result = model.run_on_unit(self.lead_unit_name, cmd)
+        self.assertEquals(result.get("Code"), "0")  # Get response from cinder
diff --git a/src/tests/functional/tests/tests.yaml b/src/tests/functional/tests/tests.yaml
index 4ba93d5..1d726c8 100644
--- a/src/tests/functional/tests/tests.yaml
+++ b/src/tests/functional/tests/tests.yaml
@@ -6,6 +6,7 @@ smoke_bundles:
   - base: bionic
 dev_bundles:
   - compute: bionic
+  - compute: focal-yoga
 target_deploy_status:
   nova-cloud-controller:
     workload-status: blocked
diff --git a/src/tests/unit/test_lib.py b/src/tests/unit/test_lib.py
index 3217837..0cf4808 100644
--- a/src/tests/unit/test_lib.py
+++ b/src/tests/unit/test_lib.py
@@ -53,7 +53,7 @@ def test_openstackservicechecks_get_keystone_credentials_unitdata(
         (
             (
                 'username=nagios, password=password, region_name=RegionOne, auth_url="http://XX.XX.XX.XX:5000/v3",'  # noqa:E501
-                "credentials_project=services, domain=service_domain"
+                "credentials_project=services, domain=service_domain, volume_api_version=3"  # noqa:E501
             ),
             {
                 "username": "nagios",
@@ -64,12 +64,13 @@ def test_openstackservicechecks_get_keystone_credentials_unitdata(
                 "project_domain_name": "service_domain",
                 "region_name": "RegionOne",
                 "auth_url": "http://XX.XX.XX.XX:5000/v3",
+                "volume_api_version": "3",
             },
         ),
         (
             (
                 'username=nagios, password=password, region_name=RegionOne, auth_url="http://XX.XX.XX.XX:5000/v2.0",'  # noqa:E501
-                "credentials_project=services"
+                "credentials_project=services, volume_api_version=3"
             ),
             {
                 "username": "nagios",
@@ -77,6 +78,7 @@ def test_openstackservicechecks_get_keystone_credentials_unitdata(
                 "tenant_name": "services",
                 "region_name": "RegionOne",
                 "auth_url": "http://XX.XX.XX.XX:5000/v2.0",
+                "volume_api_version": "3",
             },
         ),
     ],
diff --git a/src/tox.ini b/src/tox.ini
index 8cff956..69cfe81 100644
--- a/src/tox.ini
+++ b/src/tox.ini
@@ -31,6 +31,7 @@ passenv =
   OS_USER_DOMAIN_NAME
   OS_PROJECT_NAME
   OS_IDENTITY_API_VERSION
+  OS_VOLUME_API_VERSION
 
 [testenv:lint]
 commands =