-
Notifications
You must be signed in to change notification settings - Fork 447
CORS-4170: Extending in-cluster DNS support to Azure #5216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@sadasu: This pull request references CORS-4170 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.20.0" version, but no target version was set. In 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. |
@sadasu: This pull request references CORS-4170 which is a valid jira issue. In 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. |
@sadasu: This pull request references CORS-4170 which is a valid jira issue. In 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. |
b063c1c
to
deb21a6
Compare
@sadasu: This pull request references CORS-4170 which is a valid jira issue. In 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. |
/retest-required |
3 similar comments
/retest-required |
/retest-required |
/retest-required |
Unit test failure seems unrelated to the changes introduced by this PR. For example:
|
The the CoreDNS pod definition and Corefile for cloud platforms already exists due to prior work. Supported cloud platforms is extended to include Azure.
a10ec10
to
d01032b
Compare
/test unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logically lgtm based on enhancement. Are there tests/jobs to cover each platform we add?
pkg/controller/template/render.go
Outdated
} | ||
case configv1.AzurePlatformType: | ||
// If DNSType is set to `ClusterHosted`, we expect the Load Balancer IP addresses to be set. | ||
// If absent, that is exoected to be temporary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: exoected -> expected
Start CoreDNS pod for Azure DNSType is `ClusterHosted`. Building on work done for GCP and AWS. CoreDNS pod definition and CoreFile already added as part of support for GCP.
New `update-dns-server` script that adds DNS resolvers to /etc/NetworkManager/conf.d/dns-servers.conf. The script adds the host's own IP address and the cloud metadata server's IP address to the conf file. These would then get added to the local resolv.conf by NetworkManager. This script is run as part of azure-update-dns.service This service runs when the DNSType on the Azure platform is set to "ClusterHosted".
d01032b
to
4d7ad60
Compare
/retest-required |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cybertron, sadasu, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/tide refresh |
/label acknowledge-critical-fixes-only |
/verified by existing Installer pre-submit jobs. |
@sadasu: This PR has been marked as verified by In 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. |
/retest-required |
/hold |
/hold cancel |
5 similar comments
/test e2e-hypershift |
2 similar comments
@sadasu: The following tests failed, say
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. |
/retest-required |
cd9b709
into
openshift:main
- What I did
Based on enhancement : openshift/enhancements#1468
- How to verify it
Verification still in-progress.
- Description for the changelog
These changes allow MCO to read AzurePlatformStatus containing the API, API-Int and Ingress Load Balancer IPs for Azure platform. These would be used to generate the CoreFile for static CoreDNS pods running on the Bootstrap node and later on control plane nodes.
This feature is behind the featuregate
AzureClusterHostedDNSInstall
.