Bug 1685081 - "volume create --non-bootable --read-write" thinks volume creation is a blocking API, tweaks bootable & readonly flags before new volume becomes "available"
Summary: "volume create --non-bootable --read-write" thinks volume creation is a block...
Keywords:
Status: ON_DEV
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: python-openstackclient
Version: 18.0 (Zed)
Hardware: x86_64
OS: Unspecified
medium
medium
Target Milestone: ---
: ---
Assignee: mathieu bultel
QA Contact: nlevinki
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-03-04 10:36 UTC by mxie@redhat.com
Modified: 2024-04-30 13:23 UTC (History)
12 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 2074801 (view as bug list)
Environment:
Last Closed: 2021-12-31 07:27:01 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
openstack-invaild-volume.log (2.17 MB, text/plain)
2019-03-04 10:36 UTC, mxie@redhat.com
no flags Details


Links
System ID Private Priority Status Summary Last Updated
OpenStack gerrit 845991 0 None NEW Wait for volume being available to set bootable or readonly 2022-06-15 19:55:35 UTC
Red Hat Issue Tracker OSP-14618 0 None None None 2022-04-11 13:48:08 UTC

Description mxie@redhat.com 2019-03-04 10:36:38 UTC
Created attachment 1540588 [details]
openstack-invaild-volume.log

Description of problem:
There is error info about invalid volume ID during the virt-v2v openstack conversion

Version-Release number of selected component (if applicable):
virt-v2v-1.40.2-1.el7.x86_64
libguestfs-1.40.2-1.el7.x86_64
libvirt-client-4.5.0-10.el7_6.6.x86_64
qemu-kvm-rhev-2.12.0-18.el7_6.3.x86_64
nbdkit-1.8.0-1.el7.x86_64
python2-openstackclient-3.16.1-0.20180925042058.3e5a2d2.el7ost.noarch


How reproducible:
100%

Steps to reproduce:
1.Prepare a guest which have installed python2-openstackclient and virt-v2v on openstack environment

# openstack server list
+--------------------------------------+-------------------------------+--------+-----------------------+-----------------------+-----------+
| ID                                   | Name                          | Status | Networks              | Image                 | Flavor    |
+--------------------------------------+-------------------------------+--------+-----------------------+-----------------------+-----------+
| ff074bf3-51ed-4897-9a81-2ba97a69d81e | rhel7.6-v2v-conversion-server | ACTIVE | public01=10.73.224.5  | esx6.7-rhel7.6-x86_64 | m1.medium |
+--------------------------------------+-------------------------------+--------+-----------------------+-----------------------+-----------+

2.Copy keystone_admin from openstack server to guest "rhel7.6-v2v-conversion-server", then source keystone_admin to solve openstack authentication

#source keystone_admin

3.Convert a guest from vmware to openstack by virt-v2v, the conversion could be finished successfully but there is error info about invalid volume ID
# virt-v2v -ic vpx://root.73.141/data/10.73.75.219/?no_verify=1 -it vddk -io vddk-libdir=/home/vmware-vix-disklib-distrib -io  vddk-thumbprint=1F:97:34:5F:B6:C2:BA:66:46:CB:1A:71:76:7D:6B:50:1E:03:00:EA esx6.7-rhel7.6-x86_64 --password-file /tmp/passwd -o openstack -oo server-id=rhel7.6-v2v-conversion-server -on esx6.7-rhel7.6-o-openstack-performance
[  11.3] Opening the source -i libvirt -ic vpx://root.73.141/data/10.73.75.219/?no_verify=1 esx6.7-rhel7.6-x86_64 -it vddk  -io vddk-libdir=/home/vmware-vix-disklib-distrib -io vddk-thumbprint=1F:97:34:5F:B6:C2:BA:66:46:CB:1A:71:76:7D:6B:50:1E:03:00:EA
[  15.1] Creating an overlay to protect the source from being modified
[  19.5] Opening the overlay
[  91.3] Inspecting the overlay
[ 192.7] Checking for sufficient free disk space in the guest
[ 192.7] Estimating space required on target for each disk
[ 192.7] Converting Red Hat Enterprise Linux Server 7.6 (Maipo) to run on KVM
virt-v2v: This guest has virtio drivers installed.
[4003.7] Mapping filesystem data to avoid copying unused and blank areas
[4125.4] Closing the overlay
[4132.8] Assigning disks to buses
[4132.8] Checking if the guest needs BIOS or UEFI to boot
[4132.8] Initializing the target -o openstack
Failed to set volume read-only access mode flag: Invalid volume: Volume c28f106d-d1e8-4a05-9d70-748da89fb2d2 status must be available to update readonly flag, but current status is: creating. (HTTP 400) (Request-ID: req-723c12c9-67d0-43ca-8c12-36c618eea962)
[4176.4] Copying disk 1/1 to /dev/disk/by-id/virtio-c28f106d-d1e8-4a05-9 (raw)
    (100.00/100%)
[5292.4] Creating output metadata
[5315.3] Finishing off


Actual results:
As description

Expected results:
There is no error info about invalid volume ID during the virt-v2v openstack conversion

Additional info:

Comment 4 RHEL Program Management 2021-03-15 07:33:49 UTC
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Comment 5 Richard W.M. Jones 2021-03-15 10:28:00 UTC
My apologies, this bug was closed by an automated process that we have no control
over, and it should not have been.  I am reopening it.

Comment 7 RHEL Program Management 2021-12-31 07:27:01 UTC
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Comment 8 Richard W.M. Jones 2021-12-31 09:17:08 UTC
I have to apologise - this bug was closed in error by a process we have no
control over which closes bugs claiming they are "stale" when they are not.
We have tried to have this process removed, with no success.  I am reopening
it and setting the "stale" date far into the future.

Comment 9 Laszlo Ersek 2022-04-11 13:35:59 UTC
This is a bug in the openstack command line client
(python-openstackclient), introduced in commit 7e5a98bca91e ("Add some
options to "volume create" command", 2016-12-06).

As far as I can tell, the bug persists even today (current master =
dabaec5a7b1b ('Merge "volume: fix backup list of deleted volume"',
2022-04-11)).

Said commit introduced a volume_client.volumes.update_readonly_flag()
API call in both "openstackclient/volume/v1/volume.py" and
"openstackclient/volume/v2/volume.py", immediately following the
volume_client.volumes.create() API call. The expectation (I think) from
the commit's author must have been that the
volume_client.volumes.create() API call is a *blocking* one. I
emphasize: the --read-write / --read-only flag is *not* passed to the
volume creation API immediately, it is a separate API call.

"v1/volume.py", class "CreateVolume", method "take_action":

>         volume = volume_client.volumes.create(
>             parsed_args.size,
>             snapshot,
>             source_volume,
>             parsed_args.name,
>             parsed_args.description,
>             parsed_args.type,
>             user,
>             project,
>             parsed_args.availability_zone,
>             parsed_args.property,
>             image,
>         )
>
>         if parsed_args.bootable or parsed_args.non_bootable:
>             try:
>                 volume_client.volumes.set_bootable(
>                     volume.id, parsed_args.bootable)
>             except Exception as e:
>                 LOG.error(_("Failed to set volume bootable property: %s"), e)
>         if parsed_args.read_only or parsed_args.read_write:
>             try:
>                 volume_client.volumes.update_readonly_flag(
>                     volume.id,
>                     parsed_args.read_only)
>             except Exception as e:
>                 LOG.error(_("Failed to set volume read-only access "
>                             "mode flag: %s"), e)

"v2/volume.py", class "CreateVolume", method "take_action":

>         volume = volume_client.volumes.create(
>             size=size,
>             snapshot_id=snapshot,
>             name=parsed_args.name,
>             description=parsed_args.description,
>             volume_type=parsed_args.type,
>             availability_zone=parsed_args.availability_zone,
>             metadata=parsed_args.property,
>             imageRef=image,
>             source_volid=source_volume,
>             consistencygroup_id=consistency_group,
>             scheduler_hints=parsed_args.hint,
>         )
>
>         if parsed_args.bootable or parsed_args.non_bootable:
>             try:
>                 volume_client.volumes.set_bootable(
>                     volume.id, parsed_args.bootable)
>             except Exception as e:
>                 LOG.error(_("Failed to set volume bootable property: %s"), e)
>         if parsed_args.read_only or parsed_args.read_write:
>             try:
>                 volume_client.volumes.update_readonly_flag(
>                     volume.id,
>                     parsed_args.read_only)
>             except Exception as e:
>                 LOG.error(_("Failed to set volume read-only access "
>                             "mode flag: %s"), e)

Note that, in both cases, the exception is caught, logged, and then
ignored.

However, the volume creation API is not a blocking one. It just launches
the volume creation and the volume may exist in "creating" state for a
good time. The actual error message comes from the server side, namely
the "openstack-cinder" project, file "cinder/volume/api.py":

>     def update_readonly_flag(self,
>                              context: context.RequestContext,
>                              volume: objects.Volume,
>                              flag) -> None:
>         context.authorize(vol_action_policy.UPDATE_READONLY_POLICY,
>                           target_obj=volume)
>         if volume['status'] != 'available':
>             msg = _('Volume %(vol_id)s status must be available '
>                     'to update readonly flag, but current status is: '
>                     '%(vol_status)s.') % {'vol_id': volume['id'],
>                                           'vol_status': volume['status']}
>             raise exception.InvalidVolume(reason=msg)
>         self.update_volume_admin_metadata(context.elevated(), volume,
>                                           {'readonly': str(flag)})
>         LOG.info("Update readonly setting on volume "
>                  "completed successfully.",
>                  resource=volume)

(current as of master @ 85387cae8b36 ('Merge "mypy: ceph backup
driver"', 2022-04-04)).

So, this is not a bug in virt-v2v; virt-v2v complies with
python-openstackclient's command line interface, documented at
<https://docs.openstack.org/python-openstackclient/yoga/cli/command-objects/volume.html>.
The "--read-write" option is *not* documented as something that the
admin should pass separately, after the volume has been created, and
moved from "creating" status to "available" status. Therefore, it is the
above-quoted implementation that has the bug: the command line client
should first poll (wait) for the newly created volume to reach
"available" status, and *then* set the readonly/readwrite flag, with the
separate API call.

If we're annoyed by this error message, then we can just remove both the
"--read-write" flag *and* the "--non-bootable" flag from
"create_cinder_volume" [output/output_openstack.ml] @ 46298c651471.
That's because non-bootable and read-write are the default properties:
refer again to
<https://docs.openstack.org/python-openstackclient/yoga/cli/command-objects/volume.html>.

Note the following comment as well, just above "create_cinder_volume"
[output/output_openstack.ml]:

>     (* Create a new Cinder volume and wait for its status to change to
>      * "available".  Returns the volume id.
>      *)

Virt-v2v actually implements a polling loop for the new volume's status
to change from "creating" to "available", before proceeding. This is
exactly what's missing from "python-openstackclient"!

Finally, I can see that bootability is then set later to the actually
desired value, with the "volume set" command -- but that happens in
"finalize" [output/output_openstack.ml], where we know the volume is
already available. (Whereas "create_cinder_volume" is invoked from
"setup".)

Summary: we could work this around relatively easily, like this:

> diff --git a/output/output_openstack.ml b/output/output_openstack.ml
> index d0af2ac79e7e..aa01d5a67c50 100644
> --- a/output/output_openstack.ml
> +++ b/output/output_openstack.ml
> @@ -277,9 +277,7 @@ The os-* parameters and environment variables are optional.
>        List.push_back_list args [ "volume"; "create";
>                                   "-f"; "json";
>                                   "--size"; size_gb;
> -                                 "--description"; description;
> -                                 "--non-bootable";
> -                                 "--read-write" ];
> +                                 "--description"; description ];
>        Option.may (
>          fun os -> List.push_back_list args [ "--type"; os ]
>        ) output_storage;

but I'm going to do the right here, and reassign the bug to
"python-openstackclient". (Again, AFAICT, even upstream is affected,
with master @ dabaec5a7b1b; so I'm going to select the latest downstream
release -- apparently Xena).

Comment 10 Richard W.M. Jones 2022-04-12 15:18:31 UTC
> Summary: we could work this around relatively easily, like this:
[...]
> but I'm going to do the right here, and reassign the bug to

I wonder if we should do both?  We don't know how long it'll take
to fix openstack and also there's the issue that users might run
virt-v2v with older versions of openstack which don't get the fix.

Comment 11 Laszlo Ersek 2022-04-13 06:55:28 UTC
(In reply to Richard W.M. Jones from comment #10)
> > Summary: we could work this around relatively easily, like this:
> [...]
> > but I'm going to do the right here, and reassign the bug to
> 
> I wonder if we should do both?  We don't know how long it'll take
> to fix openstack and also there's the issue that users might run
> virt-v2v with older versions of openstack which don't get the fix.

I've cloned this BZ into bug 2074801.


Note You need to log in before you can comment on or make changes to this bug.