[release-4.22] OCPBUGS-99406: Custom dns configuration backport#7085
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pacevedom: This pull request references USHIFT-6968 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
openshift-ci
Bot
added
the
approved
|
/retest |
pacevedom
changed the title
openshift-ci-robot
added
the
jira/invalid-bug
|
@pacevedom: This pull request references Jira Issue OCPBUGS-99406, 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. |
|
/jira refresh |
|
@pacevedom: This pull request references Jira Issue OCPBUGS-99406, which is invalid:
Comment 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. |
|
/jira refresh |
|
@pacevedom: This pull request references Jira Issue OCPBUGS-99406, which is invalid:
Comment 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. |
|
/lgtm |
pacevedom
removed
the
jira/invalid-bug
|
/label jira/valid-bug |
openshift-ci
Bot
added
jira/valid-bug
openshift-ci-robot
added
the
verified
|
@pacevedom: This PR has been marked as verified by 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. |
Add a new ConfigFile field to the DNS config struct that allows users to specify a custom CoreDNS Corefile path. Validation ensures mutual exclusivity with dns.hosts, and checks that the file is absolute, exists, is readable, non-empty, and within the 1MiB ConfigMap limit.
When dns.configFile is set, read the file and apply its contents directly as the Corefile key in the dns-default ConfigMap, bypassing the default template rendering. When unset, existing behavior is preserved.
Add DNSConfigurationWatcherManager that watches the custom Corefile specified by dns.configFile for changes at runtime, updating the dns-default ConfigMap when the file is modified. Follows the same fsnotify + SHA256 hashing pattern as the existing hosts watcher.
Cover mutual exclusivity with dns.hosts, non-absolute path, missing file, empty file, file exceeding 1MiB limit, valid config, and default behavior when configFile is unset.
RobotFramework tests covering: - Custom Corefile is used by CoreDNS instead of the default - Runtime reload propagates changes without MicroShift restart - Cluster-local resolution works with custom Corefile
- Use apierrors.IsNotFound for ConfigMap Get errors instead of treating any error as not-found - Merge annotations into existing ConfigMap instead of replacing them to avoid clobbering annotations set by the asset apply - Handle atomic-rename file updates (Rename/Remove events) and re-add the file watch on Create so the watcher survives vim/sed -i edits - Reject non-regular files (directories, FIFOs) in validateConfigFile - Fix Should Contain assertion in robot test that treated the hostname as a failure message instead of a second assertion - Add retry wrapper to ConfigMap verification after restart
Deduplicate keywords shared between dns.robot and dns-custom-config.robot by moving them into a common resource file. Also fixes a bug in dns.robot where Router Should Resolve Hostname passed the hostname as a failure message to Should Contain instead of asserting it separately.
…test Add a ConfigMap content check after updating the Corefile at runtime, before attempting DNS resolution. This confirms the file watcher propagated the change and provides a faster, clearer failure signal.
- Consolidate duplicate file validation logic in dns.go into a shared validateFilePath helper used by both validateConfigFile and validateHosts. Also fixes validateHosts calling os.Stat twice and missing regular-file and empty-file checks. - Depend on infrastructure-services-manager instead of kube-apiserver directly, ensuring the initial DNS ConfigMap exists before the watcher starts. - Avoid double-closing the fsnotify watcher by returning ctx.Err() on context cancellation and letting the defer handle Close(). - Return lastHash instead of currentHash on updateConfigMap failure so the next identical change is not skipped.
…raction Extract shared file-watching logic from dnsconfigurationwatcher.go and hostswatcher.go into a common fileWatcher struct in filewatcher.go. Both concrete watchers become thin wrappers that embed fileWatcher and only provide constructors with their specific configuration. Also fixes bugs in the hosts watcher: double-closing the fsnotify watcher, treating any Get error as not-found, and returning currentHash on update failure.
Add a dedicated log message when the watched file is removed, and a test that verifies the ConfigMap retains its last known content after the Corefile is deleted from disk. The test uses journal cursor + log pattern matching to confirm the watcher processed the removal event before asserting ConfigMap contents.
9684ffb to
c4c4024
Compare
openshift-ci-robot
removed
the
verified
openshift-ci
Bot
removed
the
needs-rebase
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom, pmtk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pacevedom: all tests passed! 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. |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@pacevedom: This PR has been marked as verified by 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. |
180981f
into
openshift:release-4.22
|
@pacevedom: Jira Issue OCPBUGS-99406: All pull requests linked via external trackers have merged: All linked pull requests have the 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. |
4 participants
No description provided.