From d0ca4fe9fe5525e6985ea13460b0992fa4c15cf4 Mon Sep 17 00:00:00 2001
From: Alan Bishop <abishop@redhat.com>
Date: Mon, 25 Oct 2021 07:36:24 -0700
Subject: [PATCH] Clear up confusion on cinder's default volume type

This patch clears up confusion resulting from both tripleo and
cinder creating "default" volume types. The quotes indicate subtle
differences in how the term is used, and how it causes confusion for
cloud users.

TripleO added support for configuring cinder's default volume type,
and later cinder itself added its own support for a default volume
type. The cinder project's motivation was to provide a volume type
for all volumes, even when cloud administrators hadn't defined one.
But from tripleo's perspective, cinder's volume type was redundant
because tripleo *does* define a default volume type.

The confusion for cloud users is that cinder chose "__DEFAULT__" for
the name of the volume type, and "Default Volume Type" for its
description. This is misleading because tripleo's CinderDefaultVolumeType
is the actual default volume type.

Clearing up the confusion depends on whether the overcloud is a green
field deployment where no volumes have been created, or a brown field
deployment where cinder's __DEFAULT__ type may be in use. If no volumes
exist then it's safe for tripleo to simply delete cinder's __DEFAULT__
type. Otherwise, the __DEFAULT__ type's description is updated so that
it indicates the actual default type is the one established by the
CinderDefaultVolumeType parameter.

Lastly, CinderDefaultVolumeType is now constrained to prevent it being
set to an empty string. That should never happen, so this is just a
safety net.

Related-Bug: #1782217
Change-Id: Idf27c14b31dc077ef9a0e567bd502ed6842bd52b
(cherry picked from commit 4bf4866030c78c5f9343eb0084d72c4a03beefa7)
(cherry picked from commit f10e5f2e6e43c776eef51173bc0695e8c928cdfb)
(cherry picked from commit b7019a6272a44dac278e63af48c7ec427754e5d0)
---
 .../cinder/cinder-api-container-puppet.yaml     | 17 ++++++++++++++++-
 ...fy-default-volume-type-c77e7a7ddafdf172.yaml | 12 ++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/cinder-clarify-default-volume-type-c77e7a7ddafdf172.yaml

diff --git a/deployment/cinder/cinder-api-container-puppet.yaml b/deployment/cinder/cinder-api-container-puppet.yaml
index ee6f1c254e..242b5c3e68 100644
--- a/deployment/cinder/cinder-api-container-puppet.yaml
+++ b/deployment/cinder/cinder-api-container-puppet.yaml
@@ -54,6 +54,11 @@ parameters:
     default: tripleo
     description: The name of Cinder's default volume type.
     type: string
+    constraints:
+    - allowed_pattern: "[a-zA-Z0-9]+"
+      description: >
+        The default volume type must be at least 1 character and contain only
+        letters and numbers.
   CinderEnableDBPurge:
     default: true
     description: |
@@ -439,7 +444,7 @@ outputs:
             state: absent
           when:
             - step|int == 4
-        - name: Manage Cinder Volume Type
+        - name: Manage Cinder's default volume type
           become: true
           vars:
             default_volume_type: {get_param: CinderDefaultVolumeType}
@@ -452,6 +457,16 @@ outputs:
             if ! openstack volume type show "{{ default_volume_type }}"; then
                 openstack volume type create --public "{{ default_volume_type }}"
             fi
+            eval $(openstack volume type show __DEFAULT__ -f shell -c id -c description)
+            if [ -n "$id" ]; then
+                vols=$(openstack volume list -f value -c ID)
+                tripleo_descr="For internal use, '{{ default_volume_type }}' is the default volume type"
+                if [ -z "$vols" ]; then
+                    openstack volume type delete $id
+                elif [ "$description" != "$tripleo_descr" ]; then
+                    openstack volume type set $id --description "$tripleo_descr"
+                fi
+            fi
           args:
             executable: /bin/bash
           changed_when: false
diff --git a/releasenotes/notes/cinder-clarify-default-volume-type-c77e7a7ddafdf172.yaml b/releasenotes/notes/cinder-clarify-default-volume-type-c77e7a7ddafdf172.yaml
new file mode 100644
index 0000000000..1d5c7af13b
--- /dev/null
+++ b/releasenotes/notes/cinder-clarify-default-volume-type-c77e7a7ddafdf172.yaml
@@ -0,0 +1,12 @@
+---
+other:
+  - |
+    Steps are taken to minimize chances of confusion between the default
+    block storage volume type established by the CinderDefaultVolumeType
+    parameter, and cinder's own __DEFAULT__ volume type.
+
+    In a new deployment where no volumes exist, cinder's __DEFAULT__ type is
+    deleted because it is redundant. In an upgrade scenerio, if volumes exist
+    then the __DEFAULT__ type's description is updated to indicate the actual
+    default volume type is the one established by the CinderDefaultVolumeType
+    parameter.