Skip to content

added ingress support section #8638

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
Merged
merged 1 commit into from
Apr 11, 2018
Merged

added ingress support section #8638

merged 1 commit into from
Apr 11, 2018

Conversation

bfallonf
Copy link

@bfallonf bfallonf commented Apr 9, 2018 •
edited
Loading

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 9, 2018
@bfallonf bfallonf mentioned this pull request Apr 9, 2018
Closed
@JacobTanenbaum
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2018
@bfallonf
Copy link
Author

Neato. Thanks @JacobTanenbaum

@openshift/team-documentation PTAL

@bfallonf bfallonf added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 10, 2018
gaurav-nelson
gaurav-nelson reviewed Apr 10, 2018
View reviewed changes
architecture/networking/routes.adoc Outdated

For example, an ingress object configured as:

----
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add [source, yaml] tags before code blocs (2 instances)

gaurav-nelson
gaurav-nelson reviewed Apr 10, 2018
View reviewed changes
architecture/networking/routes.adoc Outdated
servicePort: 80
----

would generate the following route object:
Copy link
Contributor

Choose a reason for hiding this comment

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

s/would generate/generates

gaurav-nelson
gaurav-nelson reviewed Apr 10, 2018
View reviewed changes
architecture/networking/routes.adoc Outdated

[NOTE]
====
In order for a route to be created, an ingress object needs to have a host,
Copy link
Contributor

Choose a reason for hiding this comment

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

s/In order for/For

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe? s/needs to have/must have

gaurav-nelson
gaurav-nelson requested changes Apr 10, 2018
View reviewed changes
Copy link
Contributor

@gaurav-nelson gaurav-nelson left a comment

Choose a reason for hiding this comment

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

few nits, rest looks good. 👍

@gaurav-nelson gaurav-nelson 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 Apr 10, 2018
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2018
@bfallonf
Copy link
Author

Thanks @gaurav-nelson . I made changes as per your suggestions.

@bfallonf bfallonf added this to the Future Release milestone Apr 11, 2018
@bfallonf bfallonf merged commit 6491594 into openshift:master Apr 11, 2018
@bfallonf
Copy link
Author

/cherrypick enterprise-3.10

@openshift-cherrypick-robot
Copy link

@bfallonf: #8638 failed to apply on top of branch "enterprise-3.10":

.git/rebase-apply/patch:25: trailing whitespace.
ingress object. 
warning: 1 line adds whitespace errors.
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	architecture/networking/routes.adoc
Falling back to patching base and 3-way merge...
Auto-merging architecture/networking/routes.adoc
CONFLICT (content): Merge conflict in architecture/networking/routes.adoc
Patch failed at 0001 added ingress support section

In response to this:

/cherrypick enterprise-3.10

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.

@bfallonf bfallonf mentioned this pull request Apr 11, 2018
Merged
@bfallonf bfallonf deleted the ingress branch April 11, 2018 05:24
@vikram-redhat
Copy link
Contributor

@bfallonf - was this QEed?

@bfallonf
Copy link
Author

@vikram-redhat yes, in the PR this was based on.

@vikram-redhat
Copy link
Contributor

Removing dedicated and online labels as it doesn't apply to them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaurav-nelson gaurav-nelson gaurav-nelson approved these changes

Assignees

@JacobTanenbaum JacobTanenbaum

Labels
branch/enterprise-3.10 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 3.10 GA
Development

Successfully merging this pull request may close these issues.
6 participants
@bfallonf @JacobTanenbaum @openshift-ci-robot @openshift-cherrypick-robot @vikram-redhat @gaurav-nelson