From e80e2511cf825671a479053cc8d41463aab1caaa Mon Sep 17 00:00:00 2001
From: Artom Lifshitz <alifshit@redhat.com>
Date: Tue, 24 Jan 2017 12:27:15 -0500
Subject: [PATCH] Fix tag attribute disappearing in 2.33 and 2.37

In the context of device tagging, bugs have caused the tag attribute
to disappear starting with version 2.33 for block_devices and starting
with version 2.37 for network interfaces. In other words, block
devices could only be tagged in 2.32 and network interfaces between
2.32 and 2.36 inclusively.

This patch documents this behaviour in api-ref and introduces
microversion 2.42, which re-adds the tag in all the right places.

Change-Id: Ia0869dc6f7f5bd347ccbd0930d1d668d37695a22
Closes-bug: 1658571
Implements: blueprint fix-tag-attribute-disappearing
---
 api-ref/source/parameters.yaml                | 52 ++++++++++++++-----
 api-ref/source/servers.inc                    |  4 +-
 .../servers/v2.42/server-create-req.json      | 18 +++++++
 .../servers/v2.42/server-create-resp.json     | 22 ++++++++
 .../versions/v21-version-get-resp.json        |  2 +-
 .../versions/versions-get-resp.json           |  2 +-
 nova/api/openstack/api_version_request.py     |  6 ++-
 .../openstack/compute/block_device_mapping.py | 14 ++++-
 .../compute/rest_api_version_history.rst      | 24 +++++++++
 .../compute/schemas/block_device_mapping.py   | 12 ++---
 nova/api/openstack/compute/schemas/servers.py | 24 +++++++++
 nova/api/openstack/compute/servers.py         |  7 ++-
 .../servers/v2.42/server-create-req.json.tpl  | 18 +++++++
 .../servers/v2.42/server-create-resp.json.tpl | 22 ++++++++
 .../api_sample_tests/test_servers.py          |  9 ++++
 ...-device-role-tagging-7cfdb14f2ba4fbcf.yaml | 10 ++++
 16 files changed, 219 insertions(+), 27 deletions(-)
 create mode 100644 doc/api_samples/servers/v2.42/server-create-req.json
 create mode 100644 doc/api_samples/servers/v2.42/server-create-resp.json
 create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl
 create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-resp.json.tpl
 create mode 100644 releasenotes/notes/fix-virtual-device-role-tagging-7cfdb14f2ba4fbcf.yaml

diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml
index 748ca64a7154..efbf6971fbae 100644
--- a/api-ref/source/parameters.yaml
+++ b/api-ref/source/parameters.yaml
@@ -1174,11 +1174,14 @@ block_device_mapping_v2:
             "delete_on_termination": true,
             "tag": "disk1" }]
 
-    Starting in microversion 2.32, the tag is an optional, arbitrary attribute
-    that can be used to assign a tag to the block device. This tag is then
-    exposed to the guest in the metadata API and the config drive and is
-    associated to hardware metadata for that block device, such as bus (ex:
-    SCSI), bus address (ex: 1:0:2:0), and serial.
+    In microversion 2.32, ``tag`` is an optional string attribute that can
+    be used to assign a tag to the block device. This tag is then exposed to
+    the guest in the metadata API and the config drive and is associated to
+    hardware metadata for that block device, such as bus (ex: SCSI), bus
+    address (ex: 1:0:2:0), and serial.
+
+    A bug has caused the ``tag`` attribute to no longer be accepted starting
+    with version 2.33. It has been restored in version 2.42.
   in: body
   required: false
   type: object
@@ -1501,9 +1504,28 @@ device_resp:
   in: body
   required: true
   type: string
-device_tag:
+device_tag_bdm:
   description: |
-    An arbitrary tag.
+    A device role tag that can be applied to a block device. The guest OS of a
+    server that has devices tagged in this manner can access hardware metadata
+    about the tagged devices from the metadata API and on the config drive, if
+    enabled.
+
+    .. note:: Due to a bug, block device tags are accepted in version 2.32 and
+      subsequently starting with version 2.42.
+  in: body
+  required: false
+  type: string
+  min_version: 2.32
+device_tag_nic:
+  description: |
+    A device role tag that can be applied to a network interface. The guest OS
+    of a server that has devices tagged in this manner can access hardware
+    metadata about the tagged devices from the metadata API and on the config
+    drive, if enabled.
+
+    .. note:: Due to a bug, network interface tags are accepted between 2.32
+      and 2.36 inclusively, and subsequently starting with version 2.42.
   in: body
   required: false
   type: string
@@ -3168,12 +3190,16 @@ networks:
     object.  To provision the server instance with a NIC for an already existing port,
     specify the port-id in the ``port`` attribute in a ``networks`` object.
 
-    Starting in microversion 2.32, it's possible to optionally assign an
-    arbitrary tag to a virtual network interface, specify the tag attribute in
-    the ``network`` object. An interface's tag is exposed to the guest in the
-    metadata API and the config drive and is associated to hardware metadata
-    for that network interface, such as bus (ex: PCI), bus address (ex:
-    0000:00:02.0), and MAC address.
+    In microversion 2.32, the ``tag`` is an optional string attribute that
+    can be used to assign a tag to a virtual network interface. This tag is
+    then exposed to the guest in the metadata API and the config drive and is
+    associated to hardware metadata for that network interface, such as bus
+    (ex: PCI), bus address (ex: 0000:00:02.0), and MAC address.
+
+    A bug has caused the ``tag`` attribute to no longer be accepted starting
+    with version 2.37. Therefore, network interfaces could only be tagged in
+    versions 2.32 to 2.36 inclusively. Version 2.42 has restored the ``tag``
+    attribute.
 
     Starting with microversion 2.37, this field is required and the special
     values *auto* and *none* can be specified for networks. *auto* tells the
diff --git a/api-ref/source/servers.inc b/api-ref/source/servers.inc
index 2aea292283f3..c80dcdb7e621 100644
--- a/api-ref/source/servers.inc
+++ b/api-ref/source/servers.inc
@@ -293,7 +293,7 @@ Request
   - networks.uuid: network_uuid
   - networks.port: port
   - networks.fixed_ip: fixed_ip
-  - networks.tag: device_tag
+  - networks.tag: device_tag_nic
   - personality: personality
   - block_device_mapping_v2: block_device_mapping_v2
   - block_device_mapping_v2.device_name: device_name
@@ -303,7 +303,7 @@ Request
   - block_device_mapping_v2.guest_format: guest_format
   - block_device_mapping_v2.boot_index: boot_index
   - block_device_mapping_v2.uuid: block_device_uuid
-  - block_device_mapping_v2.tag: device_tag
+  - block_device_mapping_v2.tag: device_tag_bdm
   - config_drive: config_drive
   - key_name: key_name
   - os:scheduler_hints: os:scheduler_hints
diff --git a/doc/api_samples/servers/v2.42/server-create-req.json b/doc/api_samples/servers/v2.42/server-create-req.json
new file mode 100644
index 000000000000..4b000b235c06
--- /dev/null
+++ b/doc/api_samples/servers/v2.42/server-create-req.json
@@ -0,0 +1,18 @@
+{
+    "server" : {
+        "name" : "device-tagging-server",
+        "flavorRef" : "http://openstack.example.com/flavors/1",
+        "networks" : [{
+            "uuid" : "ff608d40-75e9-48cb-b745-77bb55b5eaf2",
+            "tag": "nic1"
+        }],
+        "block_device_mapping_v2": [{
+            "uuid": "70a599e0-31e7-49b7-b260-868f441e862b",
+            "source_type": "image",
+            "destination_type": "volume",
+            "boot_index": 0,
+            "volume_size": "1",
+            "tag": "disk1"
+        }]
+    }
+}
\ No newline at end of file
diff --git a/doc/api_samples/servers/v2.42/server-create-resp.json b/doc/api_samples/servers/v2.42/server-create-resp.json
new file mode 100644
index 000000000000..dd0bb9f2284e
--- /dev/null
+++ b/doc/api_samples/servers/v2.42/server-create-resp.json
@@ -0,0 +1,22 @@
+{
+    "server": {
+        "OS-DCF:diskConfig": "AUTO",
+        "adminPass": "S5wqy9sPYUvU",
+        "id": "97108291-2fd7-4dc2-a909-eaae0306a6a9",
+        "links": [
+            {
+                "href": "http://openstack.example.com/v2.1/6f70656e737461636b20342065766572/servers/97108291-2fd7-4dc2-a909-eaae0306a6a9",
+                "rel": "self"
+            },
+            {
+                "href": "http://openstack.example.com/6f70656e737461636b20342065766572/servers/97108291-2fd7-4dc2-a909-eaae0306a6a9",
+                "rel": "bookmark"
+            }
+        ],
+        "security_groups": [
+            {
+                "name": "default"
+            }
+        ]
+    }
+}
\ No newline at end of file
diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json
index 8f9bfd8aa5a7..64120de26c91 100644
--- a/doc/api_samples/versions/v21-version-get-resp.json
+++ b/doc/api_samples/versions/v21-version-get-resp.json
@@ -19,7 +19,7 @@
             }
         ],
         "status": "CURRENT",
-        "version": "2.41",
+        "version": "2.42",
         "min_version": "2.1",
         "updated": "2013-07-23T11:33:21Z"
     }
diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json
index e424cf7a9d29..27ad9b370360 100644
--- a/doc/api_samples/versions/versions-get-resp.json
+++ b/doc/api_samples/versions/versions-get-resp.json
@@ -22,7 +22,7 @@
                 }
             ],
             "status": "CURRENT",
-            "version": "2.41",
+            "version": "2.42",
             "min_version": "2.1",
             "updated": "2013-07-23T11:33:21Z"
         }
diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py
index 3b3b6bccc2b2..dde18537e449 100644
--- a/nova/api/openstack/api_version_request.py
+++ b/nova/api/openstack/api_version_request.py
@@ -98,6 +98,10 @@ REST_API_VERSION_HISTORY = """REST API Version History:
     * 2.39 - Deprecates image-metadata proxy API
     * 2.40 - Adds simple tenant usage pagination support.
     * 2.41 - Return uuid attribute for aggregates.
+    * 2.42 - In the context of device tagging at instance boot time,
+             re-introduce the tag attribute that, due to bugs, was lost
+             starting with version 2.33 for block devices and starting with
+             version 2.37 for network interfaces.
 """
 
 # The minimum and maximum versions of the API supported
@@ -106,7 +110,7 @@ REST_API_VERSION_HISTORY = """REST API Version History:
 # Note(cyeoh): This only applies for the v2.1 API once microversions
 # support is fully merged. It does not affect the V2 API.
 _MIN_API_VERSION = "2.1"
-_MAX_API_VERSION = "2.41"
+_MAX_API_VERSION = "2.42"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 # Almost all proxy APIs which related to network, images and baremetal
diff --git a/nova/api/openstack/compute/block_device_mapping.py b/nova/api/openstack/compute/block_device_mapping.py
index 71fae8645cd9..a6f5f7d188df 100644
--- a/nova/api/openstack/compute/block_device_mapping.py
+++ b/nova/api/openstack/compute/block_device_mapping.py
@@ -17,6 +17,7 @@
 
 from webob import exc
 
+from nova.api.openstack import api_version_request
 from nova.api.openstack.compute.schemas import block_device_mapping as \
                                                   schema_block_device_mapping
 from nova.api.openstack import extensions
@@ -74,7 +75,16 @@ class BlockDeviceMapping(extensions.V21APIExtensionBase):
             create_kwargs['legacy_bdm'] = False
 
     def get_server_create_schema(self, version):
-        if version == '2.32':
-            return schema_block_device_mapping.server_create_v232
+        request_version = api_version_request.APIVersionRequest(version)
+        version_242 = api_version_request.APIVersionRequest('2.42')
+
+        # NOTE(artom) the following conditional was merged as
+        # "if version == '2.32'" The intent all along was to check whether
+        # version was greater than or equal to 2.32. In other words, we wanted
+        # to support tags in versions 2.32 and up, but ended up supporting them
+        # in version 2.32 only. Since we need a new microversion to add request
+        # body attributes, tags have been re-added in version 2.42.
+        if version == '2.32' or request_version >= version_242:
+            return schema_block_device_mapping.server_create_with_tags
         else:
             return schema_block_device_mapping.server_create
diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst
index a3917d567859..c21466270817 100644
--- a/nova/api/openstack/compute/rest_api_version_history.rst
+++ b/nova/api/openstack/compute/rest_api_version_history.rst
@@ -336,6 +336,13 @@ user documentation.
   with 'nic1' will appear in the metadata along with its bus (PCI), bus address
   (ex: 0000:00:02.0), MAC address, and tag ('nic1').
 
+  .. note:: A bug has caused the tag attribute to no longer be accepted for
+    networks starting with version 2.37 and for block_device_mapping_v2
+    starting with version 2.33. In other words, networks could only be tagged
+    between versions 2.32 and 2.36 inclusively and block devices only in
+    version 2.32. As of version 2.42 the tag attribute has been restored and
+    both networks and block devices can be tagged again.
+
 2.33
 ----
 
@@ -344,6 +351,10 @@ user documentation.
 
     GET /v2.1/{tenant_id}/os-hypervisors?marker={hypervisor_id}&limit={limit}
 
+  In the context of device tagging at server create time, 2.33 also removes the
+  tag attribute from block_device_mapping_v2. This is a bug that is fixed in
+  2.42, in which the tag attribute is reintroduced.
+
 2.34
 ----
 
@@ -406,6 +417,10 @@ user documentation.
   Also, the ``uuid`` field in the ``networks`` object in the server create
   request is now strictly enforced to be in UUID format.
 
+  In the context of device tagging at server create time, 2.37 also removes the
+  tag attribute from networks. This is a bug that is fixed in 2.42, in which
+  the tag attribute is reintroduced.
+
 2.38 (Maximum in Newton)
 ------------------------
 
@@ -453,3 +468,12 @@ user documentation.
   `/os-aggregates` endpoint. This attribute is auto-generated upon creation of
   an aggregate. The `os-aggregates` API resource endpoint remains an
   administrator-only API.
+
+2.42
+----
+
+  In the context of device tagging at server create time, a bug has caused the
+  tag attribute to no longer be accepted for networks starting with version
+  2.37 and for block_device_mapping_v2 starting with version 2.33. Microversion
+  2.42 restores the tag parameter to both networks and block_device_mapping_v2,
+  allowing networks and block devices to be tagged again.
diff --git a/nova/api/openstack/compute/schemas/block_device_mapping.py b/nova/api/openstack/compute/schemas/block_device_mapping.py
index 8c68f8ac7526..e2183c915e72 100644
--- a/nova/api/openstack/compute/schemas/block_device_mapping.py
+++ b/nova/api/openstack/compute/schemas/block_device_mapping.py
@@ -69,17 +69,17 @@ server_create = {
     }
 }
 
-block_device_mapping_v232_new_item = {
+block_device_mapping_with_tags_new_item = {
     'tag': parameter_types.tag
 }
 
-block_device_mapping_v232 = copy.deepcopy(block_device_mapping)
-block_device_mapping_v232['properties'].update(
-    block_device_mapping_v232_new_item)
+block_device_mapping_with_tags = copy.deepcopy(block_device_mapping)
+block_device_mapping_with_tags['properties'].update(
+    block_device_mapping_with_tags_new_item)
 
-server_create_v232 = {
+server_create_with_tags = {
     'block_device_mapping_v2': {
         'type': 'array',
-        'items': block_device_mapping_v232
+        'items': block_device_mapping_with_tags
     }
 }
diff --git a/nova/api/openstack/compute/schemas/servers.py b/nova/api/openstack/compute/schemas/servers.py
index 91c5b093face..efe31b21a79a 100644
--- a/nova/api/openstack/compute/schemas/servers.py
+++ b/nova/api/openstack/compute/schemas/servers.py
@@ -105,6 +105,30 @@ base_create_v237['properties']['server']['properties']['networks'] = {
     ]}
 
 
+# 2.42 builds on 2.37 and re-introduces the tag field to the list of network
+# objects.
+base_create_v242 = copy.deepcopy(base_create_v237)
+base_create_v242['properties']['server']['properties']['networks'] = {
+    'oneOf': [
+        {'type': 'array',
+         'items': {
+             'type': 'object',
+             'properties': {
+                 'fixed_ip': parameter_types.ip_address,
+                 'port': {
+                     'oneOf': [{'type': 'string', 'format': 'uuid'},
+                               {'type': 'null'}]
+                 },
+                 'uuid': {'type': 'string', 'format': 'uuid'},
+                 'tag': parameter_types.tag,
+             },
+             'additionalProperties': False,
+         },
+        },
+        {'type': 'string', 'enum': ['none', 'auto']},
+    ]}
+
+
 base_update = {
     'type': 'object',
     'properties': {
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index c2f4b1db059a..a5fc979ca346 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -77,6 +77,7 @@ class ServersController(wsgi.Controller):
 
     schema_server_create_v232 = schema_servers.base_create_v232
     schema_server_create_v237 = schema_servers.base_create_v237
+    schema_server_create_v242 = schema_servers.base_create_v242
 
     @staticmethod
     def _add_location(robj):
@@ -169,6 +170,9 @@ class ServersController(wsgi.Controller):
                 invoke_kwds={"extension_info": self.extension_info},
                 propagate_map_exceptions=True)
         if list(self.create_schema_manager):
+            self.create_schema_manager.map(self._create_extension_schema,
+                                           self.schema_server_create_v242,
+                                           '2.42')
             self.create_schema_manager.map(self._create_extension_schema,
                                            self.schema_server_create_v237,
                                            '2.37')
@@ -517,7 +521,8 @@ class ServersController(wsgi.Controller):
     @validation.schema(schema_server_create, '2.1', '2.18')
     @validation.schema(schema_server_create_v219, '2.19', '2.31')
     @validation.schema(schema_server_create_v232, '2.32', '2.36')
-    @validation.schema(schema_server_create_v237, '2.37')
+    @validation.schema(schema_server_create_v237, '2.37', '2.41')
+    @validation.schema(schema_server_create_v242, '2.42')
     def create(self, req, body):
         """Creates a new server for a given user."""
 
diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl
new file mode 100644
index 000000000000..87a6f49f04f5
--- /dev/null
+++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl
@@ -0,0 +1,18 @@
+{
+    "server" : {
+        "name" : "device-tagging-server",
+        "flavorRef" : "%(host)s/flavors/1",
+        "networks" : [{
+            "uuid" : "ff608d40-75e9-48cb-b745-77bb55b5eaf2",
+            "tag": "nic1"
+        }],
+        "block_device_mapping_v2": [{
+            "uuid": "%(image_id)s",
+            "source_type": "image",
+            "destination_type": "volume",
+            "boot_index": 0,
+            "volume_size": "1",
+            "tag": "disk1"
+        }]
+    }
+}
diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-resp.json.tpl
new file mode 100644
index 000000000000..4b30e0cfbdb8
--- /dev/null
+++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-resp.json.tpl
@@ -0,0 +1,22 @@
+{
+    "server": {
+        "OS-DCF:diskConfig": "AUTO",
+        "adminPass": "%(password)s",
+        "id": "%(id)s",
+        "links": [
+            {
+                "href": "%(versioned_compute_endpoint)s/servers/%(uuid)s",
+                "rel": "self"
+            },
+            {
+                "href": "%(compute_endpoint)s/servers/%(uuid)s",
+                "rel": "bookmark"
+            }
+        ],
+        "security_groups": [
+            {
+                "name": "default"
+            }
+        ]
+    }
+}
diff --git a/nova/tests/functional/api_sample_tests/test_servers.py b/nova/tests/functional/api_sample_tests/test_servers.py
index ade1f10f809c..f46a9833d7e9 100644
--- a/nova/tests/functional/api_sample_tests/test_servers.py
+++ b/nova/tests/functional/api_sample_tests/test_servers.py
@@ -193,6 +193,15 @@ class ServersSampleJson237Test(ServersSampleBase):
         self._post_server(use_common_server_api_samples=False)
 
 
+class ServersSampleJson242Test(ServersSampleBase):
+    microversion = '2.42'
+    sample_dir = 'servers'
+    scenarios = [('v2_42', {'api_major_version': 'v2.1'})]
+
+    def test_servers_post(self):
+        self._post_server(use_common_server_api_samples=False)
+
+
 class ServersUpdateSampleJsonTest(ServersSampleBase):
 
     def test_update_server(self):
diff --git a/releasenotes/notes/fix-virtual-device-role-tagging-7cfdb14f2ba4fbcf.yaml b/releasenotes/notes/fix-virtual-device-role-tagging-7cfdb14f2ba4fbcf.yaml
new file mode 100644
index 000000000000..7448664baee5
--- /dev/null
+++ b/releasenotes/notes/fix-virtual-device-role-tagging-7cfdb14f2ba4fbcf.yaml
@@ -0,0 +1,10 @@
+fixes:
+  - |
+    In the context of virtual device role tagging at server create time, the
+    2.42 microversion restores the tag attribute to networks and
+    block_device_mapping_v2. A bug has caused the tag attribute to no longer be
+    accepted starting with version 2.33 for block_device_mapping_v2 and
+    starting with version 2.37 for networks. In other words, block devices
+    could only be tagged in version 2.32 and network interfaces between
+    versions 2.32 and 2.36 inclusively. Starting with 2.42, both network
+    interfaces and block devices can be tagged again.