Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CASMCMS-9145: Create BOS option arch_check_requires_ims #376

Merged
merged 1 commit into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- Added `arch_check_requires_ims` BOS option. This determines whether or not a failure to
get IMS data is considered fatal when validating image architecture in a boot set. By default
this is false. Note that this has no effect for boot sets whose images are not in IMS, nor
for boot sets whose architecture is `Other`.

### Changed
- Refactored some BOS Options code to use abstract base classes, to avoid code duplication.

Expand Down
9 changes: 9 additions & 0 deletions api/openapi.yaml.in
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,15 @@ components:
session_limit_required:
type: boolean
description: If true, Sessions cannot be created without specifying the limit parameter.
arch_check_requires_ims:
type: boolean
description: |
This option modifies how BOS behaves when validating the architecture of a boot image in a boot set.
Specifically, this option comes into play when BOS needs data from IMS in order to do this validation, but
IMS is unreachable.
In the above situation, if this option is true, then the validation will fail.
Otherwise, if the option is false, then a warning will be logged, but the validation will not
be failed because of this.
additionalProperties: true
minProperties: 1
maxProperties: 1024
Expand Down
5 changes: 5 additions & 0 deletions src/bos/common/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
# code should either import this dict directly, or (preferably) access
# its values indirectly using a DefaultOptions object
DEFAULTS = {
'arch_check_requires_ims': False,
'cleanup_completed_session_ttl': "7d",
'clear_stage': False,
'component_actual_state_ttl': "4h",
Expand Down Expand Up @@ -60,6 +61,10 @@ def get_option(self, key: str) -> Any:
# All these do is convert the response to the appropriate type for the option,
# and return it.

@property
def arch_check_requires_ims(self) -> bool:
return bool(self.get_option('arch_check_requires_ims'))

@property
def cleanup_completed_session_ttl(self) -> str:
return str(self.get_option('cleanup_completed_session_ttl'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
from botocore.exceptions import ClientError

from bos.common.options import BaseOptions
from bos.common.utils import exc_type_msg
from bos.common.utils import exc_type_msg, requests_retry_session
from bos.operators.utils.boot_image_metadata import BootImageMetaData, BootImageMetaDataBadRead
from bos.operators.utils.clients.ims import get_image, get_ims_id_from_s3_url, ImageNotFound, \
DEFAULT_IMS_IMAGE_ARCH
from bos.operators.utils.clients.ims import get_arch_from_image_data, get_image, \
get_ims_id_from_s3_url, ImageNotFound
from bos.operators.utils.clients.s3 import S3BootArtifacts, S3MissingConfiguration, S3Url, \
ArtifactNotFound

Expand Down Expand Up @@ -223,25 +223,21 @@ def arch(self):
s3_url)
return None
try:
ims_image_data = get_image(ims_id)
except ImageNotFound:
LOGGER.warning(
"Can't determine architecture of '%s' because image '%s' does not exist in IMS",
s3_url.url, ims_id)
return None
if self.options.arch_check_requires_ims:
image_data = get_image(ims_id)
else:
# If IMS being inaccessible is not a fatal error, then reduce the number
# of retries we make, to prevent a lengthy delay

# A pylint bug generates a false positive error for this call
# https://github.com/pylint-dev/pylint/issues/2271
session=requests_retry_session(retries=4) # pylint: disable=redundant-keyword-arg
image_data = get_image(ims_id, session=session)
return get_arch_from_image_data(image_data)
except Exception as err:
LOGGER.error("Error getting IMS image data for '%s' (S3 path '%s'): %s", ims_id,
s3_url.url, exc_type_msg(err))
# If the image is not found, re-raise the exception
# If it's a different error, and arch_check_requires_ims is set, re-raise the exception
# Otherwise, return None
if isinstance(err, ImageNotFound) or self.options.arch_check_requires_ims:
raise err
return None
try:
return ims_image_data["arch"]
except KeyError:
LOGGER.warning("Defaulting to '%s' because 'arch' field not set in IMS image '%s' "
"(s3 path '%s'): %s", DEFAULT_IMS_IMAGE_ARCH, ims_id, s3_url.url,
ims_image_data)
return DEFAULT_IMS_IMAGE_ARCH
except Exception as err:
LOGGER.error("IMS image '%s': %s", ims_id, ims_image_data)
LOGGER.error("Error getting 'arch' field for IMS image '%s' (s3 path '%s'): %s", ims_id,
s3_url.url, exc_type_msg(err))
return None
22 changes: 22 additions & 0 deletions src/bos/operators/utils/clients/ims.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,25 @@ def get_ims_id_from_s3_url(s3_url: S3Url) -> str|None:
return IMS_S3_KEY_RE_PROG.match(s3_url.key).group(1)
except (AttributeError, IndexError):
return None


def get_arch_from_image_data(image_data: dict) -> str:
"""
Returns the value of the 'arch' field in the image data
If it is not present, logs a warning and returns the default value
"""
try:
arch = image_data['arch']
except KeyError:
LOGGER.warning("Defaulting to '%s' because 'arch' not set in IMS image data: %s",
DEFAULT_IMS_IMAGE_ARCH, image_data)
return DEFAULT_IMS_IMAGE_ARCH
except Exception as err:
LOGGER.error("Unexpected error parsing IMS image data (%s): %s", exc_type_msg(err),
image_data)
raise
if arch:
return arch
LOGGER.warning("Defaulting to '%s' because 'arch' set to null value in IMS image data: %s",
DEFAULT_IMS_IMAGE_ARCH, image_data)
return DEFAULT_IMS_IMAGE_ARCH
8 changes: 6 additions & 2 deletions src/bos/server/controllers/v2/boot_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from bos.common.utils import exc_type_msg
from bos.operators.utils.boot_image_metadata import BootImageMetaData
from bos.operators.utils.boot_image_metadata.factory import BootImageMetaDataFactory
from bos.operators.utils.clients.ims import ImageNotFound
from bos.operators.utils.clients.s3 import S3Object, ArtifactNotFound
from bos.server.controllers.v2.options import OptionsData
from bos.server.utils import canonize_xname, ParsingException
Expand Down Expand Up @@ -214,7 +215,10 @@ def validate_boot_set_arch(bs: dict, image_metadata: BootImageMetaData) -> None:
if arch == 'Other':
raise CannotValidateBootSetArch("Boot set arch set to 'Other'")

ims_image_arch = image_metadata.arch
try:
ims_image_arch = image_metadata.arch
except ImageNotFound as err:
raise CannotValidateBootSetArch(str(err)) from err

if ims_image_arch is None:
raise CannotValidateBootSetArch("Can't determine architecture of boot artifacts")
Expand Down Expand Up @@ -277,7 +281,7 @@ def validate_sanitize_boot_set(bs_name: str, bs_data: dict, options_data: Option
validate_boot_set_arch(bs_data, image_metadata)
except CannotValidateBootSetArch as err:
LOGGER.warning('%s', bs_data)
LOGGER.warning("Bboot set '%s': %s", bs_name, err)
LOGGER.warning("Boot set '%s': %s", bs_name, err)
except Exception as err:
raise ParsingException(f"Error found validating arch of boot set '{bs_name}': " \
f"{exc_type_msg(err)}") from err
Expand Down
Loading