Skip to content

OPNET-678: Allow disabling internal DNS#9924

Merged
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
cybertron:disable-internal-dns
Dec 3, 2025
Merged

OPNET-678: Allow disabling internal DNS#9924
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
cybertron:disable-internal-dns

Conversation

@cybertron
Copy link
Copy Markdown
Member

No description provided.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Sep 3, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 3, 2025
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2025
@cybertron cybertron force-pushed the disable-internal-dns branch from 322d1f0 to ec64c52 Compare November 17, 2025 22:45
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2025
@cybertron
Copy link
Copy Markdown
Member Author

This includes #10097 but I split the api bump out because I think it should be easier to review in isolation.

@cybertron cybertron changed the title Allow disabling internal DNS OPNET-678: Allow disabling internal DNS Nov 17, 2025
@cybertron cybertron marked this pull request as ready for review November 17, 2025 22:48
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Nov 17, 2025

@cybertron: This pull request references OPNET-678 which is a valid jira issue.

Details

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 17, 2025
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2025
@openshift-ci openshift-ci Bot requested review from andfasano and honza November 17, 2025 22:49
@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 18, 2025

/retest

@cybertron cybertron force-pushed the disable-internal-dns branch 3 times, most recently from 9d31157 to dc21cb3 Compare November 18, 2025 22:33
Copy link
Copy Markdown
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 🚀

I have a couple small questions only :D

Also, I think we need to drop the changes for openshift/api vendor since it's already on main? Maybe, a quick rebase on top of main will naturally solve it...

{
FeatureGateName: features.FeatureGateOnPremDNSRecords,
Condition: v.DNSRecordsType == configv1.DNSRecordsTypeExternal,
Field: field.NewPath("platform", "vsphere", "DNSRecordsType"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Field: field.NewPath("platform", "vsphere", "DNSRecordsType"),
Field: field.NewPath("platform", "vsphere", "dnsRecordsType"),

nit: we should use the serialized name of the field for field path, right? Same questions for openstack, baremetal, and nutanix too 💭

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like that's consistent with the existing checks. I'll change it.

APIServerInternalIPs: installConfig.Config.Platform.BareMetal.APIVIPs,
IngressIPs: installConfig.Config.Platform.BareMetal.IngressVIPs,
LoadBalancer: installConfig.Config.Platform.BareMetal.LoadBalancer,
DNSRecordsType: installConfig.Config.Platform.BareMetal.DNSRecordsType,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Do we need to set this field in the infrastructure status for platform vsphere, openstack, and nutanix too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes. Fixed.

cybertron and others added 3 commits November 19, 2025 11:10
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>
Also makes the featuregate validations consistent with the existing
checks.
@cybertron cybertron force-pushed the disable-internal-dns branch from dc21cb3 to 534ea54 Compare November 19, 2025 18:01
@cybertron
Copy link
Copy Markdown
Member Author

Also fixed the integration test failure.

@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 19, 2025

/test verify-vendor

Copy link
Copy Markdown
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 19, 2025
@mkowalski
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2025
@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 24, 2025

/hold cancel
/retest

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2025
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 90b08c7 and 2 for PR HEAD 534ea54 in total

@cybertron
Copy link
Copy Markdown
Member Author

/retest-required

The openstack job seems kind of unstable lately, but it did pass on an earlier version of this patch so I don't think it's a legitimate failure.

@cybertron
Copy link
Copy Markdown
Member Author

/retest-required

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 54f4b77 and 1 for PR HEAD 534ea54 in total

@cybertron
Copy link
Copy Markdown
Member Author

/retest-required

The openstack job failed on different tests this time. Progress, I guess?

@mkowalski
Copy link
Copy Markdown
Contributor

/test e2e-openstack-ovn

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD b4fa331 and 0 for PR HEAD 534ea54 in total

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/hold

Revision 534ea54 was retested 3 times: holding

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 26, 2025
@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 26, 2025

/test e2e-openstack-ovn

again?

@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 26, 2025

Hmm, looks like the install couldn't complete... I am not sure what's going on here 😞

@cybertron
Copy link
Copy Markdown
Member Author

I've reached out to the openstack team to see if they have any ideas. This has passed multiple other on-prem jobs and I can see from some of the job logs that coredns was working correctly in the clusters, so I don't think this patch is causing the failure.

@tthvo
Copy link
Copy Markdown
Member

tthvo commented Nov 26, 2025

Thanks Ben, I'll leave it to you to remove the hold then :D Hopefully, it is resolved 🤞

@mkowalski
Copy link
Copy Markdown
Contributor

/hold cancel

Whenever openstack is done with their fix

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2025
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD b4fa331 and 2 for PR HEAD 534ea54 in total

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 7d0584e and 1 for PR HEAD 534ea54 in total

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD fd5a518 and 0 for PR HEAD 534ea54 in total

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 2, 2025

@cybertron: 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/e2e-aws-ovn-fips dc21cb3 link false /test e2e-aws-ovn-fips
ci/prow/e2e-aws-ovn-single-node dc21cb3 link false /test e2e-aws-ovn-single-node
ci/prow/e2e-aws-ovn-heterogeneous dc21cb3 link false /test e2e-aws-ovn-heterogeneous
ci/prow/e2e-aws-ovn-imdsv2 dc21cb3 link false /test e2e-aws-ovn-imdsv2
ci/prow/e2e-metal-assisted 534ea54 link false /test e2e-metal-assisted
ci/prow/e2e-nutanix-ovn 534ea54 link false /test e2e-nutanix-ovn
ci/prow/e2e-openstack-proxy 534ea54 link false /test e2e-openstack-proxy
ci/prow/e2e-agent-two-node-fencing-ipv4 534ea54 link false /test e2e-agent-two-node-fencing-ipv4
ci/prow/okd-scos-e2e-vsphere-ovn 534ea54 link false /test okd-scos-e2e-vsphere-ovn
ci/prow/e2e-metal-ovn-two-node-fencing 534ea54 link false /test e2e-metal-ovn-two-node-fencing

Full PR test history. Your PR dashboard.

Details

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.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/hold

Revision 534ea54 was retested 3 times: holding

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2025
@patrickdillon
Copy link
Copy Markdown
Contributor

/hold cancel
/skip

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2025
@patrickdillon
Copy link
Copy Markdown
Contributor

/override ci/prow/e2e-openstack-ovn

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 2, 2025

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-openstack-ovn

Details

In response to this:

/override ci/prow/e2e-openstack-ovn

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.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 75bbd5d and 2 for PR HEAD 534ea54 in total

@openshift-merge-bot openshift-merge-bot Bot merged commit 3b6ba6b into openshift:main Dec 3, 2025
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria verified-later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants