-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Bug 1715635: docs/user/aws/install_upi: Document bring-your-own-DNS #2221
Bug 1715635: docs/user/aws/install_upi: Document bring-your-own-DNS #2221
Conversation
2e64f89
to
ccfebc4
Compare
ccfebc4
to
a973089
Compare
a973089
to
d70f94b
Compare
@wking: This pull request references Bugzilla bug 1715635, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 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 kubernetes/test-infra repository. |
/retest |
Green enough. CC @abhinavdahiya for review. |
docs/user/aws/install_upi.md
Outdated
|
||
If you do so, you'll need to [add](#add-the-ingress-dns-records) the following records to your private and public zones: | ||
|
||
* `*.apps.{baseDomain}.`, which should point at the ingress load balancer. |
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.
I think we were also going to list the required subdomains if the user didn't want to put wildcard and mention that each new route will have to manually added later on.
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.
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.
I think we were also going to list the required subdomains...
Done with 65cbc79 -> 14e0691, although none of these are strictly required for cluster operation, are they? I'd expect in-cluster components to reach in-cluster services using in-cluster routing. You'd only need, for example, console and OAuth DNS records if you wanted to expose the console to users and administrators. Or am I misunderstanding?
d70f94b
to
65cbc79
Compare
Some users want to provide their own *.apps DNS records instead of delegating that to the ingress operator [1]. With this commit, we tell the ingress operator not to worry about managing any hosted zones, and walk users through how they can create the expected records [2] themselves. Removing the zones from the YAML manifest via sed or other POSIX command was too complicated, so I've given up on that and moved to Python and PyYAML [3]. There are many possible alternatives, but PyYAML seemed the most likely to be already installed, it's packaged for many systems if users want to install it, and the syntax is fairly readable if users want to accomplish the same task with a different tool of their choice. The Python examples are more readable as multi-line strings than if they were one-liners, and they can still be copy-pasted into a shell. Once faq [4] or similar becomes more common on user systems, we can replace this with: $ DATA="$(faq '.compute[0].replicas=0' install-config.yaml)" $ echo "${DATA}" >install-config.yaml and similar. For not, I'm not suggesting admins monitor for other DNSRecord objects [5] and fullful them as they show up. In case we do decide to have folks monitor them later, here's a sample: $ oc -n openshift-ingress-operator get -o yaml dnsrecord default-wildcard apiVersion: ingress.operator.openshift.io/v1 kind: DNSRecord metadata: creationTimestamp: "2019-08-22T20:45:00Z" finalizers: - operator.openshift.io/ingress-dns generation: 1 labels: ingresscontroller.operator.openshift.io/owning-ingresscontroller: default name: default-wildcard namespace: openshift-ingress-operator ownerReferences: - apiVersion: operator.openshift.io/v1 blockOwnerDeletion: true controller: true kind: IngressController name: default uid: b31db6db-c51d-11e9-8a7a-02ae97362ddc resourceVersion: "8847" selfLink: /apis/ingress.operator.openshift.io/v1/namespaces/openshift-ingress-operator/dnsrecords/default-wildcard uid: b59fbbfa-c51d-11e9-8a7a-02ae97362ddc spec: dnsName: '*.apps.wking.devcluster.openshift.com.' recordType: CNAME targets: - ab37f072ec51d11e98a7a02ae97362dd-240922428.us-west-2.elb.amazonaws.com status: zones: - dnsZone: tags: Name: wking-nfnsr-int kubernetes.io/cluster/wking-nfnsr: owned - dnsZone: id: Z3URY6TWQ91KVV The route listing is from a cluster running [6]. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1715635 [2]: https://github.com/openshift/cluster-ingress-operator/blob/9ce86811e6aeea58d3b2b8955591d50cd9311885/pkg/operator/controller/ingress/dns.go#L75-L115 [3]: https://pyyaml.org/ [4]: https://github.com/jzelinskie/faq [5]: https://github.com/openshift/cluster-ingress-operator/blob/d115a146611871c47f44f239346ecea0015993d9/pkg/api/v1/types.go#L18-L25 [6]: https://mirror.openshift.com/pub/openshift-v4/clients/ocp-dev-preview/4.2.0-0.nightly-2019-08-25-233755/
65cbc79
to
14e0691
Compare
|
||
```console | ||
$ rm -f openshift/99_openshift-cluster-api_master-machines-*.yaml openshift/99_openshift-cluster-api_worker-machinesets-*.yaml | ||
``` | ||
|
||
You are free to leave the compute MachineSets in if you want to create compute machines via the machine API, but if you do you may need to update the various references (`subnet`, etc.) to match your environment. | ||
|
||
### Remove DNS Zones |
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.
is it useful to mark Optional
in the title?
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.
If so, we should do the same with other Optional
tasks.
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.
Both sentences in this section open with If ...
. I think that's enough to make the optional-ness clear, but don't have a problem if folks want to adjust headers to hit that nail even more firmly ;).
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, wking 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
@wking: All pull requests linked via external trackers have merged. Bugzilla bug 1715635 has been moved to the MODIFIED state. 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 kubernetes/test-infra repository. |
/cc @kalexand-rh we'll need to get this reflected in product docs too, same situation on GCP as well. https://bugzilla.redhat.com/show_bug.cgi?id=1745907 |
… creation I just dropped this in at the end in 14e0691 (docs/user/aws/install_upi: Document bring-your-own-DNS, 2019-08-14, openshift#2221), but you need it for a functioning cluster (something about console and OAuth and mumble mumble). Jeremiah got the placement right for GCP in 16d4d38 (upi/gcp: document manual creation of apps DNS records, 2019-08-29, openshift#2289). This commit updates AWS to match.
… creation I just dropped this in at the end in 14e0691 (docs/user/aws/install_upi: Document bring-your-own-DNS, 2019-08-14, openshift#2221), but you need it for a functioning cluster (something about console and OAuth and mumble mumble). Jeremiah got the placement right for GCP in 16d4d38 (upi/gcp: document manual creation of apps DNS records, 2019-08-29, openshift#2289). This commit updates AWS to match.
… creation I just dropped this in at the end in 14e0691 (docs/user/aws/install_upi: Document bring-your-own-DNS, 2019-08-14, openshift#2221), but you need it for a functioning cluster (something about console and OAuth and mumble mumble). Jeremiah got the placement right for GCP in 16d4d38 (upi/gcp: document manual creation of apps DNS records, 2019-08-29, openshift#2289). This commit updates AWS to match.
Some users want to provide their own
*.apps
DNS records instead of delegating that to the ingress operator 1. With this commit, we tell the ingress operator not to worry about managing any hosted zones, and walk users through how they can create the expected records themselves.This is a WIP until I fill in a few outstanding FIXMEs.