Skip to content

Add base domain to DNS config #146

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 2 commits into from
Dec 17, 2018
Merged

Add base domain to DNS config #146

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bf7332
Add base domain to DNS config
ironcladlou Dec 11, 2018
e3f9640
Improve the BaseDomain docs
ironcladlou Dec 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config/v1/types_dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ type DNS struct {
}

type DNSSpec struct {
// baseDomain is the base domain of the cluster. All managed DNS records will
// be sub-domains of this base.
//
// For example, given the base domain `openshift.example.com`, an API server
// DNS record may be created for `cluster-api.openshift.example.com`.
BaseDomain string `json:"baseDomain"`
Copy link
Contributor

@abhinavdahiya abhinavdahiya Dec 11, 2018

Choose a reason for hiding this comment

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

This might be useful in Infra ?

Copy link
Contributor

@knobunc knobunc Dec 12, 2018

Choose a reason for hiding this comment

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

Yeah, this is probably a higher level config setting. Both DNS and the IngressOperator need it. I am not sure if anyone else wants to know, so I don't know if keeping it in DNS is sufficient for now.

Copy link
Contributor

@Miciah Miciah Dec 12, 2018

Choose a reason for hiding this comment

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

Is there supposed to be any correspondence between config object and operator? Logically DNS-related configuration belongs here: "DNS holds cluster-wide information about DNS." That doesn't specifically mean config for the DNS operator.

Copy link
Contributor Author

@ironcladlou ironcladlou Dec 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@Miciah shares my interpretation of this type. If the base domain (the only required DNS input to the installer?) doesn't belong here I'm not sure what does.

Copy link
Contributor

@deads2k deads2k Dec 17, 2018

Choose a reason for hiding this comment

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

Yeah, this is probably a higher level config setting. Both DNS and the IngressOperator need it. I am not sure if anyone else wants to know, so I don't know if keeping it in DNS is sufficient for now.

As a cluster-admin, if I want to change my DNS suffix, where would I expect to do it? The types here are not mapped to operators, but instead to features. The feature here being DNS.

The ingress operator is perfectably able to consume the information from the DNS config type if it needs it as an input.

}

type DNSStatus struct {
Expand Down