[payjain]

Summary

Add an optional agentConfiguration field to KlusterletSpec so the klusterlet agents' health probe port, metrics port and host networking can be configured. Today the health check port is hardcoded to 8443, so agents running with hostNetwork: true fail to start when that port is already in use on the node.

The structure mirrors BindConfiguration in ClusterManager's webhook configuration, as agreed in #439:

apiVersion: operator.open-cluster-management.io/v1
kind: Klusterlet
spec:
  agentConfiguration:
    bindConfiguration:
      healthProbePort: 9443   # default 8443, range 1024-65535
      metricsPort: 9090       # default 8080, <= 0 disables metrics
      hostNetwork: true

Notes:

• healthProbePort has a minimum of 1024 because the agents run as non-root.
• metricsPort has no omitempty, the same as ClusterManager, so 0 is kept and can disable metrics.
• The field is optional and existing Klusterlets are unaffected.

A follow-up PR in open-cluster-management-io/ocm will wire these fields into the agent deployments (ocm#1032).

Testing

• make verify: passes (0 lint issues)
• make test: passes
• make test-integration: 170/170 specs pass, including new AgentConfiguration specs for defaulting, custom values and rejecting a privileged port

Related issue(s)

Fixes #439
Ref: open-cluster-management-io/ocm#1032

Summary by CodeRabbit

• New Features

  • Added optional agent configuration for Klusterlets.
  • Added controls for host networking, health probe ports, and metrics ports.
  • Added configurable defaults and validation for supported port ranges.
  • Metrics can be disabled using a nonpositive port value.

• Tests

  • Added integration coverage for default settings, custom configurations, and invalid health probe ports.

[mikeshng]

/assign @rokej

/approve

This looks fine to me. Leaving the final tag to the operator maintainer: @rokej

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikeshng, payjain

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:

• OWNERS [mikeshng]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d85fb86b-3d32-4aae-a90e-750b1c49ad42

📥 Commits

Reviewing files that changed from the base of the PR and between d46e9f1 and cc5dd05.

📒 Files selected for processing: 5

• operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml
• operator/v1/types_klusterlet.go
• operator/v1/zz_generated.deepcopy.go
• operator/v1/zz_generated.model_name.go
• test/integration/api/klusterlet_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Walkthrough

The Klusterlet API adds optional agent configuration for health probe and metrics ports and host networking. The CRD schema and generated methods support the new configuration. Integration tests cover defaults, custom settings, and health probe port validation.

Changes

Agent configuration

Layer / File(s)	Summary
Add agent bind configuration operator/v1/types_klusterlet.go, operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml, operator/v1/zz_generated.deepcopy.go, operator/v1/zz_generated.model_name.go	KlusterletSpec gains optional AgentConfiguration and bind settings for health probe and metrics ports and host networking. The CRD schema defines port defaults and validation. Generated methods add deep-copy and model-name support.
Test bind configuration test/integration/api/klusterlet_test.go	Integration tests check default values, custom ports and host networking, and rejection of health probe port 80.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Feature · Severity of issue fixed: Medium

Suggested reviewers: mikeshng, suvaanshkumar

Merge Risk: ⚪ Minimal · up to cc5dd

The reviewed agent bind settings preserve the documented metrics-disable behavior, and no actionable merge-blocking issue remains after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the primary change, adding AgentConfiguration to KlusterletSpec. The ✨ prefix matches the repository template for a feature.
Description check		The description includes the required Summary and Related issue(s) sections. It explains the API changes, configuration behavior, testing results, and linked issues.
Linked Issues check		Issue #439 requires an optional AgentConfiguration in KlusterletSpec with a configurable agent health probe port and a BindConfiguration pattern. The reviewed API adds KlusterletSpec.AgentConfiguratio…
Out of Scope Changes check		The changes remain connected to #439. MetricsPort and HostNetwork are part of the same agent bind configuration and support the host-network port-conflict use case described by the PR and issue. The g…
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…

Full details: Linked Issues check

Explanation

Issue #439 requires an optional AgentConfiguration in KlusterletSpec with a configurable agent health probe port and a BindConfiguration pattern. The reviewed API adds KlusterletSpec.AgentConfiguration and AgentBindConfiguration.HealthProbePort. The CRD defines a default of 8443 and validates ports from 1024 through 65535. Integration tests cover the default, custom port 9443, and rejection of port 80. Generated deepcopy, model-name, and CRD artifacts are present. Agent deployment wiring is correctly deferred to ocm#1032, which is not a coding requirement of #439 for this API change.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to #439. MetricsPort and HostNetwork are part of the same agent bind configuration and support the host-network port-conflict use case described by the PR and issue. The generated artifacts and integration tests support the new API. No unrelated change is established. The referenced ocm#1032 deployment wiring is not included.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[rokej]

/lgtm