Skip to content

extra-software: Add per-arch config support - #1137

Merged
kostyanf14 merged 1 commit into
HCK-CI:masterfrom
kostyanf14:VIRTWINKVM-2716
Sep 14, 2026
Merged

kostyanf14 merged 1 commit into
HCK-CI:masterfrom
kostyanf14:VIRTWINKVM-2716

Conversation

@kostyanf14

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread lib/auxiliary/extra_software/manager.rb Fixed
Comment thread lib/auxiliary/extra_software/manager.rb Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved moderate issues affect architecture selection, staging, downloading, and installation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds per-architecture extra-software configuration support across Ruby preparation and PowerShell installation.

Changes:

  • Supports architecture-specific config names and AMD64/x64 aliases.
  • Resolves, downloads, and installs software per target architecture.
  • Prepares configs for platform clients and studio installation.
File summaries
File Summary
lib/engines/hckinstall/setup-scripts/extra_software.ps1 Adds architecture-specific config lookup. Moderate finding (1 vote): candidate ordering can select a different config than Ruby.
lib/auxiliary/extra_software/manager.rb Adds architecture-aware preparation and installation. Moderate findings: fallback architectures may not be prepared (1 vote); rejected architectures may still be staged or installed (2 votes); unused platform architectures may trigger missing-config errors (1 vote); conflicting filenames can reuse the wrong download (1 vote).
Review details

Suppressed comments (5)

lib/auxiliary/extra_software/manager.rb:192

  • machine_arch resolves a client with no explicit arch to platform.client_arch or Project::DEFAULT_ARCH, but this loop only prepares configs for clients whose arch is non-nil. On a valid mixed platform (one client has an explicit architecture and another relies on the fallback), the fallback client's config is never prepared and installation is skipped after the warning in sw_config_for. Add the same fallback architecture here.
        arches << client.arch if client.arch

lib/auxiliary/extra_software/manager.rb:193

  • platform_arches now includes every declared client architecture, but Functest prepares extra software during engine initialization and boots only the union of client IDs selected by the chosen tests. On a mixed-architecture platform, a package needed by the selected x64 client but lacking an x86 config for an unused client will therefore raise ExtraSoftwareMissingConfig before the run starts. Limit this to architectures that the engine will actually prepare, or pass explicit architectures from the callers.
      arches << platform.client_arch if platform.client_arch
      platform.clients.each_value do |client|
        arches << client.arch if client.arch
      end

lib/auxiliary/extra_software/manager.rb:179

  • The alias ordering is not consistent between the Ruby preparation path and the PowerShell runtime path: for platform arch x64, Ruby checks x64 before amd64, while a Windows AMD64 process checks amd64 before x64. If both alias-specific files exist, the package is prepared from one config but the VM installs using the other. Canonicalize x64 to amd64 (or use the same precedence in both implementations).
      arch_name = arch.to_s.downcase
      variants = [arch_name]
      variants << 'x64' if arch_name == 'amd64'
      variants << 'amd64' if arch_name == 'x64'
      variants.uniq

lib/auxiliary/extra_software/manager.rb:95

  • When two architectures use configs with the same file_name but different download URLs, the first architecture downloads <extra_software>/<name>/<file_name> and the second call sees that path already exists and skips its download. The second architecture will then install the first architecture's binary; use architecture-specific download paths or reject conflicting file_name/URL pairs before downloading.
          download_software(name, config)

lib/engines/hckinstall/setup-scripts/extra_software.ps1:40

  • The PowerShell candidate order does not match config_path_candidates: this interleaves <kit>-<arch> and <arch> files, so if (for example) amd64-config.json and <kit>-x64-config.json both exist, the host loads the latter while the setup script loads the former. That can install a package using a different file_name or command than the one downloaded. Search all kit/architecture variants before the architecture-only variants, as the Ruby code does.
    foreach ($archVariant in (Get-ArchConfigVariants $arch)) {
        $config_list += "${Directory}\${full_kit}-${archVariant}-config.json"
        $config_list += "${Directory}\${archVariant}-config.json"
    }
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/auxiliary/extra_software/manager.rb
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
@kostyanf14
kostyanf14 merged commit 7c30b7e into HCK-CI:master Sep 14, 2026
11 checks passed
@kostyanf14
kostyanf14 deleted the VIRTWINKVM-2716 branch September 14, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants