diff --git a/docs/releases/pending/hardware-from-file-or-blob.fmf b/docs/releases/pending/hardware-from-file-or-blob.fmf new file mode 100644 index 0000000000..a8aadfd8a3 --- /dev/null +++ b/docs/releases/pending/hardware-from-file-or-blob.fmf @@ -0,0 +1,39 @@ +description: | + Values accepted by the ``hardware`` key of + :tmt:story:`/spec/plans/provision` plugins change: + + * The ``hardware`` key in plan metadata still accepts the + :tmt:story:`hardware requirement specification`, + that does not change: + + .. code-block:: yaml + + provision: + hardware: + cpu: + model: 79 + + * Individual ``key=value`` snippets passed via command-line + are no longer allowed: ``--hardware cpu.mnodel=79`` will not + work anymore. + + * The ``hardware`` key now accepts also JSON or YAML file references + using the ``@`` notation: + + .. code-block:: yaml + + provision: + hardware: "@hw.yaml" + + * Command-line now accepts both full JSON blobs, + ``--hardware '{"cpu": {"model": 79}}'`` or JSON and YAML file + references using the ``@`` notation, + ``--hardware @hw.json``. + + .. note:: + + Since plan metadata are written using FMF, which is a YAML + extension, and YAML can be rewritten as JSON, ``hardware`` key + can be seen as accepting "JSON blob" too, which effectively + aligns ``--hardware`` CLI option with ``hardware`` key in terms + of supported input types. diff --git a/tests/provision/hardware/data/hardware.json b/tests/provision/hardware/data/hardware.json new file mode 100644 index 0000000000..311ffac98f --- /dev/null +++ b/tests/provision/hardware/data/hardware.json @@ -0,0 +1,3 @@ +{ + "hostname": "baz.redhat.com" +} diff --git a/tests/provision/hardware/data/hardware.yaml b/tests/provision/hardware/data/hardware.yaml new file mode 100644 index 0000000000..b4afc94ccc --- /dev/null +++ b/tests/provision/hardware/data/hardware.yaml @@ -0,0 +1 @@ +hostname: quux.redhat.com diff --git a/tests/provision/hardware/test.sh b/tests/provision/hardware/test.sh index f1cc5e099c..91960588b0 100755 --- a/tests/provision/hardware/test.sh +++ b/tests/provision/hardware/test.sh @@ -43,6 +43,27 @@ rlJournalStart rlAssertGrep "Invalid unit: expected a data size unit" $rlRun_LOG rlPhaseEnd + rlPhaseStartTest "Check various input methods for hardware key" + # Just the fmf input that's already in the plan + rlRun -s "tmt -vv run --id $run --dry provision plan --name '^/plan/$PROVISION_HOW\$'" + rlAssertGrep "method: bios" $rlRun_LOG + + # Invalid input, key=val no longer supported + rlRun -s "tmt -vv run --id $run --dry provision --how $PROVISION_HOW --hardware hostname=bar.redhat.com plan --name '^/plan/$PROVISION_HOW\$'" 2 + + # JSON blob via command line + rlRun -s "tmt -vv run --id $run --dry provision --how $PROVISION_HOW --hardware '{\"hostname\": \"bar.redhat.com\"}' plan --name '^/plan/$PROVISION_HOW\$'" + rlAssertGrep "hostname: bar.redhat.com" $rlRun_LOG + + # JSON blob from a file + rlRun -s "tmt -vv run --id $run --dry provision --how $PROVISION_HOW --hardware @$(pwd)/hardware.json plan --name '^/plan/$PROVISION_HOW\$'" + rlAssertGrep "hostname: baz.redhat.com" $rlRun_LOG + + # YAML blob from a file + rlRun -s "tmt -vv run --id $run --dry provision --how $PROVISION_HOW --hardware @$(pwd)/hardware.yaml plan --name '^/plan/$PROVISION_HOW\$'" + rlAssertGrep "hostname: quux.redhat.com" $rlRun_LOG + rlPhaseEnd + rlPhaseStartCleanup rlRun "popd" rlRun "rm -r $run" 0 "Remove run directory" diff --git a/tests/unit/test_hardware.py b/tests/unit/test_hardware.py index ab450cc7a2..372111fe7a 100644 --- a/tests/unit/test_hardware.py +++ b/tests/unit/test_hardware.py @@ -149,47 +149,6 @@ def test_normalize_hardware(root_logger) -> None: tmt.guest.normalize_hardware('', spec, root_logger) -@pytest.mark.parametrize( - ('spec', 'expected_exc', 'expected_message'), - [ - ( - ('disk[1].size=15GB', 'disk.size=20GB'), - tmt.utils.SpecificationError, - r"^Hardware requirement 'disk\.size=20GB' lacks entry index \(disk\[N\]\)\.$", - ), - ( - ('network[1].type=eth', 'network.type=eth'), - tmt.utils.SpecificationError, - r"^Hardware requirement 'network\.type=eth' lacks entry index \(network\[N\]\)\.$", - ), - ( - ('disk=20GB',), - tmt.utils.SpecificationError, - r"^Hardware requirement 'disk=20GB' lacks child property \(disk\[N\].M\)\.$", - ), - ( - ('network=eth',), - tmt.utils.SpecificationError, - r"^Hardware requirement 'network=eth' lacks child property \(network\[N\].M\)\.$", - ), - ], - ids=[ - 'disk.size lacks index', - 'network.size lacks index', - 'disk lacks child property', - 'network lacks child property', - ], -) -def test_normalize_invalid_hardware( - spec: tmt.hardware.constraints.Spec, - expected_exc: type[Exception], - expected_message: str, - root_logger, -) -> None: - with pytest.raises(expected_exc, match=expected_message): - tmt.guest.normalize_hardware('', spec, root_logger) - - FULL_HARDWARE_REQUIREMENTS = """ beaker: pool: "!= foo.*" diff --git a/tmt/guest/__init__.py b/tmt/guest/__init__.py index c8c09b366a..a1c35459af 100644 --- a/tmt/guest/__init__.py +++ b/tmt/guest/__init__.py @@ -33,7 +33,6 @@ import tmt import tmt.hardware -import tmt.hardware.constraints import tmt.log import tmt.package_managers import tmt.steps @@ -65,6 +64,7 @@ Path, ProvisionError, ShellScript, + _normalize_structured_blob, configure_constant, effective_workdir_root, ) @@ -1133,7 +1133,7 @@ def _flag(field: str, label: str) -> tuple[str, str, str]: def normalize_hardware( key_address: str, - raw_hardware: Union[None, tmt.hardware.constraints.Spec, tmt.hardware.Hardware], + raw_hardware: Any, logger: tmt.log.Logger, ) -> Optional[tmt.hardware.Hardware]: """ @@ -1150,101 +1150,15 @@ def normalize_hardware( if isinstance(raw_hardware, tmt.hardware.Hardware): return raw_hardware - # From command line - if isinstance(raw_hardware, (list, tuple)): - merged: dict[str, Any] = {} - - for raw_datum in raw_hardware: - components = tmt.hardware.constraints.ConstraintComponents.from_spec(raw_datum) - - if ( - components.name not in tmt.hardware.constraints.CHILDLESS_CONSTRAINTS - and components.child_name is None - ): - raise tmt.utils.SpecificationError( - f"Hardware requirement '{raw_datum}' lacks " - f"child property ({components.name}[N].M)." - ) - - if ( - components.name in tmt.hardware.constraints.INDEXABLE_CONSTRAINTS - and components.peer_index is None - ): - raise tmt.utils.SpecificationError( - f"Hardware requirement '{raw_datum}' lacks entry index ({components.name}[N])." - ) - - if components.peer_index is not None: - # This should not happen, the test above already ruled - # out `child_name` being `None`, but mypy does not know - # everything is fine. - assert components.child_name is not None # narrow type - - if components.name not in merged: - merged[components.name] = [] - - # Calculate the number of placeholders needed. - placeholders = components.peer_index - len(merged[components.name]) + 1 - - # Fill in empty spots between the existing ones and the - # one we're adding with placeholders. - if placeholders > 0: - merged[components.name].extend([{} for _ in range(placeholders)]) - - merged[components.name][components.peer_index][components.child_name] = ( - f'{components.operator} {components.value}' - ) - - elif components.name == 'cpu' and components.child_name == 'flag': - if components.name not in merged: - merged[components.name] = {} - - if 'flag' not in merged['cpu']: - merged['cpu']['flag'] = [] - - merged['cpu']['flag'].append(f'{components.operator} {components.value}') - - elif components.child_name: - if components.name not in merged: - merged[components.name] = {} - - merged[components.name][components.child_name] = ( - f'{components.operator} {components.value}' - ) - - else: - merged[components.name] = f'{components.operator} {components.value}' - - # Very crude, we will need something better to handle `and` and - # `or` and nesting. - def _drop_placeholders(data: dict[str, Any]) -> dict[str, Any]: - new_data: dict[str, Any] = {} - - for key, value in data.items(): - if isinstance(value, list): - new_data[key] = [] - - for item in value: - if isinstance(item, dict) and not item: - continue - - new_data[key].append(item) - - else: - new_data[key] = value - - return new_data - - # TODO: if the index matters - and it does, because `disk[0]` is - # often a "root disk" - we need sparse list. Cannot prune - # placeholders now, because it would turn `disk[1]` into `disk[0]`, - # overriding whatever was set for the root disk. - # https://github.com/teemtee/tmt/issues/3004 for tracking. - # merged = _drop_placeholders(merged) - - return tmt.hardware.Hardware.from_spec(merged) + raw_hardware = _normalize_structured_blob( + key_address, + raw_hardware, + "hardware requirements, " + "'@' followed by a JSON or YAML filename containing hardware requirements, " + "or a JSON blob containing hardware requirements", + logger, + ) - # From fmf return tmt.hardware.Hardware.from_spec(raw_hardware) @@ -1321,7 +1235,6 @@ class GuestData( Hardware requirements the provisioned guest must satisfy. """, metavar='KEY=VALUE', - multiple=True, normalize=normalize_hardware, serialize=lambda hardware: hardware.to_spec() if hardware else None, unserialize=lambda serialized: ( diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py index 110ce4a677..91f3792f74 100644 --- a/tmt/utils/__init__.py +++ b/tmt/utils/__init__.py @@ -6209,6 +6209,68 @@ def normalize_data_amount( raise NormalizationError(key_address, raw_value, 'a quantity or a string') +def _normalize_structured_blob( + key_address: str, + raw_value: Any, + expected_type: str, + logger: tmt.log.Logger, +) -> Any: + if raw_value is None: + return None + + if isinstance(raw_value, str): + raw_content: str + loader: Callable[[str], Any] + + def _detect_loader() -> Callable[[str], Any]: + if raw_value.lower().endswith(('.yaml', '.yml')): + return from_yaml + + return from_json + + if is_url(raw_value): + loader = _detect_loader() + + # Create retry session for longer retries, see #1229 + session = retry_session.create( + allowed_methods=('GET',), + logger=logger, + ) + + try: + response = session.get(raw_value) + response.raise_for_status() + + raw_content = response.text + + except requests.RequestException as error: + raise GeneralError(f"Failed to download from URL '{raw_value}'.") from error + + elif raw_value.startswith('@'): + if not raw_value[1:]: + raise NormalizationError(key_address, raw_value, expected_type) + + filepath = Path(raw_value[1:]) + + if not filepath.is_file(): + raise GeneralError(f"File '{filepath}' doesn't exist.") + + loader = _detect_loader() + raw_content = filepath.read_text() + + else: + loader = from_json + raw_content = raw_value + + try: + return loader(raw_content) + + except Exception as exc: + raise NormalizationError(key_address, raw_value, expected_type) from exc + + return raw_value + + # TODO: once we replace our custom "containers" with pydantic's `MetadataContainer`, # this enum and `_field_value_sources` should move there. class FieldValueSource(enum.Enum):