Skip to content
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

Create getDNSClusterDomain #580

Merged

Conversation

averdagu
Copy link
Contributor

@averdagu averdagu commented Nov 12, 2024

Openshift DNS creates it's records using the value in clusterDomain. This information could be customizable in the futre.

Currently on most of the operators they're using a hardcoded value, this function allows to all operators get this information from lib-common, and in case in the future this value is customizable we'd only need to change it here.

Related-Issue: OSPRH-3627

averdagu added a commit to averdagu/ovn-operator that referenced this pull request Nov 12, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
@mtomaska
Copy link

LGTM

@mtomaska mtomaska self-requested a review November 12, 2024 18:59
package clusterdns

// GetDNSClusterDomain - Return openshift dns Cluster Domain name
func GetDNSClusterDomain() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we keep returning an error ? I guess when there is the way to check if from the cluster we also have to change the parameters which get passed into the func

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I guess we should return error (in order to not need to modify all operators using this function) when we modify logic and try to get value from cluster.
And I don't think we ever need to use any parameter, as openshift dns operator only accepts one object in the cluster, so there's no need to parameterizice it. [0]

I just realize that I return "value, error" but is it possible that the convention we're using is "error, value"?

[0] https://github.com/openshift/cluster-dns-operator/blob/208d50c1a5e0aaeb991366daa749abdffa803224/pkg/operator/controller/controller.go#L180-L184

Copy link
Contributor

Choose a reason for hiding this comment

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

but how would you query the dns object from the cluster? we'd need to either pass in that object, or pass in a client to be able to get it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh true. So since if this code changes we'll need to update the operators you suggest to remove the error returned?

Copy link
Contributor

Choose a reason for hiding this comment

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

since we have to change it anyways, yes lets remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

averdagu added a commit to averdagu/ovn-operator that referenced this pull request Nov 13, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/ovn-operator that referenced this pull request Nov 13, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
Openshift DNS creates it's records using the value in clusterDomain.
This information could be customizable in the futre.

Currently on most of the operators they're using a hardcoded value,
this function allows to all operators get this information from
lib-common, and in case in the future this value is customizable
we'd only need to change it here.
@averdagu averdagu force-pushed the retrieve-core-dns-domain branch from 0abe832 to 81476aa Compare November 13, 2024 16:05
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 6dc9fd0 into openstack-k8s-operators:main Nov 14, 2024
2 checks passed
averdagu added a commit to averdagu/ovn-operator that referenced this pull request Nov 18, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/ovn-operator that referenced this pull request Nov 18, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 2, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 2, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 2, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
Resolves: OSPRH-3627
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 2, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/telemetry-operator that referenced this pull request Dec 2, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
Resolves: OSPRH-3627
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
Resolves: OSPRH-3627
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/telemetry-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
Resolves: OSPRH-3627
averdagu added a commit to averdagu/telemetry-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/telemetry-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/openstack-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Depends-on: openstack-k8s-operators/lib-common#580
Resolves: OSPRH-3627
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/infra-operator that referenced this pull request Dec 10, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
averdagu added a commit to averdagu/ovn-operator that referenced this pull request Dec 16, 2024
Openshift coreDNS creates the domain name using an string
located in dnses.operator.openshift.io. This string can
change in the future, calling lib-common/GetDNSClusterDomain
the responsability of gathering this information correctly
only falls under lib-common intead of all operators.

Resolves: OSPRH-3627
Depends-on: openstack-k8s-operators/lib-common#580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants