Skip to content

NO-JIRA: Make hardware requirements more readable#10500

Open
zaneb wants to merge 1 commit intoopenshift:mainfrom
zaneb:hw-requirements-json
Open

NO-JIRA: Make hardware requirements more readable#10500
zaneb wants to merge 1 commit intoopenshift:mainfrom
zaneb:hw-requirements-json

Conversation

@zaneb
Copy link
Copy Markdown
Member

@zaneb zaneb commented Apr 14, 2026

Store the hardware requirements in a separate JSON file, and process it using jq.

Log the final value to stderr in human-readable form before writing the shell-encoded data to the environment file.

Summary by CodeRabbit

  • Chores
    • Added a centralized default hardware-requirements profile to standardize sizing across roles.
    • Improved the configuration script with stricter error handling and dynamic adjustment of disk-size requirements based on detected manifests.
  • Tests
    • Updated agent ignition tests to expect the new hardware-requirements data in generated outputs.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 14, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@zaneb: This pull request explicitly references no jira issue.

Details

In response to this:

Store the hardware requirements in a separate JSON file, and process it using jq.

Log the final value to stderr in human-readable form before writing the shell-encoded data to the environment file.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8bd0f876-3590-4f53-b0be-7d3b01e27ae1

📥 Commits

Reviewing files that changed from the base of the PR and between edb10c2 and 441a0ed.

📒 Files selected for processing (3)
  • data/data/agent/files/usr/local/bin/configure-assisted-hw-requirements.sh
  • data/data/agent/files/usr/local/share/assisted-service/default_hw_requirements.json
  • pkg/asset/agent/image/ignition_test.go
✅ Files skipped from review due to trivial changes (2)
  • pkg/asset/agent/image/ignition_test.go
  • data/data/agent/files/usr/local/share/assisted-service/default_hw_requirements.json

Walkthrough

A Bash helper script was converted to strict mode and reworked to compute hardware validator requirements by applying conditional jq transforms to an external baseline JSON file. A new baseline hardware requirements JSON was added, and a unit test helper was updated to expect the new file in generated Ignition assets.

Changes

Cohort / File(s) Summary
Script refactor
data/data/agent/files/usr/local/bin/configure-assisted-hw-requirements.sh
Enabled strict Bash (set -euo pipefail), replaced hardcoded env-file path with DATA_DIR, removed manual JSON interpolation, added validator_query() and hw_requirements() to build a jq program and inject computed HW_VALIDATOR_REQUIREMENTS into the env file.
Baseline requirements data
data/data/agent/files/usr/local/share/assisted-service/default_hw_requirements.json
Added a new default hardware requirements profile with CPU, RAM, disk_size_gb, and installation disk-speed thresholds for master, arbiter, worker, and sno, plus network thresholds for non-SNO roles.
Test update
pkg/asset/agent/image/ignition_test.go
Updated test helper (commonFiles()) to include the new default_hw_requirements.json so generated Ignition output includes the added data file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main objective of the PR: refactoring hardware requirements storage from hardcoded/interpolated values to a separate JSON file for better readability.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The PR modifies only the commonFiles() helper function to add default_hw_requirements.json without changing any Ginkgo test titles or introducing dynamic content.
Test Structure And Quality ✅ Passed The PR change adds a single line to the commonFiles() helper function in a traditional Go unit test file, not a Ginkgo test. This minimal data list modification does not involve test logic, assertions, or Ginkgo framework usage.
Microshift Test Compatibility ✅ Passed The pull request does not add any new Ginkgo e2e tests. The investigation confirmed that pkg/asset/agent/image/ignition_test.go uses standard Go testing with testify assertions (not Ginkgo), and contains zero Ginkgo test declarations (It, Describe, Context, When). The only modification to this file is adding one line to the commonFiles() helper function to include the new default_hw_requirements.json file path. Since no new Ginkgo tests are introduced in this PR, the MicroShift compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests are added. Changes are limited to a shell script update, a new JSON data file, and modifications to an existing test helper function.
Topology-Aware Scheduling Compatibility ✅ Passed PR adds hardware requirements data file and utility script without introducing any scheduling constraints or deployment manifests with affinity rules.
Ote Binary Stdout Contract ✅ Passed PR modifies shell script, JSON data file, and test helper code only; no OTE binary process-level code changes detected.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This pull request does not add any new Ginkgo e2e tests. The PR consists of a shell script for hardware requirements processing, a new JSON data configuration file, and a one-line update to an existing test helper function.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from pawanpinjarkar and rwsu April 14, 2026 03:38
@zaneb
Copy link
Copy Markdown
Member Author

zaneb commented Apr 16, 2026

/retest

1 similar comment
@zaneb
Copy link
Copy Markdown
Member Author

zaneb commented Apr 29, 2026

/retest

@zaneb zaneb force-pushed the hw-requirements-json branch from edb10c2 to 441a0ed Compare April 30, 2026 04:12
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 30, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rwsu for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zaneb zaneb force-pushed the hw-requirements-json branch 2 times, most recently from 7f83aeb to 7674422 Compare May 4, 2026 00:22
Store the hardware requirements in a separate JSON file, and process it
using jq.

Log the final value to stderr in human-readable form before writing the
shell-encoded data to the environment file.
@zaneb zaneb force-pushed the hw-requirements-json branch from 7674422 to cc57758 Compare May 4, 2026 04:26
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 4, 2026

@zaneb: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-compact-ipv4-appliance-diskimage cc57758 link false /test e2e-agent-compact-ipv4-appliance-diskimage
ci/prow/e2e-agent-two-node-fencing-ipv4 cc57758 link false /test e2e-agent-two-node-fencing-ipv4

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@zaneb
Copy link
Copy Markdown
Member Author

zaneb commented May 4, 2026

/verified by @zaneb

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label May 4, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@zaneb: This PR has been marked as verified by @zaneb.

Details

In response to this:

/verified by @zaneb

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants