* agent/installconfig: Add two-node-with-fencing topology and refactor
two-node validation
* feat: add override for control plane fencing creds
Signed-off-by: ehila <ehila@redhat.com>
* Add TNF fencing credentials override test
* Update integration test with new validation result
* Update installer verification and tests to only allow URLs with redfish on them for Two Nodes with Fencing topology
* Update validation check for redfish
* Remove simultaneous dual replica feature set restriction
* Update fencing address validation to include port
* Update validation to disallow http
* Update and expand url validation tests
* Revert "Update validation to disallow http"
This reverts commit e9595a8d4f.
* Update variable name
* Update tests
* Add YAML tags to Credential struct for fencing
Add explicit yaml struct tags to the Credential type to ensure proper
YAML serialization with lowercase field names (e.g., 'hostname' instead
of 'hostName'). This is required for the assisted-service to correctly
parse the fencing credentials file.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Add fencing credentials file generation for TNF clusters
Generate /etc/assisted/hostconfig/fencing-credentials.yaml containing
all fencing credentials from controlPlane.fencing.credentials[]. This
file is embedded in the agent ISO and consumed by assisted-service
during TNF cluster installation.
Key changes:
- Add OptionalInstallConfig to Ignition Dependencies()
- Add addFencingCredentials() function to generate the YAML file
- Call addFencingCredentials() in Generate() after NTP sources
- Add comprehensive unit tests for the new function
The single-file approach avoids directory naming collisions between
MAC-based host directories and hostname-based fencing credentials.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Revert fencing credentials override
The fencing credentials are now passed to assisted-service via the
hostconfig/fencing-credentials.yaml file embedded in the ISO, making
the install-config annotation override unnecessary.
This reverts commits:
- 105b3c95c9 Add TNF fencing credentials override test
- a06d1a766b feat: add override for control plane fencing creds
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Improve fencing credentials test coverage
Enhance TestIgnition_addFencingCredentials with:
- File owner verification (assert root ownership)
- Append behavior test with pre-existing files
- Fix misleading test name and add second credential to match
valid TNF configuration (2 credentials required)
- Remove unused expectError field from test struct
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Support vendor-specific redfish schemes in fencing validation
Vendor-specific redfish schemes like idrac-redfish:// and ilo5-redfish://
use HTTPS (port 443) by default, so they should be valid without an
explicit port number.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* unit tests: Add missing OptionalInstallConfig dependency in ignition test
The TestIgnition_Generate test was panicking because the
OptionalInstallConfig asset was missing from the test dependencies.
This caused dependencies.Get() to return a nil value when the
addFencingCredentials function tried to access it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* agent: Refactor fencing credentials into standalone asset
Move fencing credentials generation from inline ignition.go code into a
proper FencingCredentials asset following the installer's asset pattern.
This refactor:
- Creates pkg/asset/agent/manifests/fencingcredentials.go as a
WritableAsset with Dependencies, Generate, Files, and Load methods
- Adds comprehensive unit tests in fencingcredentials_test.go
- Integrates FencingCredentials into AgentManifests dependency graph
- Removes addFencingCredentials() from ignition.go
- Adds positive integration test for TNF with fencing credentials
- Changes output path from /etc/assisted/hostconfig/ to
/etc/assisted/manifests/ (standard manifests location)
The asset automatically returns empty Files() for non-TNF clusters,
so no fencing-credentials.yaml is generated unless fencing is configured.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Improve fencing credentials code quality
- Add explicit YAML library aliasing for clarity (goyaml for marshal,
k8syaml for unmarshal) with documentation explaining why different
libraries are used for each operation
- Improve error message to include credential count for debugging
- Add test case for empty fencing credentials array
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Fix CI failures for gofmt and integration tests
- Add blank line between k8s.io and github.com/openshift import groups
in ignition_test.go to satisfy gci formatting requirements
- Add featureSet: TechPreviewNoUpgrade to tnf_with_fencing_credentials
integration test to enable the DualReplica feature gate required for
TNF fencing configuration
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Move fencing credentials to agentconfig package
FencingCredentials is a host-scoped configuration asset, not a
cluster-scoped manifest. Moving it from manifests/ to agentconfig/
aligns with the package's purpose and follows the pattern used by
other host configuration assets like AgentHosts.
This change also updates ignition.go to import from the new location
and removes the now-unused fencing credentials from agent.go manifests.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Add FencingCredentials to ignition test dependencies
The TestIgnition_Generate test was failing with a panic because the
FencingCredentials asset was added as a dependency to Ignition.Generate()
but wasn't included in the test's buildIgnitionAssetDefaultDependencies()
helper function.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Fix fencing credentials path in integration test
The integration test expected the fencing credentials file at
/etc/assisted/manifests/ but assisted-service reads it from
/etc/assisted/hostconfig/ (HOST_CONFIG_DIR default). The installer
correctly embeds the file at hostconfig/, so the test expectation
was wrong.
Changed test path from manifests to hostconfig to match both the
installer implementation and assisted-service expectations.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Add nolint directive for gosec G101 false positive
The gosec linter flags fencingCredentialsFilename as "potential
hardcoded credentials" (G101) because the variable name contains
"credentials". This is a false positive - the variable contains
a filename string, not actual credentials.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* agent: Fix expected YAML field order in TNF integration test
The expected fencing-credentials.yaml had fields in a different order
than the actual YAML serialization output. Updated the expected file
to match the actual field order: hostname, username, password, address,
certificateVerification.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---------
Signed-off-by: ehila <ehila@redhat.com>
Co-authored-by: ehila <ehila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
* azure: validate cluster name against Azure reserved words
Azure prohibits the use of certain reserved words and trademarks
in resource names. This change adds validation to reject cluster
names containing any of the 43 reserved words documented by Azure,
preventing deployment failures with ReservedResourceName errors.
Reserved words checked include:
- Complete reserved words (40): AZURE, OFFICE, EXCHANGE, etc.
- Substring forbidden (2): MICROSOFT, WINDOWS
- Prefix forbidden (1): LOGIN
* update the checking logic on reserved words
* fix the gofmt issues
** Remove the validation when the `endpoint` field is filled out in install-config.
The feature has been tested and the feature gate should no longer be required.
** Remove the test for the featuregate.
Wire in the internalDNSRecords API field to install-config so we can
disable the internal DNS records for deployments using a
user-managed loadbalancer.
Co-Authored-By: Claude <noreply@anthropic.com>
PowerVC is an OpenStack based cloud provider with some significant
differences. Since we can use the OpenStack provider for most of the
work, we will create a thin provider which will only handle the
differences.
Changes in the apimachinery and client-go packages broke our unit
tests after upgrade. client-go stopped serializing the empty
preferences: {} field for the kubeconfig. apimachinery, with
c58e197ee8
caused much more extensive breakage by changing the format with
which errors are returned, and our unit tests hard code the
expected error messages.
For the most part, I used claude to fix these issues, and here
is the summary it created:
Changes Made
Root Cause: The k8s apimachinery package (errors.go:93-96) now uses json.Marshal()
to format BadValue in error messages instead of Go's native format. Error Format Changes:
Arrays: []string{"a", "b"} → ["a","b"] (JSON format, no spaces)
Nil values: []string(nil) → null
Structs: aws.Subnet{ID:"x", Roles:...} → {"id":"x"} (JSON with lowercase keys)
Custom types: gcp.OSImage{Name:"x", Project:"y"} → {"name":"x","project":"y"}
1. Add an install-config option to specify `sourcePolicy` that
captures user's intention regarding the fallback policy when there
is an error getting images from the mirror
2. Update the ImageDigestcwMirrorSet manifest with this policy. API
changes were not required because this value was already present in
the API.
3. Update logic to translate from deprecated `imageContentSources`
to `imageDigestSources` with the new `sourcePolicy` in place.
4. Updated install-config validation and tests for this new field.
Added platform-agnostic multi-disk support using Ignition configuration embedded in MachineConfigs
Created new disk types: etcd, swap, and user-defined disks
Implemented disk setup validation and feature gates
Added machine config generation for disk provisioning
Review and unit tests were assisted-by: cursor
In Two Node OpenShift with Fencing (TNF), we need to render secrets to the
openshift-etcd namespace to later be consumed by the openshift-etcd-operator to
initialize pacemaker. This PR consumes the secrets from the install-config and
attempts to render a new secret file to the openshift-etcd namespace. I've also
included a few updates to the tests for the fencing.credentials block for
completeness.
Depends on https://github.com/openshift/installer/pull/9640
• This commit enhances VIPs validation to ensure the primary IP
family of the VIPs matches the primary IP family of all the network fields.
• Code repurposed from prior closed PR (#7504)
Co-Authored-By: Maysa Macedo <mdemaced@redhat.com>
Co-Authored-By: Danny Kokkinos <dkokkino@redhat.com>
Two Node OpenShift (TNF) is DevPreview in 4.19. In order to ensure that ironic doesn't try to manage
the power state of the nodes, we add a check for the DualReplica topology after the control-plane nodes
are provisioned during bootstrapping and detach them from ironic.
In a future release, when fencing is enabled, it will be important to enforce that this remains an invariant
for the DualReplica control-plane topology. There is currently nothing preventing the annotation that detaches
these nodes from being removed.
* pkg/types/azure: remove SystemAssigned ID
SystemAssigned Identities are not supported in any capacity in MAPZ.
Due to that they were feature gated for future CAPZ->MAPZ transition.
The CAPZ Identity API creates further issues in that, the value to be
used for name/scope is unclear and when deleting clusters the
role assignment of the identity is leaked.
No users have asked for this functionality, so lets revert it to
reduce our complexity and load.
* fixup! pkg/types/azure: remove SystemAssigned ID
CAPZ supports system-assigned identities and multiple user-assigned
identities, but MAPI supports neither. MAPI manages control-plane
nodes day-2 via the control-plane machineset operator. So we cannot
currently support these features even for control-plane nodes as
the identities would be unrecoverable if the nodes are recreated
day 2.
As MAPI is in process of migrating to CAPI, this commit puts these
configurations behind a feature gate so that they may be unlocked
in the MAPI->CAPI migration.
- refactor some method location for clearer code
- use feature gate in a better structured way to align with standards
- Add compute validation
- tests for added changes
- use log in a old legacy method
- better API description
Signed-off-by: Michael Shitrit <mshitrit@redhat.com>
- Fencing credentials refactored to a Fencing parent holding a list of credentials
- Fencing credentials moved under ControlPlane
- Validations and tests updated
Signed-off-by: Michael Shitrit <mshitrit@redhat.com>
