[WIP] OCPBUGS-63472: Include --copy-network for all ABI installations
The --copy-network flag needs to be included so that network configuration changes applied through the agent-tui/nmtui persist after the node reboots.
List all the issues related to this PR
- [ ] New Feature
- [ ] Enhancement
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD
What environments does this code impact?
- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [ ] Operator Managed Deployments
- [x] None
How was this code tested?
- [ ] assisted-test-infra environment
- [x] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [ ] No tests needed
Checklist
- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see CONTRIBUTING guide)
- [x] This change does not require a documentation update (docstring,
docs, README, etc) - [ ] Does this change include unit-tests (note that code changes require unit-tests)
Reviewers Checklist
- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?
@rwsu: This pull request references Jira Issue OCPBUGS-63472, which is invalid:
- expected the bug to target the "4.21.0" version, but no target version was set
Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.
The bug has been updated to refer to the pull request using the external bug tracker.
In response to this:
The --copy-network flag needs to be included so that network configuration changes applied through the agent-tui/nmtui persist after the node reboots.
List all the issues related to this PR
- [ ] New Feature
- [ ] Enhancement
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD
What environments does this code impact?
- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [ ] Operator Managed Deployments
- [x] None
How was this code tested?
- [ ] assisted-test-infra environment
- [x] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [ ] No tests needed
Checklist
- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see CONTRIBUTING guide)
- [x] This change does not require a documentation update (docstring,
docs, README, etc)- [ ] Does this change include unit-tests (note that code changes require unit-tests)
Reviewers Checklist
- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
/jira refresh
@rwsu: This pull request references Jira Issue OCPBUGS-63472, which is valid. The bug has been moved to the POST state.
3 validation(s) were run on this bug
- bug is open, matching expected state (open)
- bug target version (4.21.0) matches configured target version for branch (4.21.0)
- bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
Requesting review from QA contact: /cc @mhanss
In response to this:
/jira refresh
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 43.87%. Comparing base (62373a0) to head (a5ab3bb).
:warning: Report is 10 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #8155 +/- ##
==========================================
+ Coverage 43.84% 43.87% +0.03%
==========================================
Files 414 414
Lines 71843 71893 +50
==========================================
+ Hits 31498 31546 +48
+ Misses 37483 37477 -6
- Partials 2862 2870 +8
| Files with missing lines | Coverage Δ | |
|---|---|---|
| cmd/agentbasedinstaller/register.go | 51.78% <100.00%> (+16.28%) |
:arrow_up: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Issues go stale after 90d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle stale
Walkthrough
The changes introduce conditional placeholder StaticNetworkConfig generation in InfraEnv creation when NMStateConfig is absent, ensuring ABI workflow support. Additionally, comprehensive test coverage validates behavior across scenarios with present/absent network configurations, cluster references, and error conditions.
Changes
| Cohort / File(s) | Summary |
|---|---|
InfraEnv Registration Logic cmd/agentbasedinstaller/register.go |
Adds staticNetworkConfig local variable and implements conditional placeholder injection. When NMStateConfig processing yields no entries, a minimal YAML placeholder with dummy interface is generated to enable --copy-network flow while preserving real config from NetworkManager keyfiles. StaticNetworkConfig is now unconditionally assigned to InfraenvCreateParams. |
InfraEnv Registration Tests cmd/agentbasedinstaller/register_test.go |
Introduces comprehensive test suite for RegisterInfraEnv with BeforeEach/AfterEach scaffolding, test manifests, and temporary directory management. Validates StaticNetworkConfig population with real configs, placeholder generation when NMStateConfig missing, cluster reference propagation, and error handling. Adds mockInfraEnvTransport mock type to capture and verify API parameters. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
[!TIP]
🧪 Unit Test Generation v2 is now available!
We have significantly improved our unit test generation capabilities.
To enable: Add this to your
.coderabbit.yamlconfiguration:reviews: finishing_touches: unit_tests: enabled: trueTry it out by using the
@coderabbitai generate unit testscommand on your code files or under ✨ Finishing Touches on the walkthrough!Have feedback? Share your thoughts on our Discord thread!
Comment @coderabbitai help to get the list of available commands and usage tips.
@rwsu: This pull request references Jira Issue OCPBUGS-63472, which is invalid:
- expected the bug to target either version "4.22." or "openshift-4.22.", but it targets "4.21.0" instead
Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.
In response to this:
Enable --copy-network flag when NetworkManager keyfiles are detected in /etc/NetworkManager/system-connections/. This ensures static network configuration persists after installation for both NMStateConfig and agent TUI configured networks.
When keyfiles exist but no NMStateConfig is provided, set a placeholder StaticNetworkConfig with a dummy0 interface (type: dummy, state: down). This passes validation and triggers --copy-network without interfering with the real network configuration from NetworkManager keyfiles.
Also refactored functions to accept fs.FS parameter for testability and fixed path compatibility issues (fs.FS requires relative paths).
List all the issues related to this PR
- [ ] New Feature
- [ ] Enhancement
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD
What environments does this code impact?
- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [ ] Operator Managed Deployments
- [x] None
How was this code tested?
- [ ] assisted-test-infra environment
- [x] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [x] Manual, created network config using agent-tui/nmtui and observed it persisted after node is installed
- [ ] No tests needed
Checklist
- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see CONTRIBUTING guide)
- [x] This change does not require a documentation update (docstring,
docs, README, etc)- [x] Does this change include unit-tests (note that code changes require unit-tests)
Reviewers Checklist
- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
/jira refresh
@rwsu: This pull request references Jira Issue OCPBUGS-63472, which is valid.
3 validation(s) were run on this bug
- bug is open, matching expected state (open)
- bug target version (4.22.0) matches configured target version for branch (4.22.0)
- bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Requesting review from QA contact: /cc @bmanzari
The bug has been updated to refer to the pull request using the external bug tracker.
In response to this:
/jira refresh
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: rwsu
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~cmd/agentbasedinstaller/OWNERS~~ [rwsu]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@rwsu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/edge-lint | a5ab3bbd2259583e1ea82ebb6250dd479a915942 | link | true | /test edge-lint |
| ci/prow/edge-unit-test | a5ab3bbd2259583e1ea82ebb6250dd479a915942 | link | true | /test edge-unit-test |
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.