Upstream work on the discoverability of certain recommended volume type aspects.

[josephineSei]

As a user, I want to be able to look into a volume type object and see all aspects it fulfills so that I can choose the best suited volume type for my volumes.

In #265 a standard for volume types is created. Right now SCS, providers and consumers need to rely on Tags in the description of a volume type to discover volume types with the recommended aspects encrypted and replicated.

We want to find a way to use the internal extra_spec in volume types for the description of those two aspects, when they are present in a volume type. If this is not possible, we would like to introduce another property, which has to be set by the admin, when setting the volume type. Only after that we will have the possibility to automatically check for a volume type with replication or encyption.

Definition of Done:

• The aspect of encryption is discoverable for a user role in upstream OpenStack
• The aspect of replication is discoverable for a user role in upstream OpenStack
• The standard is changed to use the upstream way of discovering replication and encryption in a volume type

[josephineSei]

For volume types there exist already the concept of user visible extra specs, that are triggered by policy:
https://docs.openstack.org/cinder/latest/admin/user-visible-extra-specs.html
It should be possible to add support for an encryption extra spec, as everything necessary is already there. The encryption parameters can be accessed as an admin like this:

stack@woc15:~/devstack$ openstack volume type show LUKS --encryption-type
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Field              | Value                                                                                                                                                                                                                             |
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| access_project_ids | None                                                                                                                                                                                                                              |
| description        | None                                                                                                                                                                                                                              |
| encryption         | cipher='aes-xts-plain64', control_location='front-end', created_at='2023-11-30T16:13:15.000000', deleted='False', deleted_at=, encryption_id='872a9738-34d0-43d6-977a-91ea4858e74f', key_size='256', provider='luks', updated_at= |
| id                 | bee35d94-d42c-42d8-b226-32be9ca5b694                                                                                                                                                                                              |
| is_public          | True                                                                                                                                                                                                                              |
| name               | LUKS                                                                                                                                                                                                                              |
| properties         |                                                                                                                                                                                                                                   |
| qos_specs_id       | None                                                                                                                                                                                                                              |
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

For replication it is harder, as it depends on the used backend. That means, it cannot be derived from inside openstack but would need the input from the provider, whether there is replication or not. I will go into discussion with the Cinder developers about this.

But it might be possible to easily do that as the setting of extra specs is only allowed for admins now and only when the volume type is not in use. So it would just be necessary to make something like this visible to users:

openstack volume type set --property replications=3 LUKS
openstack volume type show LUKS --encryption-type
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Field              | Value                                                                                                                                                                                                                             |
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| access_project_ids | None                                                                                                                                                                                                                              |
| description        | None                                                                                                                                                                                                                              |
| encryption         | cipher='aes-xts-plain64', control_location='front-end', created_at='2023-11-30T16:13:15.000000', deleted='False', deleted_at=, encryption_id='872a9738-34d0-43d6-977a-91ea4858e74f', key_size='256', provider='luks', updated_at= |
| id                 | bee35d94-d42c-42d8-b226-32be9ca5b694                                                                                                                                                                                              |
| is_public          | True                                                                                                                                                                                                                              |
| name               | LUKS                                                                                                                                                                                                                              |
| properties         | replications='3'                                                                                                                                                                                                                  |
| qos_specs_id       | None                                                                                                                                                                                                                              |
+--------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

This is currently only visible for admins.

[josephineSei]

While going through the cinder code to check the places for any upstream patches, I stumbled across the policies:
https://github.com/openstack/cinder/blob/master/cinder/policies/volume_type.py#L156
This allows users to read the extra specs but it can be shut down through a policy file (only allowing admin access). We should keep that in mind.

[josephineSei]

I am currently trying to implement a visible user extra spec for encryption, that is automatically set when creating or unset when deleting an encryption type in here:
https://github.com/openstack/cinder/blob/master/cinder/api/contrib/volume_type_encryption.py
It should not be setable outside of that workflow.

[josephineSei]

I am currently running into some errors with my implementation and try to fix that:

openstack volume type create --encryption-provider nova.volume.encryptors.luks.LuksEncryptor --encryption-cipher aes-xts-plain64 --encryption-key-size 256 --encryption-control-location front-end LUKS
Failed to set encryption information for this volume type: The resource could not be found.<br /><br />


 (HTTP 404)
local variable 'encryption' referenced before assignment

[josephineSei]

I found a neat solution for a possible encryption extra spec:

stack@devstack:~/devstack$ openstack volume type create --encryption-provider nova.volume.encryptors.luks.LuksEncryptor --encryption-cipher aes-xts-plain64 --encryption-key-size 256 --encryption-control-location front-end LUKS3
+-------------+------------------------------------------------------------------------------+
| Field       | Value                                                                        |
+-------------+------------------------------------------------------------------------------+
| description | None                                                                         |
| encryption  | cipher='aes-xts-plain64', control_location='front-end',                      |
|             | encryption_id='135648f5-a979-4ee4-a5ed-14115431bf0f', key_size='256',        |
|             | provider='nova.volume.encryptors.luks.LuksEncryptor'                         |
| id          | d0ff021e-9379-452d-8596-f1669447edbd                                         |
| is_public   | True                                                                         |
| name        | LUKS3                                                                        |
+-------------+------------------------------------------------------------------------------+
stack@devstack:~/devstack$ openstack volume type show LUKS3
+--------------------+--------------------------------------+
| Field              | Value                                |
+--------------------+--------------------------------------+
| access_project_ids | None                                 |
| description        | None                                 |
| id                 | d0ff021e-9379-452d-8596-f1669447edbd |
| is_public          | True                                 |
| name               | LUKS3                                |
| properties         | encryption_enabled='1'               |
| qos_specs_id       | None                                 |
+--------------------+--------------------------------------+
stack@devstack:~/devstack$ source openrc demo
WARNING: setting legacy OS_TENANT_NAME to support cli tools.
stack@devstack:~/devstack$ openstack volume type show LUKS3
+--------------------+--------------------------------------+
| Field              | Value                                |
+--------------------+--------------------------------------+
| access_project_ids | None                                 |
| description        | None                                 |
| id                 | d0ff021e-9379-452d-8596-f1669447edbd |
| is_public          | True                                 |
| name               | LUKS3                                |
| properties         | encryption_enabled='1'               |
+--------------------+--------------------------------------+

It is created when creating an encrypted volume type and visible to users.
I will polish it and make an upstream patchset and ask them, if that would fit.

[josephineSei]

I pushed my changes to https://review.opendev.org/c/openstack/cinder/+/907519 to discuss them with upstream.

[josephineSei]

I pushed my changes to https://review.opendev.org/c/openstack/cinder/+/907519 to discuss them with upstream.

I was fixing a few Errors and found out, that creating custom properties / extra specs lets the volume creation fail. I am asking Upstream about the details of this.

[josephineSei]

I am investigating a hint that it might be the scheduler, who is responsible for the errors of custom properties. Mailing List Discussion: https://lists.openstack.org/archives/list/[email protected]/thread/ZWXFYIQ3FSFC5MGTSIMVEHCEJRQQRQ3X/

[josephineSei]

I am investigating a hint that it might be the scheduler, who is responsible for the errors of custom properties. Mailing List Discussion: https://lists.openstack.org/archives/list/[email protected]/thread/ZWXFYIQ3FSFC5MGTSIMVEHCEJRQQRQ3X/

The cause of this is indeed the Cinder Scheduler, that checks ALL properties (== extra_specs) here, for their compatability with the backends: https://github.com/openstack/cinder/blob/master/cinder/scheduler/filters/capabilities_filter.py#L56

Changing this Filter would be possible, but whether we would only allow one or two special extra_specs to be skipped in this filter or to cheange the filter using a list of extra_specs to check for their presence and compatability is something we need to discuss with upstream.

[josephineSei]

I attended the Cinder meeting to discuss my findings, but unfortunately there was not enough time, we will continue the discussion via ML or in the next week.

https://meetings.opendev.org/meetings/cinder/2024/cinder.2024-02-07-14.00.log.html

[fkr]

thanks @josephineSei for keeping this updated.

[josephineSei]

After trying to reach out to people and going through the patches again, I added this as a discussion point for the midcycle ptg: https://etherpad.opendev.org/p/cinder-caracal-midcycles

[josephineSei]

To be better prepared for the discussion tomorrow, I try to look for alternative ways to include the visibility of either volume type defined or administrator defined aspects in volume types.

While it may be possible to include a method in the API call for showing a volume type, that gives normal users information about whether an encryption type is present or not, this is not a feasible way for the replication, that is backend-specific. This aspect has to be set by the admin explicitly but is NOT allowed to interact with the volume scheduler, as it might lead to Errors.

mysql> describe volume_types;
+--------------+--------------+------+-----+---------+-------+
| Field        | Type         | Null | Key | Default | Extra |
+--------------+--------------+------+-----+---------+-------+
| created_at   | datetime     | YES  |     | NULL    |       |
| updated_at   | datetime     | YES  |     | NULL    |       |
| deleted_at   | datetime     | YES  |     | NULL    |       |
| deleted      | tinyint(1)   | YES  |     | NULL    |       |
| id           | varchar(36)  | NO   | PRI | NULL    |       |
| name         | varchar(255) | YES  |     | NULL    |       |
| qos_specs_id | varchar(36)  | YES  | MUL | NULL    |       |
| is_public    | tinyint(1)   | YES  |     | NULL    |       |
| description  | varchar(255) | YES  |     | NULL    |       |
+--------------+--------------+------+-----+---------+-------+
9 rows in set (0.01 sec)

mysql> describe volume_type_extra_specs;
+----------------+--------------+------+-----+---------+----------------+
| Field          | Type         | Null | Key | Default | Extra          |
+----------------+--------------+------+-----+---------+----------------+
| created_at     | datetime     | YES  |     | NULL    |                |
| updated_at     | datetime     | YES  |     | NULL    |                |
| deleted_at     | datetime     | YES  |     | NULL    |                |
| deleted        | tinyint(1)   | YES  |     | NULL    |                |
| id             | int          | NO   | PRI | NULL    | auto_increment |
| volume_type_id | varchar(36)  | NO   | MUL | NULL    |                |
| key            | varchar(255) | YES  |     | NULL    |                |
| value          | varchar(255) | YES  |     | NULL    |                |
+----------------+--------------+------+-----+---------+----------------+
8 rows in set (0.00 sec)

mysql> describe encryption;
+------------------+--------------+------+-----+---------+-------+
| Field            | Type         | Null | Key | Default | Extra |
+------------------+--------------+------+-----+---------+-------+
| created_at       | datetime     | YES  |     | NULL    |       |
| updated_at       | datetime     | YES  |     | NULL    |       |
| deleted_at       | datetime     | YES  |     | NULL    |       |
| deleted          | tinyint(1)   | YES  |     | NULL    |       |
| cipher           | varchar(255) | YES  |     | NULL    |       |
| control_location | varchar(255) | NO   |     | NULL    |       |
| key_size         | int          | YES  |     | NULL    |       |
| provider         | varchar(255) | NO   |     | NULL    |       |
| volume_type_id   | varchar(36)  | NO   |     | NULL    |       |
| encryption_id    | varchar(36)  | NO   | PRI | NULL    |       |
+------------------+--------------+------+-----+---------+-------+
10 rows in set (0.01 sec)

The database shows, that the encryption parameters are saved in a different table, while something to simply store information would be the extra_specs table or to create a new additional table. This would have the downside, that next to the extra_specs table there would be a need for an additional metadata table or something alike. This would blow up the database and it would put work on the API methods and maybe even the view of the volume type objects. In that case the extra specs and metadata would share the "properties field" of the volume type object:

openstack volume type show LUKS --encryption-type
+--------------------+----------------------------------------------------------------------+
| Field              | Value                                                                |
+--------------------+----------------------------------------------------------------------+
| access_project_ids | None                                                                 |
| description        | None                                                                 |
| encryption         | cipher='aes-xts-plain64', control_location='front-end',              |
|                    | created_at='2024-02-01T09:33:28.000000', deleted='False',            |
|                    | deleted_at=, encryption_id='d7995c8e-83a4-4535-94ee-151e482dac2e',   |
|                    | key_size='256',                                                      |
|                    | provider='nova.volume.encryptors.luks.LuksEncryptor', updated_at=    |
| id                 | 39bc174e-c075-4296-8a09-f3b274ae0caa                                 |
| is_public          | True                                                                 |
| name               | LUKS                                                                 |
| properties         |                                                                      |
| qos_specs_id       | None                                                                 |
+--------------------+----------------------------------------------------------------------+

A better option would be to put the extra_specs into a separate field and leave the properties field for other metadata.
But I doubt that it would be likely to adjust OpenStack like this.

• In the end I can conclude, that the best option would be to open up the extra_specs to also hold key-value pairs that are not used by the scheduler. (most likely the easiest way to implement - no API or DB changes)
• The second best option would be to define a new field for any user input and information, that works exactly like the properties field and would require a new DB table, new API endpoints and some rework for the views. (API and DB changes, but backwards compatible)
• The third option would be to put the information into the properties field mixed with the extra_specs but only in the API. Having a separation between two tables in the DB and just mixing those inputs, when information is requested. (new DB table, API behavior might change, a lot of work in separating the properties -> imho this is Error prone)
• The last option would be to define a new field and move all extra_specs into that field and keep the properties field for any other user visible information. (DB and API changes, NOT backward compatible, best separation but most unlikely option)

[josephineSei]

I have attended the Cinder midcycle meeting to discuss about the use cases we have and what we want to achieve. I explained everything and we also discussed about several options to achieve this (like the ones I mentioned in the last comment)
Unfortunately they also have no certain direction to push me to. So the discussion on how we can achieve to have it will continue on the ML and may even result in a spec for Cinder. Most likely we either only get an easy way to get the ecnryption OR we might need to adjust DB and API with a new metadata field.

The etherpad of the midcycle with an Action Item for me: https://etherpad.opendev.org/p/cinder-caracal-midcycles

[josephineSei]

I have created a blueprint¹ and am working out a spec for the change in the volume types (still under work, as I also have to consider all DB, API and other impacts ALL options would have right now). I wrote an email to the mailinglist² already containing the options to discuss, so the topic gets more attention.

Footnotes

1. https://blueprints.launchpad.net/cinder/+spec/user-visible-information-in-volume-types ↩

2. https://lists.openstack.org/archives/list/[email protected]/thread/PP7IMXVOO2SM47JRMDYYVB2IA3XIEZD5/ ↩

[josephineSei]

Someone already answered on the ML, but this input does not help for backends with ceph. It just shows exactly the problem we have, when using backend internal configuration to achieve replication, which is transparent for Cinder:

$ sudo ceph osd pool ls detail
....
pool 46 'docker_volumes' replicated size 3 min_size 2 crush_rule 2 object_hash......

$ cinder get-pools --detail
+-----------------------------+----------------------------------------------------------------------------------------------+
| Property                    | Value                                                                                        |
+-----------------------------+----------------------------------------------------------------------------------------------+
| allocated_capacity_gb       | XXX                                                                                          |
| backend_state               | up                                                                                           |
| driver_version              | x.y.0                                                                                        |
| filter_function             | None                                                                                         |
| free_capacity_gb            | xyzxyz.ab                                                                                    |
| goodness_function           | None                                                                                         |
| location_info               | ceph:/etc/ceph/ceph-ext.conf..................                                               |
| max_over_subscription_ratio | xx.x                                                                                         |
| multiattach                 | True                                                                                         |
| name                        | block@three_times_replicated-sitewide#three_times_replicated-sitewide                        |
| provisioned_capacity_gb     | xxx                                                                                          |
| replication_enabled         | False                                                                                        |
| reserved_percentage         | 0                                                                                            |
| storage_protocol            | ceph                                                                                         |
| thin_provisioning_support   | True                                                                                         |
| timestamp                   | 2024-02-16T08:16:13.541591                                                                   |
| total_capacity_gb           | xyzxyz.ab                                                                                    |
| vendor_name                 | Open Source                                                                                  |
| volume_backend_name         | three_times_replicated-sitewide                                                              |
+-----------------------------+----------------------------------------------------------------------------------------------+

I pushed the first version of the spec to gerrit¹. Where I tried to show the impact of the several options on the workflow in Cinder. This is necessary, after we did not come to a decision in the midcycle meeting. I hope we get a decision on what to do until or latest at the next ptg.

Footnotes

1. https://review.opendev.org/c/openstack/cinder-specs/+/909195 ↩

[josephineSei]

I attended the Public Cloud Sig to align with the process of discoverability in overall openstack.
And I also attended the Cinder meeting and put the spec on the review list.

[josephineSei]

I edited the spec according to the review I got and fixed some grammatical errors.

[josephineSei]

I attended the Cinder meeting and again put the spec on the review list.
I also added the spec to the etherpad for the next PTG as a topic to discuss:
https://etherpad.opendev.org/p/dalmatian-ptg-cinder

[josephineSei]

I prepared the spec patch for the next cycle.

[josephineSei]

After having the new document sturcture for the next cycle I updated the spec patch again.
https://review.opendev.org/c/openstack/cinder-specs/+/909195

[josephineSei]

I prepared myself for the dicussion of the different ways for user visible information in volume types in the PTG that will happen today or maybe tomorrow.

[josephineSei]

Results of the PTG discussion

I presented some options on how to tackle the user visibility and we came to an agreement:

1. A new DB table for volume_type_metadata should be exclusively used for information from the deployer for the user.
2. This should be presented in a new metadata field for volume types in the API.
3. To standardize the phrases in the metadata field, metadefs can be used in addition.

Metadef API

The metadef API currently does exist in Glance:
https://docs.openstack.org/api-ref/image/v2/metadefs-index.html

And from this document it will define metadata keys for various resources in OpenStack:
https://docs.openstack.org/glance/latest/user/metadefs-concepts.html

Adding a new definition there seems to be the equivalent to a standard in scs. Users could apply to this but are not forced to.

Nevertheless for SCS it might be worth looking into this metadefs API in a separate issue.

For the scope of this issue this is just a possible second phase after introducing a way for deployers to add metadata to volume types. (#565)

Work items

• rewrite the spec

• add patches to Cinder after the spec was merged (only for the DB and the API)

• add patches to OSC/SDK to be able to add something to the new metadata field

• adjust Standard and Test Script to use the new field (maybe done after the dalmatian release)

2nd Phase:

• add metadef description for encrypted and replicated volume types

[josephineSei]

I updated the spec: https://review.opendev.org/c/openstack/cinder-specs/+/909195

[josephineSei]

stack@devstack:~/devstack$ openstack volume type unset --metadata tiger test
stack@devstack:~/devstack$ openstack volume type show test
+--------------------+--------------------------------------+
| Field              | Value                                |
+--------------------+--------------------------------------+
| access_project_ids | None                                 |
| description        | None                                 |
| id                 | 87f89839-6f55-4548-b041-e2b451dbcf89 |
| is_public          | True                                 |
| metadata           | {'cat': 'miau'}                      |
| name               | test                                 |
| properties         |                                      |
| qos_specs_id       | None                                 |
+--------------------+--------------------------------------+

The things still missing for functionality are: creating a volume type with some metadata
Another thing is the ability to filter by volume type metadata - this is not yet done.

For upstream this is too late for 2024.2, but can be targeted for the next cycle. There are tests missing like unit tests and tempest tests. These are also needed for upstream.

[josephineSei]

I started looking into implementing a filter. When listing all volume types I want to be able to filter by the presence of certain metadata.

[josephineSei]

The filtering mechanism now also works:

stack@devstack:~/devstack$ openstack volume type list --metadata 'tiger'='growl'
+--------------------------------------+------+-----------+
| ID                                   | Name | Is Public |
+--------------------------------------+------+-----------+
| 87f89839-6f55-4548-b041-e2b451dbcf89 | test | True      |
+--------------------------------------+------+-----------+
stack@devstack:~/devstack$ openstack volume type list --metadata 'tiger'='growl' --long
+--------------------------------------+------+-----------+-------------+------------+
| ID                                   | Name | Is Public | Description | Properties |
+--------------------------------------+------+-----------+-------------+------------+
| 87f89839-6f55-4548-b041-e2b451dbcf89 | test | True      | None        |            |
+--------------------------------------+------+-----------+-------------+------------+

But seeing the output, i think I should also adjust the CLI to properly show the metadata in the listing, the information is already given by the API:

RESP BODY: {"volume_types": [{"id": "87f89839-6f55-4548-b041-e2b451dbcf89", "name": "test", "is_public": true, "description": null, "metadata": {"tiger": "growl", "cat": "miau"}, "extra_specs": {}, "qos_specs_id": null, "os-volume-type-access:is_public": true}]}
GET call to volumev3 for http://192.168.23.161/volume/v3/db05e1116cb04ac689d82c738a5f103c/types?metadata=%7B%27tiger%27%3A+%27growl%27%7D&is_public=None used request id req-5a8f83db-9c56-4d61-afeb-ce5eaf7c64f7
+--------------------------------------+------+-----------+
| ID                                   | Name | Is Public |
+--------------------------------------+------+-----------+
| 87f89839-6f55-4548-b041-e2b451dbcf89 | test | True      |
+--------------------------------------+------+-----------+

[josephineSei]

I started taking a look into the failing tests and will also add unit tests.

[josephineSei]

I rebased the patches and added unit tests for the cinderclient. I will continue with the other patches.

[josephineSei]

The pipeline for the client patches is green.
I am now investigating the Errors on the Cinder patches, to prepare for the next PTG.

[josephineSei]

The patch, which adds the database table (https://review.opendev.org/c/openstack/cinder/+/918316) and just also ask for the metadata table content, when the volume types are listed / showed - has only one failing unit test, which is not due to the changes, some mypy failures (also in code parts I did not change) and tempest tests, which require the tempest change to be in place.

The first things cannot be handled by me, and the tempest tests - i looked through the failing tests and it seems that all (also the failing plugin tests) seem to fail validation, which needs the schema adjustment in tempest. So there are two patches which rely on each other, that cannot be merged without each other - I will ask Cinder, how they deal with such things.

Also I think there is no more unit tests needed for this patch then the ones I adjusted and the one I wrote, that checks the db upgrade itself.

I now start looking into the other Cinder patch, that focuses on adding new API endpoints, and the handling between endpoints and the db.

[josephineSei]

While implementing the missing shown metadata column in the openstack volume type list --long answer and trying to fix the unit tests therefore, I once again came to the point to think about microversions. Cinder requires microversions for each new API endpoint / changes response, etc... So the Cinder API part will definitely need a microversion and therefor also the openstackclient and the unit tests in the openstack client have to be aware of that microversion.

I will hold on further implementing unit tests for the client and start digging into the microversion stuff more. So this will not yet be commited to upstream patches:

stack@devstack:~/devstack$ openstack volume type list --long
+--------------------------------------+-------------+-----------+--------------------------------------------------------------------------+------------+
| ID                                   | Name        | Is Public | Description                                                              | Properties |
+--------------------------------------+-------------+-----------+--------------------------------------------------------------------------+------------+
| 87f89839-6f55-4548-b041-e2b451dbcf89 | test        | True      | None                                                                     |            |
| 15002270-b9ed-4b7f-ba08-cea35f3acc1f | LUKS        | True      | [scs:encrypted, replicated] Content will be replicated three times to    |            |
|                                      |             |           | ensure consistency and availability for your data. LUKS encryption is    |            |
|                                      |             |           | used.                                                                    |            |
| 74241c6f-6678-4388-9f69-d16d91793cc1 | ceph        | True      | None                                                                     |            |
| 41a4e540-0660-4562-aeb1-e3891121c88f | __DEFAULT__ | True      | Default Volume Type                                                      |            |
+--------------------------------------+-------------+-----------+--------------------------------------------------------------------------+------------+
clean_up ListVolumeType: 
END return value: 0
stack@devstack:~/devstack$ nano /opt/stack/data/venv/lib/python3.10/site-packages/openstackclient/volume/v3/volume_type.py
stack@devstack:~/devstack$ openstack volume type list --long
+--------------------------------------+-------------+-----------+------------------------------------------------------------+------------+---------------------------+
| ID                                   | Name        | Is Public | Description                                                | Properties | Metadata                  |
+--------------------------------------+-------------+-----------+------------------------------------------------------------+------------+---------------------------+
| 87f89839-6f55-4548-b041-e2b451dbcf89 | test        | True      | None                                                       |            | cat='miau', tiger='growl' |
| 15002270-b9ed-4b7f-ba08-cea35f3acc1f | LUKS        | True      | [scs:encrypted, replicated] Content will be replicated     |            |                           |
|                                      |             |           | three times to ensure consistency and availability for     |            |                           |
|                                      |             |           | your data. LUKS encryption is used.                        |            |                           |
| 74241c6f-6678-4388-9f69-d16d91793cc1 | ceph        | True      | None                                                       |            |                           |
| 41a4e540-0660-4562-aeb1-e3891121c88f | __DEFAULT__ | True      | Default Volume Type                                        |            |                           |
+--------------------------------------+-------------+-----------+------------------------------------------------------------+------------+---------------------------+

[josephineSei]

I started adding a microversion for my patch by following this documentation: /home/kiari/Dokumente/upstream/cinder/api-ref/source/v3/samples/volume_type/volume-type-show-response.json
I am working on point 8 of the "other necessary changes" before I will add the decorators. I pushed my current state of work to this patch: https://review.opendev.org/c/openstack/cinder/+/928794

[josephineSei]

I need to adjust all the patches to work properly with microversions (that means also adjust the openstackclient).
But as of right now it seems to work (at least for that command):

stack@devstack:~/devstack$ openstack volume type show test
+--------------------+--------------------------------------+
| Field              | Value                                |
+--------------------+--------------------------------------+
| access_project_ids | None                                 |
| description        | None                                 |
| id                 | 87f89839-6f55-4548-b041-e2b451dbcf89 |
| is_public          | True                                 |
| name               | test                                 |
| properties         |                                      |
| qos_specs_id       | None                                 |
+--------------------+--------------------------------------+
stack@devstack:~/devstack$ openstack volume type show test --os-volume-api-version 3.72
versions supported by client: 3.0 - 3.71
stack@devstack:~/devstack$ nano /opt/stack/data/venv/lib/python3.10/site-packages/cinderclient/api_versions.py 
stack@devstack:~/devstack$ openstack volume type show test --os-volume-api-version 3.72
+--------------------+-------------------------------------------------------+
| Field              | Value                                                 |
+--------------------+-------------------------------------------------------+
| access_project_ids | None                                                  |
| description        | None                                                  |
| id                 | 87f89839-6f55-4548-b041-e2b451dbcf89                  |
| is_public          | True                                                  |
| metadata           | {'fox': 'ehehehehe', 'tiger': 'growl', 'cat': 'miau'} |
| name               | test                                                  |
| properties         |                                                       |
| qos_specs_id       | None                                                  |
+--------------------+-------------------------------------------------------+

This may also make adjustments of test easier.

[josephineSei]

After introducing the microversion I was able to simplify the first patch to add the database table:
https://review.opendev.org/c/openstack/cinder/+/918316

This is good, because smaller changes are easier to review :)

[josephineSei]

I looked through the failing tests in the database and the API patch, most of them seem to come from other problems. I made a few adjustments to the API patch to fix the failing ones, my patch was responsible for.
I now start adding unit tests for the new API version, hopefully having some ready for the PTG this week

[josephineSei]

I am working on unit tests but can only find small amounts of time due to the PTG and other stuff.

[josephineSei]

We discussed a few points at the PTG and the Cinder teams promised me reviews. I already started implementing some feedback - like kicking out notifications.
Another point is, that I somehow need to test the performance of the db calls, because, with many types and and request the db calls might have a bigger impact on performance. I need to look through this and whether I can test this on my devstack.

[josephineSei]

Back on working on the unit test and thinking about the db performance

[josephineSei]

I marked the spec as active and need to investigate a bit more in unit tests, the first test seems to run on my machine: https://review.opendev.org/c/openstack/cinder/+/928794

[josephineSei]

I stumbled upon a functional unit test, that has a very strange behavior:

When I execute the functional unit test to check the extensions I get an ERROR:

$ tox -efunctional-py39  -vv -- cinder.tests.functional.api_sample_tests.test_extensions.ExtensionsSampleJsonTest.test_extensions
....
....
==============================
Failed 1 tests - output below:
==============================

cinder.tests.functional.api_sample_tests.test_extensions.ExtensionsSampleJsonTest.test_extensions
-------------------------------------------------------------------------------------------------

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 394, in _verify_response
    response_result = self._compare_result(template_data,

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 280, in _compare_result
    matched_value = self._compare_dict(

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 183, in _compare_dict
    res = self._compare_result(expected[key], result[key],

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 284, in _compare_result
    matched_value = self._compare_list(

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 238, in _compare_list
    raise NoMatch('\n'.join(error))

    cinder.tests.functional.api_samples_test_base.NoMatch: Extra list items in Response:
{'alias': 'os-types-metadata', 'description': 'Type metadata support.', 'links': [], 'name': 'TypesMetadata', 'updated': '2024-09-09T00:00:00+00:00'}

When I try to add the new microversion in the extension-list-response sample, I get that Error:

==============================
Failed 1 tests - output below:
==============================

cinder.tests.functional.api_sample_tests.test_extensions.ExtensionsSampleJsonTest.test_extensions
-------------------------------------------------------------------------------------------------

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 416, in _verify_response
    self._compare_result(template_data, sample_data, "Sample")

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 280, in _compare_result
    matched_value = self._compare_dict(

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 183, in _compare_dict
    res = self._compare_result(expected[key], result[key],

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 284, in _compare_result
    matched_value = self._compare_list(

      File ".../cinder/cinder/tests/functional/api_samples_test_base.py", line 238, in _compare_list
    raise NoMatch('\n'.join(error))

    cinder.tests.functional.api_samples_test_base.NoMatch: Extra list items in template:
{'alias': 'os-types-metadata', 'description': 'Type metadata support.', 'links': [], 'name': 'TypesMetadata', 'updated': '%(extension_update)s'}

The failing part here is not in the original test file, but in the abstract test-base class, where three things are compared:

1. The response from the actual call to the function
2. The template I first adjusted
3. a document in the api-ref

This is hard to detect, because the original test does not indicate the not only 2 things are compared, but in fact 3.
This can only be concluded, when going through the base class.

So whenever there is a template to be adjusted, it may not be the only thing to adjust in Cinder so that your unit tests will be successful.

[josephineSei]

I am continuing working on unit tests and attended the Cinder weekly meeting to raise attention for reviews.

[josephineSei]

unit tests adjustments to reflect the new microversion is going into a good direction: https://review.opendev.org/c/openstack/cinder/+/928794

[josephineSei]

Only a few more unit tests to add here: https://review.opendev.org/c/openstack/cinder/+/928794/17/cinder/tests/unit/api/contrib/test_types_metadata.py

[josephineSei]

I finished unit tests, I think. I will take another look in the functional and maybe tempest tests.

[josephineSei]

After fixing some failures in the OpenStack zuuil pipeline, I started looking into creating tempest tests with a microversion. This leads to the question, whether tests should reside in tempest itself or rather in the tempest plugin from Cinder.
For now I think it should be tempest itself, but it could be argued, that these are just API and database calls and thus should rather reside in the cinder tempest plugin. You can read here when tests should be part of tempest and when not.

[josephineSei]

I again raised awareness for the patches in the cinder team meeting and also told them in the cinder channel, that we will not have that much time any more from december on to work on upstream items.
Beside that I worked on the tempest tests.

[josephineSei]

I have a first draft of tempest tests, which should be sufficient in the amount of test cases: https://review.opendev.org/c/openstack/tempest/+/935142
I still need to look into the pipeline and check possible errors.

[josephineSei]

Momentan funktioniert der Aufruf mit der korrekten microversion in meinen tempest tests nicht. Daher schlagen sie fehl: https://review.opendev.org/c/openstack/tempest/+/935142

Ich versuche das Problem anzugehen und zu lösen.

[josephineSei]

I'm still on the tempest tests and also updated the clients to reflect the use of microversions. I still need to test all the patches together.

[josephineSei]

The documentation about the usage of microversions in tempest is not very clear. I am still struggeling with this.
And I missed the dependence of the cinderclient patch on the cinder-API patch. I hope that fixes the CI-pipeline.

[josephineSei]

i fixed the tests for the cinderclient and move forward on the tempest tests with making the tests work with the correct microversion.

[josephineSei]

I tried a few more things to address that the tests use the correct microversion in the tempest tests.

[josephineSei]

The patches are in a good state, I just could not get tempest to accept the microversions:
https://review.opendev.org/q/topic:%22user-visible-metadata-in-volume-types%22

Nevertheless the major part of work is done, it needs reviews from upstream openstack cinder team and hopefully only smaller adjustments.