cinder assigns default volume_type even when it shouldn't

[Brian Rosmaita]

NOTE: the original bug report was about 'cinder_img_volume_type' not being respected, but this applies to other cases in which a user does *not* specify a volume_type as part of a volume_create request, for example, a source_volid or a snapshot_id.

When the 'cinder_img_volume_type' image property is on an image, a volume created from that image is supposed to use the specified volume_type (unless a different volume_type is specified in the volume-create request). Currently, all such create request result in a volume of the default volume_type (either operator configured, or __DEFAULT__).

This is a regression from Change I4da0c13b5b3f8174a30b8557f968d6b9e641b091 in Train.

It happens here at the REST API layer:
[0] https://opendev.org/openstack/cinder/src/commit/85e60732e21ee30937d43d8b77295e4525e1c8fc/cinder/api/v3/volumes.py#L324-L328
(that's from v3, but the same code is in v2)

The change was part of an effort to eliminate untyped volumes in Cinder. The problem is that it sets a volume_type too soon (probably so that the volume_type element in the API response wouldn't be null. But as a result, it bypasses the selection code in the cinder-volume service [1], which would infer the volume_type based on source volume type, snapshot volume type, or image volume type. So basically, unless a user explicitly sets a volume_type in the original API request, that user is going to wind up with the default type.

I think the way to fix this is to (roughly) move the code from [0] into [1]. We could just document that the volume_type can be null in the API response while a volume is still building.

Change I4da0c13b5b3f8174a30b8557f968d6b9e641b091, however, touched a *lot* of files, so there may be some dependencies on the volume_type being set early that I'm not seeing at first glance.

[1] https://opendev.org/openstack/cinder/src/commit/85e60732e21ee30937d43d8b77295e4525e1c8fc/cinder/volume/flows/api/create_volume.py#L356-L371

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/730183

[Brian Rosmaita (brian-rosmaita)]

Added a test to cinder-tempest-plugin for this:
https://review.opendev.org/737380

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/730183
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=674c8e7286999bb6408291de1dc6395aac2b04b1
Submitter: Zuul
Branch: master

commit 674c8e7286999bb6408291de1dc6395aac2b04b1
Author: Brian Rosmaita <email address hidden>
Date: Wed May 20 16:15:26 2020 -0400

    Default volume_type set too early

    If a volume_type is not specified in a volume-create request, change
    I4da0c13b5b3f8174a30b8557f968d6b9e641b091 (introduced in Train) sets a
    default volume_type in the REST API layer. This prevents the
    selection logic in cinder.volume.flows.api.create_volume.
    ExtractVolumeRequestTask from being able to infer the appropriate
    volume_type from the source volume, snapshot, or image metadata, and
    has caused a regression where the created volume is of the default
    type instead of the inferred type.

    This patch removes setting the default volume_type in the REST API
    and modifies the selection code in ExtractVolumeRequestTask slightly
    to make sure a volume_type is always assigned in that function, and
    adds and revises some tests.

    Change-Id: I05915f2e32b1229ad320cd1c5748de3d63183b91
    Closes-bug: #1879578

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/738791

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/738791
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=c1bdb233cfd5b32a8b60b481e67abc9b4613893c
Submitter: Zuul
Branch: stable/ussuri

commit c1bdb233cfd5b32a8b60b481e67abc9b4613893c
Author: Brian Rosmaita <email address hidden>
Date: Wed May 20 16:15:26 2020 -0400

    Default volume_type set too early

    If a volume_type is not specified in a volume-create request, change
    I4da0c13b5b3f8174a30b8557f968d6b9e641b091 (introduced in Train) sets a
    default volume_type in the REST API layer. This prevents the
    selection logic in cinder.volume.flows.api.create_volume.
    ExtractVolumeRequestTask from being able to infer the appropriate
    volume_type from the source volume, snapshot, or image metadata, and
    has caused a regression where the created volume is of the default
    type instead of the inferred type.

    This patch removes setting the default volume_type in the REST API
    and modifies the selection code in ExtractVolumeRequestTask slightly
    to make sure a volume_type is always assigned in that function, and
    adds and revises some tests.

    Change-Id: I05915f2e32b1229ad320cd1c5748de3d63183b91
    Closes-bug: #1879578
    (cherry picked from commit 674c8e7286999bb6408291de1dc6395aac2b04b1)
    Conflicts:
      cinder/tests/unit/volume/flows/api/test_create_volume.py
      - cinder.tests.unit.test (victoria) -> cinder.test (pre-victoria)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/train
Review: https://review.opendev.org/740598

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/740598
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=5122b146589368189ae1645985cc10030c90ecc5
Submitter: Zuul
Branch: stable/train

commit 5122b146589368189ae1645985cc10030c90ecc5
Author: Brian Rosmaita <email address hidden>
Date: Wed May 20 16:15:26 2020 -0400

    Default volume_type set too early

    If a volume_type is not specified in a volume-create request, change
    I4da0c13b5b3f8174a30b8557f968d6b9e641b091 (introduced in Train) sets a
    default volume_type in the REST API layer. This prevents the
    selection logic in cinder.volume.flows.api.create_volume.
    ExtractVolumeRequestTask from being able to infer the appropriate
    volume_type from the source volume, snapshot, or image metadata, and
    has caused a regression where the created volume is of the default
    type instead of the inferred type.

    This patch removes setting the default volume_type in the REST API
    and modifies the selection code in ExtractVolumeRequestTask slightly
    to make sure a volume_type is always assigned in that function, and
    adds and revises some tests.

    Change-Id: I05915f2e32b1229ad320cd1c5748de3d63183b91
    Closes-bug: #1879578
    (cherry picked from commit 674c8e7286999bb6408291de1dc6395aac2b04b1)
    Conflicts:
      cinder/tests/unit/volume/flows/api/test_create_volume.py
      - cinder.tests.unit.test (victoria) -> cinder.test (pre-victoria)
    (cherry picked from commit c1bdb233cfd5b32a8b60b481e67abc9b4613893c)
    Conflicts:
      cinder/volume/flows/api/create_volume.py
      - add six, remove collections
      cinder/tests/unit/volume/flows/api/test_create_volume.py
      - add mock, remove unittest.mock

[David Coronel (davecore)]

I confirm that cinder version 2:15.4.0-0ubuntu1~cloud0 from bionic-train/proposed fixes my issue in LP #1885357 (Default volume_type set too early).

I can now create volumes from snapshots of non default types.

[Vern Hart (vern)]

When booting from an image, copy to volume, the default volume type (which has the property RESKEY:availability_zones='zone1') causes a failure when trying to boot an instance in zone2.

I had hoped that this bug's fix would resolve my problem but it does not.

Does that mean the nova "boot from a volume copied from an image" workflow is different? Can this be solved in cinder or do we need to hack nova to somehow use a different default volume-type depending on the zone?

[Brian Rosmaita (brian-rosmaita)]

@Vern: we don't currently have per-zone default types. You could try:

- make the default volume type not specify an AZ and be generic enough that the type can be built in any of your AZs, or

- create a new volume type for boot from volume scenarios that doesn't specify an AZ and is generic enough that the type can be built in any of your AZs, and then add the name of this volume type as the image property 'cinder_img_volume_type' to each image

[Brian Rosmaita (brian-rosmaita)]

This fix was released in 15.4.0 (train) and 16.2.0 (ussuri)