Skip to content

Conversation

@fonta-rh
Copy link

No description provided.

…sh on them for Two Nodes with Fencing topology
This function can now be used for both:
- controlPlane.fencing.credentials validation (install-config)
- platform.baremetal.hosts[].bmc validation (agent hostconfig generation)
- Add generateFencingCredentialsYAML() helper to convert BMC to YAML
- Extend HostConfigFiles() to generate fencing-credentials.yaml per host
- Convert BMC.DisableCertificateVerification (bool) to certificateVerification (string)
- Only generate file if BMC address is configured
Tests cover:
- generateFencingCredentialsYAML() with various BMC configurations
- Certificate verification enabled/disabled conversion
- HostConfigFiles() with BMC configured hosts
- HostConfigFiles() without BMC (worker nodes)
- Multiple hosts with mixed BMC configurations
The validation now correctly rejects IPMI addresses and only accepts
RedFish-compatible BMC addresses. Updated c1(), c2(), c3() test
helpers to use redfish+https:// scheme instead of ipmi://
Validates that baremetal host BMC addresses are RedFish-compatible
for Two-Node Fencing (TNF) clusters. This ensures both sources of
fencing credentials are validated:

1. controlPlane.fencing.credentials[] - for cluster-etcd-operator
2. platform.baremetal.hosts[].bmc - for host provisioning and ABI

The validation only applies to TNF clusters (2 control plane replicas
with fencing enabled) and only validates master/control plane nodes.

Added comprehensive tests covering:
- Valid RedFish BMC addresses
- Invalid IPMI addresses (should fail)
- Non-TNF clusters (should not validate)
- Worker nodes (can use IPMI)
…remetal.hosts[].bmc

This PR focuses exclusively on validating baremetal.hosts[].bmc addresses
for TNF (Two-Node Fencing) clusters. Validation of controlPlane.fencing.credentials
will be handled in a separate PR.

Changes:
- Remove validateRedfishURL() and validateFencingCredentialAddress() from installconfig.go
- Remove call to validateFencingCredentialAddress() in validateFencingCredentials()
- Keep all baremetal BMC validation in pkg/types/baremetal/validation/platform.go

The baremetal validation package has its own validateBMCAddressForFencing()
implementation to avoid import cycles.
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 11, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 11, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Move RedFish BMC address validation to pkg/types/common to be shared between:
- controlPlane.fencing.credentials validation (future use)
- platform.baremetal.hosts[].bmc validation (for TNF clusters)

This avoids import cycles and provides a single source of truth for RedFish
BMC address validation, including:
- URL format validation
- RedFish protocol requirement (IPMI not supported)
- Port validation (explicit or inferred from https/http scheme)

Updated baremetal validation to use the shared function instead of a local
implementation, ensuring consistent validation across the codebase.
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 26, 2025

[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 rvanderp3 for approval. For more information see the Code Review Process.

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

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

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

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant