starlingx/config
Code Issues Proposed changes

Preserve states for functional user and bmc image

Browse Source 
Currently, there are two kinds of functional images: user and bmc.
However there is no attribute to distinguish between the two kinds.
If an user image is already applied, applying bmc image would delete
the state of the applied user image.

A new parameter is added to specify a functional BMC image.
The state of last updated user image and BMC image are to be kept.
The order in which the two kinds of BMC images is applied determine
which one is to be kept.
If a BMC image with retimer is applied before a BMC image without
retimer, the states of both images must be kept.
If a BMC image without retimer is applied before a BMC image with
retimer, the state of the BMC image without retimer can be deleted.

Tox unit tests are added for the various scenarios.

Closes-Bug: 1951602

Change-Id: Ifdcbc09d9f14270a57ecc307fbea7517b048351b
Signed-off-by: Teresa Ho <teresa.ho@windriver.com>
This commit is contained in:
Teresa Ho 2021-11-17 19:04:47 -05:00
parent 27f4d85c65
commit 2b23452a1c
9 changed files with 223 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
api-ref/source/api-ref-sysinv-v1-config.rst
View File

@ -5137,6 +5137,7 @@ itemNotFound (404)
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"applied_labels (Optional)", "plain", "xsd:list", "The device image applied to the device labels."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
"uuid (Optional)", "plain", "csapi:UUID", "The universally unique identifier for this object."
"links (Optional)", "plain", "xsd:list", "For convenience, resources contain links to themselves. This allows a client to easily obtain rather than construct resource URIs. The following types of link relations are associated with resources: a self link containing a versioned link to the resource, and a bookmark link containing a permanent link to a resource that is appropriate for long term storage."
@ -5156,6 +5157,7 @@ itemNotFound (404)
"description": null,
"name": null,
"image_version": null,
"bmc": false,
"retimer_included": false,
"applied_labels":
{
@ -5235,6 +5237,7 @@ itemNotFound (404)
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"applied_labels (Optional)", "plain", "xsd:list", "The device image applied to the device labels."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
"uuid (Optional)", "plain", "csapi:UUID", "The universally unique identifier for this object."
"links (Optional)", "plain", "xsd:list", "For convenience, resources contain links to themselves. This allows a client to easily obtain rather than construct resource URIs. The following types of link relations are associated with resources: a self link containing a versioned link to the resource, and a bookmark link containing a permanent link to a resource that is appropriate for long term storage."
@ -5254,6 +5257,7 @@ itemNotFound (404)
"description": null,
"name": null,
"image_version": null,
"bmc": false,
"retimer_included": false,
"applied_labels":
{
@ -5293,6 +5297,7 @@ badMediaType (415)
"name (Optional)", "plain", "xsd:string", "The name of the device image."
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
**Response parameters**
@ -5310,6 +5315,7 @@ badMediaType (415)
"name (Optional)", "plain", "xsd:string", "The name of the device image."
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
"applied_labels (Optional)", "plain", "xsd:list", "The device image applied to the device labels."
"uuid (Optional)", "plain", "csapi:UUID", "The universally unique identifier for this object."
@ -5329,6 +5335,7 @@ badMediaType (415)
"description": null,
"name": null,
"image_version": null,
"bmc": false,
"retimer_included": false,
"applied_labels": null
}
@ -5380,6 +5387,7 @@ badMediaType (415)
"name (Optional)", "plain", "xsd:string", "The name of the device image."
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
"applied_labels (Optional)", "plain", "xsd:list", "The device image applied to the device labels."
"uuid (Optional)", "plain", "csapi:UUID", "The universally unique identifier for this object."
@ -5399,6 +5407,7 @@ badMediaType (415)
"description": null,
"name": null,
"image_version": null,
"bmc": false,
"retimer_included": false,
"applied_labels":
{
@ -5454,6 +5463,7 @@ badMediaType (415)
"name (Optional)", "plain", "xsd:string", "The name of the device image."
"description (Optional)", "plain", "xsd:string", "The description of the device image."
"image_version (Optional)", "plain", "xsd:string", "The version of the device image."
"bmc (Optional)", "plain", "xsd:boolean", "This indicates whether it is a BMC functional image."
"retimer_included (Optional)", "plain", "xsd:boolean", "This indicates whether the retimer firmware is included in the BMC functional image."
"applied_labels (Optional)", "plain", "xsd:list", "The device image applied to the device labels."
"uuid (Optional)", "plain", "csapi:UUID", "The universally unique identifier for this object."
@ -5473,6 +5483,7 @@ badMediaType (415)
"description": null,
"name": null,
"image_version": null,
"bmc": false,
"retimer_included": false,
"applied_labels": null
}

4
sysinv/cgts-client/cgts-client/cgtsclient/v1/device_image.py
View File

@ -1,5 +1,5 @@
#
# Copyright (c) 2020 Wind River Systems, Inc.
# Copyright (c) 2020-2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
@ -12,7 +12,7 @@ from cgtsclient import exc
CREATION_ATTRIBUTES = [
'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version', 'uuid', 'retimer_included']
'name', 'description', 'image_version', 'uuid', 'bmc', 'retimer_included']
class DeviceImage(base.Resource):

13
sysinv/cgts-client/cgts-client/cgtsclient/v1/device_image_shell.py
View File

@ -1,5 +1,5 @@
#
# Copyright (c) 2020 Wind River Systems, Inc.
# Copyright (c) 2020-2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
@ -13,7 +13,7 @@ def _print_device_image_show(obj):
'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version',
'applied', 'applied_labels', 'retimer_included']
'applied', 'applied_labels', 'bmc', 'retimer_included']
if isinstance(obj, dict):
data = [(f, obj.get(f, '')) for f in fields]
@ -37,11 +37,11 @@ def do_device_image_list(cc, args):
labels = ['uuid', 'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version', 'retimer_included',
'name', 'description', 'image_version', 'bmc', 'retimer_included',
'applied', 'applied_labels']
fields = ['uuid', 'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version', 'retimer_included',
'name', 'description', 'image_version', 'bmc', 'retimer_included',
'applied', 'applied_labels']
device_images = cc.device_image.list()
utils.print_list(device_images, fields, labels, sortby=1)
@ -81,6 +81,9 @@ def do_device_image_list(cc, args):
@utils.arg('-u', '--uuid',
metavar='<uuid>',
help='UUID of the device image')
@utils.arg('--bmc',
metavar='<true/false>',
help='BMC image')
@utils.arg('--retimer-included',
metavar='<true/false>',
help='Retimer firmware included in BMC FW binary')
@ -93,7 +96,7 @@ def do_device_image_upload(cc, args):
field_list = ['uuid', 'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version', 'retimer_included']
'name', 'description', 'image_version', 'bmc', 'retimer_included']
# Prune input fields down to required/expected values
user_fields = dict((k, v) for (k, v) in vars(args).items()

57
sysinv/sysinv/sysinv/sysinv/api/controllers/v1/device_image.py
View File

@ -1,9 +1,10 @@
#
# Copyright (c) 2020 Wind River Systems, Inc.
# Copyright (c) 2020-2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
from distutils.util import strtobool
import os
import pecan
from pecan import expose
@ -81,6 +82,9 @@ class DeviceImage(base.APIBase):
image_version = wtypes.text
"The version of the device image"
bmc = bool
"Represent the functional image is a BMC image"
retimer_included = bool
"Retimer firmware included in BMC firmware binary"
@ -112,7 +116,7 @@ class DeviceImage(base.APIBase):
['id', 'uuid', 'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version',
'applied', 'applied_labels', 'retimer_included'])
'applied', 'applied_labels', 'bmc', 'retimer_included'])
# insert applied labels for this device image if they exist
device_image = _get_applied_labels(device_image)
@ -249,7 +253,7 @@ class DeviceImageController(rest.RestController):
field_list = ['uuid', 'bitstream_type', 'pci_vendor', 'pci_device',
'bitstream_id', 'key_signature', 'revoke_key_id',
'name', 'description', 'image_version', 'retimer_included']
'name', 'description', 'image_version', 'bmc', 'retimer_included']
data = dict((k, v) for (k, v) in pecan.request.POST.items()
if k in field_list and not (v is None))
msg = _validate_syntax(data)
@ -434,9 +438,21 @@ def _validate_bitstream_type(dev_img):
elif (dev_img['bitstream_type'] == dconstants.BITSTREAM_TYPE_KEY_REVOCATION and
'revoke_key_id' not in dev_img):
msg = _("revoke_key_id is required for key revocation bitstream type")
elif (dev_img['bitstream_type'] != dconstants.BITSTREAM_TYPE_FUNCTIONAL and
'bmc' in dev_img.keys()):
msg = _("bmc option is only applicable to functional image")
elif (dev_img['bitstream_type'] != dconstants.BITSTREAM_TYPE_FUNCTIONAL and
'retimer_included' in dev_img.keys()):
msg = _("retimer_included option is only applicable to functional BMC image")
elif dev_img['bitstream_type'] == dconstants.BITSTREAM_TYPE_FUNCTIONAL:
bmc = False
retimer_included = False
if 'bmc' in dev_img.keys():
bmc = bool(strtobool(dev_img['bmc']))
if 'retimer_included' in dev_img.keys():
retimer_included = bool(strtobool(dev_img['retimer_included']))
if not bmc and retimer_included:
msg = _("retimer_included option is only applicable to functional BMC image")
return msg
@ -483,6 +499,10 @@ def _validate_syntax(device_image):
not cutils.is_uuid_like(device_image['uuid'])):
msg = _("uuid must be a valid UUID")
return msg
if ('bmc' in device_image.keys() and
not cutils.is_valid_boolstr(device_image['bmc'])):
msg = _("Parameter bmc must be a valid bool string")
return msg
if ('retimer_included' in device_image.keys() and
not cutils.is_valid_boolstr(device_image['retimer_included'])):
msg = _("Parameter retimer_included must be a valid bool string")
@ -566,16 +586,27 @@ def process_device_image_apply(pcidevice_id, device_image, label_id=None):
# return False for nothing to do
response = False
else:
# Remove the existing device_image_state record
pecan.request.dbapi.device_image_state_destroy(r.uuid)
# Remove the existing device image label if any
if label_id:
try:
img_lbl = pecan.request.dbapi.device_image_label_get_by_image_label(
img.id, label_id)
pecan.request.dbapi.device_image_label_destroy(img_lbl.uuid)
except exception.DeviceImageLabelNotFoundByKey:
pass
do_remove = True
# If the new image and the applied image are of a different kind (user/bmc),
# do not remove the image.
# If the new BMC image does not have retimer and an applied BMC image
# does have retimer, keep that applied device image state.
if device_image.bmc != img.bmc:
do_remove = False
if (device_image.bmc and
(not device_image.retimer_included and img.retimer_included)):
do_remove = False
if do_remove:
# Remove the existing device_image_state record
pecan.request.dbapi.device_image_state_destroy(r.uuid)
# Remove the existing device image label if any
if label_id:
try:
img_lbl = pecan.request.dbapi.device_image_label_get_by_image_label(
img.id, label_id)
pecan.request.dbapi.device_image_label_destroy(img_lbl.uuid)
except exception.DeviceImageLabelNotFoundByKey:
pass
elif img.bitstream_type == dconstants.BITSTREAM_TYPE_KEY_REVOCATION:
if img.id == device_image.id:
if r.status in [dconstants.DEVICE_IMAGE_UPDATE_COMPLETED,

23
sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/123_device_image_bmc.py Normal file
View File

@ -0,0 +1,23 @@
#
# Copyright (c) 2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
from sqlalchemy import Boolean, Column, MetaData, Table
ENGINE = 'InnoDB'
CHARSET = 'utf8'
def upgrade(migrate_engine):
meta = MetaData()
meta.bind = migrate_engine
dev_img_functional = Table('device_images_functional', meta, autoload=True)
dev_img_functional.create_column(Column('bmc', Boolean, default=False))
def downgrade(migrate_engine):
# Downgrade is unsupported in this release.
raise NotImplementedError('SysInv database downgrade is unsupported.')

1
sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py
View File

@ -1593,6 +1593,7 @@ class DeviceImageFunctional(DeviceImageCommon, DeviceImage):
__tablename__ = 'device_images_functional'
bitstream_id = Column(String(255), nullable=True)
bmc = Column(Boolean, nullable=False, default=False)
retimer_included = Column(Boolean, nullable=False, default=False)
__mapper_args__ = {

4
sysinv/sysinv/sysinv/sysinv/objects/device_image.py
View File

@ -1,5 +1,5 @@
#
# Copyright (c) 2020 Wind River Systems, Inc.
# Copyright (c) 2020-2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
@ -27,6 +27,7 @@ class DeviceImage(base.SysinvObject):
'image_version': utils.str_or_none,
'applied': utils.bool_or_none,
'capabilities': utils.dict_or_none,
'bmc': utils.bool_or_none,
'retimer_included': utils.bool_or_none,
}
@ -36,6 +37,7 @@ class DeviceImage(base.SysinvObject):
'name',
'description',
'image_version',
'bmc',
'retimer_included'}
@base.remotable_classmethod

130
sysinv/sysinv/sysinv/sysinv/tests/api/test_device_image.py
View File

@ -1,5 +1,5 @@
#
# Copyright (c) 2020 Wind River Systems, Inc.
# Copyright (c) 2020-2021 Wind River Systems, Inc.
#
# SPDX-License-Identifier: Apache-2.0
#
@ -295,6 +295,25 @@ class TestPostDeviceImage(TestDeviceImage, dbbase.ControllerHostTestCase):
self.assertIn("retimer_included option is only applicable to"
" functional BMC image", str(result))
def test_create_functional_image_non_bmc_with_retimer(self):
# Test creation of device image
bitstream_file = os.path.join(os.path.dirname(__file__), "data",
'bitstream.bit')
data = {
'bitstream_type': dconstants.BITSTREAM_TYPE_ROOT_KEY,
'pci_vendor': fpga_constants.N3000_VENDOR,
'pci_device': fpga_constants.N3000_DEVICE,
'key_signature': '12345',
'bmc': True,
}
upload_file = [('file', bitstream_file)]
result = self.post_with_files('/device_images', data,
upload_files=upload_file,
headers=self.API_HEADERS,
expect_errors=True)
self.assertIn("bmc option is only applicable to"
" functional image", str(result))
def test_create_bitstream_type_invalid(self):
# Test creation of device image
bitstream_file = os.path.join(os.path.dirname(__file__), "data",
@ -362,6 +381,20 @@ class TestPatch(TestDeviceImage):
pci_vendor='80ee',
pci_device='beef',
bitstream_id='6789')
self.device_image_bmc = dbutils.create_test_device_image(
bitstream_type=dconstants.BITSTREAM_TYPE_FUNCTIONAL,
pci_vendor='80ee',
pci_device='beef',
bitstream_id='0x2300011001030F',
bmc=True,
retimer_included=False)
self.device_image_bmc_retimer = dbutils.create_test_device_image(
bitstream_type=dconstants.BITSTREAM_TYPE_FUNCTIONAL,
pci_vendor='80ee',
pci_device='beef',
bitstream_id='0x2300011001030F',
bmc=True,
retimer_included=True)
def test_device_image_apply_all_hosts(self):
# Test applying device image to all hosts with fpga devices
@ -459,6 +492,101 @@ class TestPatch(TestDeviceImage):
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that an entry of image to device mapping is updated
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image2.id, self.pci_device.id)
self.assertEqual(self.device_image2.id, dev_img_state.image_id)
def test_device_image_apply_functional_user_bmc(self):
# Test applying second device image with label
# Assign label to a device
self.post_json('/device_labels',
[{'pcidevice_uuid': self.pci_device.uuid},
{'key1': 'value1'}],
headers=self.API_HEADERS)
# Apply the device user image
path = '/device_images/%s?action=apply' % self.device_image.uuid
response = self.patch_json(path, [{'key1': 'value1'}],
headers=self.API_HEADERS)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that an entry of image to device mapping is updated
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image.id, self.pci_device.id)
self.assertEqual(dconstants.DEVICE_IMAGE_UPDATE_PENDING,
dev_img_state.status)
# Test 1: Apply a functional BMC device image
path = '/device_images/%s?action=apply' % self.device_image_bmc.uuid
response = self.patch_json(path, [{'key1': 'value1'}],
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that the entries for both images exist
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image.id, self.pci_device.id)
self.assertEqual(self.device_image.id, dev_img_state.image_id)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image_bmc.id, self.pci_device.id)
self.assertEqual(self.device_image_bmc.id, dev_img_state.image_id)
# Test 2: Apply a functional BMC retimer device image
path = '/device_images/%s?action=apply' % self.device_image_bmc_retimer.uuid
response = self.patch_json(path, [{'key1': 'value1'}],
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that the old bmc image is replaced with new bmc image with retimer
state_list = self.dbapi.device_image_state_get_list()
self.assertEqual(len(state_list), 2)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image.id, self.pci_device.id)
self.assertEqual(self.device_image.id, dev_img_state.image_id)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image_bmc_retimer.id, self.pci_device.id)
self.assertEqual(self.device_image_bmc_retimer.id, dev_img_state.image_id)
# Test 3: Apply a BMC image w/o retimer
path = '/device_images/%s?action=apply' % self.device_image_bmc.uuid
response = self.patch_json(path, [{'key1': 'value1'}],
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that all three states exist, the old BMC retimer image state exists
state_list = self.dbapi.device_image_state_get_list()
self.assertEqual(len(state_list), 3)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image.id, self.pci_device.id)
self.assertEqual(self.device_image.id, dev_img_state.image_id)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image_bmc_retimer.id, self.pci_device.id)
self.assertEqual(self.device_image_bmc_retimer.id, dev_img_state.image_id)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image_bmc.id, self.pci_device.id)
self.assertEqual(self.device_image_bmc.id, dev_img_state.image_id)
# Test 4: Apply a BMC image with retimer
path = '/device_images/%s?action=apply' % self.device_image_bmc_retimer.uuid
response = self.patch_json(path, [{'key1': 'value1'}],
headers=self.API_HEADERS,
expect_errors=True)
self.assertEqual(response.content_type, 'application/json')
self.assertEqual(response.status_code, http_client.OK)
# Verify that state for BMC image without retimer is deleted
state_list = self.dbapi.device_image_state_get_list()
self.assertEqual(len(state_list), 2)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image.id, self.pci_device.id)
self.assertEqual(self.device_image.id, dev_img_state.image_id)
dev_img_state = self.dbapi.device_image_state_get_by_image_device(
self.device_image_bmc_retimer.id, self.pci_device.id)
self.assertEqual(self.device_image_bmc_retimer.id, dev_img_state.image_id)
def test_device_image_remove_all_hosts(self):
# Test removing device image for all hosts with fpga devices
# Remove the device image

2
sysinv/sysinv/sysinv/sysinv/tests/db/utils.py
View File

@ -1609,6 +1609,8 @@ def get_test_device_image(**kw):
'name': kw.get('name'),
'description': kw.get('description'),
'version': kw.get('version'),
'bmc': kw.get('bmc'),
'retimer_included': kw.get('retimer_included'),
}
return device_image