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.

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

Jump to bottom

bug 1743483 adding DNS record data #17190

Merged
merged 1 commit into from
Nov 6, 2019
Merged

bug 1743483 adding DNS record data #17190

merged 1 commit into from
Nov 6, 2019

Conversation

kalexand-rh
Copy link
Contributor

https://bugzilla.redhat.com/show_bug.cgi?id=1743483

Adding the steps to create DNS records to match https://github.com/openshift/installer/pull/2221/files#diff-a2ee8aa448a0244512469c9c7126465f\

I think that more work is required to satisfy the bug, but I'm not sure that I have all of the information that is required.

@kalexand-rh kalexand-rh added this to the Future Release milestone Oct 10, 2019
@kalexand-rh kalexand-rh self-assigned this Oct 10, 2019
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2019
@openshift-docs-preview-bot
Copy link

The preview will be available shortly at:

wking
wking reviewed Oct 10, 2019
View reviewed changes
installing/installing_aws_user_infra/installing-aws-user-infra.adoc Outdated
[id="installing-custom-dns-records"]
== Creating custom DNS records

If you must create the DNS entries for your cluster, create them now.
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs to go into modules/installation-aws-user-infra-installation.adoc so it can happen after removing bootstrap resources and before wait-for install-complete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then why's yours after wait-for install-complete?

Copy link
Member

Choose a reason for hiding this comment

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

Then why's yours after wait-for install-complete?

Fix in flight openshift/installer#2496 :)

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2019
@kalexand-rh
Copy link
Contributor Author

@wking, do you see any other changes, or should I pass this to QE?

gpei
gpei reviewed Oct 25, 2019
View reviewed changes
modules/installation-create-ingress-dns-records.adoc Outdated Show resolved Hide resolved
modules/installation-create-ingress-dns-records.adoc
> ]
> }'
----
<1> For `<private_hosted_zone_id>`, specify the value from the output of the CloudFormation template for DNS and load balancing.
Copy link

Choose a reason for hiding this comment

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

This is not mentioned in this doc, for how to obtain <private_hosted_zone_id>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a prereq that says that you've deployed an AWS UPI cluster.

modules/installation-create-ingress-dns-records.adoc Show resolved Hide resolved
modules/installation-create-ingress-dns-records.adoc

Z3AADJGX6KTTL2
----
<1> For `<external_ip>`, specify the value of the external IP address of the Ingress Operator load balancer that you obtained.
Copy link

Choose a reason for hiding this comment

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

the value of the EXTERNAL-IP address of the Ingress Operator...

may be better to correspond to the description above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll modify the surrounding text a little, but our style conventions mean that EXTERNAL-IP and <external_ip> need to stay the way they are. (One is referring to the label that people need to check, and the other conforms to our variable format.)

modules/installation-create-ingress-dns-records.adoc Outdated Show resolved Hide resolved
modules/installation-create-ingress-dns-records.adoc Outdated Show resolved Hide resolved
modules/installation-create-ingress-dns-records.adoc Outdated Show resolved Hide resolved
@jianlinliu
Copy link

LGTM.

@kalexand-rh kalexand-rh changed the title [WIP] bug 1743483 adding DNS record data bug 1743483 adding DNS record data Nov 6, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2019
@kalexand-rh kalexand-rh added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 6, 2019
@kalexand-rh
Copy link
Contributor Author

@openshift/team-documentation, PTAL?

@kalexand-rh kalexand-rh removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2019
sheriff-rh
sheriff-rh approved these changes Nov 6, 2019
View reviewed changes
Copy link
Contributor

@sheriff-rh sheriff-rh left a comment

Choose a reason for hiding this comment

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

Looks great, Kathryn!

@sheriff-rh sheriff-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 6, 2019
@kalexand-rh
Copy link
Contributor Author

Thanks! Merging.

@kalexand-rh kalexand-rh merged commit 3d8956c into openshift:master Nov 6, 2019
@kalexand-rh
Copy link
Contributor Author

/cherrypick enterprise-4.2

@kalexand-rh
Copy link
Contributor Author

/cherrypick enterprise-4.3

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 6, 2019
Merged
@openshift-cherrypick-robot
Copy link

@kalexand-rh: new pull request created: #17938

In response to this:

/cherrypick enterprise-4.2

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.

@kalexand-rh kalexand-rh deleted the BZ1743483 branch November 6, 2019 19:06
@openshift-cherrypick-robot
Copy link

@kalexand-rh: new pull request created: #17939

In response to this:

/cherrypick enterprise-4.3

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.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wking wking wking left review comments

@gpei gpei gpei left review comments

@sheriff-rh sheriff-rh sheriff-rh approved these changes

Assignees

@kalexand-rh kalexand-rh

Labels
branch/enterprise-4.2 branch/enterprise-4.3 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.3 GA
Development

Successfully merging this pull request may close these issues.
9 participants
@kalexand-rh @openshift-docs-preview-bot @jianlinliu @openshift-cherrypick-robot @wking @gpei @sheriff-rh @vikram-redhat @openshift-ci-robot