Skip to content

Conversation

@yasun1
Copy link
Contributor

@yasun1 yasun1 commented Jan 23, 2026

Summary by CodeRabbit

  • New Features

    • Adapter requirements are now configurable via environment variables HYPERFLEET_CLUSTER_ADAPTERS and HYPERFLEET_NODEPOOL_ADAPTERS, with sensible defaults.
  • Documentation

    • Added an "Adapter Requirements" section with examples and Helm values/templates.
  • Refactor

    • Core services now consume the configurable adapter requirements and mapping logic replaced hard-coded lists.
  • Tests

    • Added unit tests covering env-driven adapter loading and fallback behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Walkthrough

The PR introduces a new AdapterRequirementsConfig that reads HYPERFLEET_CLUSTER_ADAPTERS and HYPERFLEET_NODEPOOL_ADAPTERS from the environment (JSON arrays) with defaults. Services (ClusterService, NodePoolService) are updated to accept and store this config instead of using hard-coded adapter lists. Status aggregation logic references the configuration, and docs/Helm values were updated with examples and commented env templates. Tests for the config parsing were added.

Sequence Diagram

sequenceDiagram
    participant Startup as Application Startup
    participant Plugin as Plugin (clusters/nodePools)
    participant Config as AdapterRequirementsConfig
    participant Env as Environment Variables
    participant Service as ClusterService/NodePoolService
    participant StatusLogic as Status Aggregation

    Startup->>Plugin: Initialize
    Plugin->>Config: NewAdapterRequirementsConfig()
    Config->>Env: LoadFromEnv()
    Env-->>Config: HYPERFLEET_CLUSTER_ADAPTERS / HYPERFLEET_NODEPOOL_ADAPTERS
    Config-->>Plugin: return adapter config
    Plugin->>Service: NewClusterService(..., adapterConfig) / NewNodePoolService(..., adapterConfig)
    Service->>Service: store adapterConfig
    Service->>StatusLogic: UpdateStatusFromAdapters()
    StatusLogic->>Service: read adapterConfig.RequiredClusterAdapters / RequiredNodePoolAdapters
    StatusLogic->>StatusLogic: Compute phase using configured adapters
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

lgtm, approved

Suggested reviewers

  • aredenba-rh
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding configurable adapter requirements via a new configuration system, which is reflected across all modified files.

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

✨ Finishing touches
  • 📝 Generate docstrings

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


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

@rh-amarin
Copy link
Contributor

/lgtm as a temporary fix
We will have to revisit how configuration is managed in hyperfleet-api when we approve the new standard

@rh-amarin
Copy link
Contributor

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Jan 23, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rh-amarin

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

The pull request process is described 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

@openshift-merge-bot openshift-merge-bot bot merged commit d2d13e4 into openshift-hyperfleet:main Jan 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants