Skip to content

Conversation

t-cas
Copy link

@t-cas t-cas commented Aug 1, 2025

Today, node-resolver daemonset in openshift-dns-operator namespace is referencing a SERVICES environment variable allowing to update /etc/hosts of all nodes of the cluster with entries for custom services of the platform, so it can be targeted directly from nodes. This allow the nodes to have access to internal openshift registry directly using DNS entry image-registry.openshift-image-registry.svc
Only issue is that this value for SERVICES env variable is harcoded in cluster-dns-operator code, so it does not allow to reference other services available also at node level.

This PR update DNS operator.openshift.io custom resource and associated controller to support a list of openshift services so they will be added automatically in /etc/hosts and available on all cluster nodes.

This is tracked by RFE-4145

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 1, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 1, 2025

@t-cas: This pull request references RFE-4145 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 feature request to target the "4.20.0" version, but no target version was set.

In response to this:

Today, node-resolver daemonset in openshift-dns-operator namespace is referencing a SERVICES environment variable allowing to update /etc/hosts of all nodes of the cluster with entries for custom services of the platform, so it can be targeted directly from nodes. This allow the nodes to have access to internal openshift registry directly using DNS entry image-registry.openshift-image-registry.svc
Only issue is that this value for SERVICES env variable is harcoded in cluster-dns-operator code, so it does not allow to reference other services available also at node level.

This PR update DNS operator.openshift.io custom resource and associated controller to support a list of openshift services so they will be added automatically in /etc/hosts and available on all cluster nodes.

This is tracked by RFE-4145

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 openshift-ci bot requested review from knobunc and Miciah August 1, 2025 12:44
Copy link
Contributor

openshift-ci bot commented Aug 1, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign frobware for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 1, 2025
Copy link
Contributor

openshift-ci bot commented Aug 1, 2025

Hi @t-cas. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@candita
Copy link
Contributor

candita commented Aug 1, 2025

@t-cas thanks for your contribution. There are two issues here. First, the RFE has not been accepted, and second, you will need to make OpenShift API changes in github.com/openshft/api and vendor them instead of directl editing vendor/github.com/openshift/api/operator/v1/types_dns.go

/hold

@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 Aug 1, 2025
@t-cas
Copy link
Author

t-cas commented Aug 4, 2025

@candita thanks for your feedback, api change done in this PR openshift/api#2435 as requested

The change is rather small, and it is here already implemented and tested, how can we move forward with RFE ?


// Add any additional services from the spec
if len(dns.Spec.NodeServices) > 0 {
for _, service := range dns.Spec.NodeServices {
Copy link
Member

Choose a reason for hiding this comment

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

@candita Will this ensure that the list, assuming items aren't changing, are in a stable order or do we need to sort them to prevent churn?

@sdodson
Copy link
Member

sdodson commented Aug 5, 2025

/test verify-deps

@sdodson
Copy link
Member

sdodson commented Aug 5, 2025

/test verify

Copy link
Contributor

openshift-ci bot commented Aug 5, 2025

@t-cas: 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/verify-deps 4bc22ff link true /test verify-deps
ci/prow/verify 4bc22ff link true /test verify

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.

@candita
Copy link
Contributor

candita commented Aug 7, 2025

@t-cas @sdodson just an update - decision needs to wait until our team lead is back from PTO next week. However, just to set expectations, we do have higher priority items for review.

@candita
Copy link
Contributor

candita commented Aug 13, 2025

/assign @Miciah
/assign @alebedev87

@candita
Copy link
Contributor

candita commented Aug 20, 2025

@t-cas please investigate the failing verify/verify-deps test before review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants