OCPBUGS-88511: Disable apps DNS validation for add-nodes - #10737
OCPBUGS-88511: Disable apps DNS validation for add-nodes#10737PratikUplenchwar wants to merge 1 commit into
Conversation
Skip apps-domain-name-resolved-correctly for the add-nodes ISO by setting DISABLED_HOST_VALIDATIONS in assisted-service.env. Day-1 installs leave the value empty so the validation still runs. Assisted-by: Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci-robot
added
jira/severity-moderate
|
@PratikUplenchwar: This pull request references Jira Issue OCPBUGS-88511, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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 NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThe change adds workflow-specific disabled host validation configuration to ignition template data and renders it in the assisted-service environment. Add-nodes disables ChangesHost validation configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/asset/agent/image/ignition_test.go (1)
137-139: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the rendered environment contract.
These assertions test the selector only. Add a focused assertion through template rendering or
TestIgnition_Generatethat checks:
- add-nodes:
DISABLED_HOST_VALIDATIONS=apps-domain-name-resolved-correctly- install:
DISABLED_HOST_VALIDATIONS=This catches template-name and Generate-wiring regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asset/agent/image/ignition_test.go` around lines 137 - 139, Add focused rendered-output assertions in TestIgnition_Generate or a comparable template-rendering test for the DISABLED_HOST_VALIDATIONS environment variable: verify add-nodes renders apps-domain-name-resolved-correctly and install renders an empty value. Keep the existing disabledHostValidationsForWorkflow selector assertions, and exercise the actual template-name and Generate wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/asset/agent/image/ignition_test.go`:
- Around line 137-139: Add focused rendered-output assertions in
TestIgnition_Generate or a comparable template-rendering test for the
DISABLED_HOST_VALIDATIONS environment variable: verify add-nodes renders
apps-domain-name-resolved-correctly and install renders an empty value. Keep the
existing disabledHostValidationsForWorkflow selector assertions, and exercise
the actual template-name and Generate wiring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ab11fa59-a1d2-4ae2-b931-de196c3bfcbd
📒 Files selected for processing (3)
data/data/agent/files/usr/local/share/assisted-service/assisted-service.env.templatepkg/asset/agent/image/ignition.gopkg/asset/agent/image/ignition_test.go
|
@zaneb would you like to review this? |
|
This seems reasonable to me. I still can't think of a reason we would need to validate the Ingress on day 2. |
|
Scheduling required tests: Scheduling tests matching the |
|
/retest-required |
|
@PratikUplenchwar: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@zaneb: This pull request references Jira Issue OCPBUGS-88511, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
3 participants
Summary
oc adm node-image create, setDISABLED_HOST_VALIDATIONS=apps-domain-name-resolved-correctlyinassisted-service.envso assisted-service skips the apps ingress DNS check.Context
apps-domain-name-resolved-correctlyfails when the cluster ingress domain was changed post-install, because add-nodes validates against the install-timebaseDomainrather than the live ingress domain.Engineering direction is to disable this check for the add-nodes workflow instead of plumbing a live ingress domain.
Supersedes the closed approach in:
Test plan
oc adm node-image createand confirm host is not blocked onapps-domain-name-resolved-correctlyFixes: https://issues.redhat.com/browse/OCPBUGS-88511
Made with Cursor
Summary by CodeRabbit