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

DOC-994 Istio ingress configuration #1734

Merged
merged 8 commits into from
Jan 7, 2022

Conversation

@kaitlynmichael kaitlynmichael added in progress Changes are in progress K8s Kubernetes labels Jan 5, 2022
@kaitlynmichael kaitlynmichael self-assigned this Jan 5, 2022
@laurentdroin
Copy link
Contributor

The IP addresses have an invalid format. For example, 12.345.678.910 will strike people as invalid since values would be <= 255 for ipv4. I know that it shouldn't matter since they are not supposed to be used as-is anyway but still, I can guarantee this will annoy many nerds, not just me :-)

@laurentdroin
Copy link
Contributor

v1alpha3 is old, let's not use it for the istio resources (I know it's all over the official istio examples but still).
Instead, we should go with "networking.istio.io/v1beta1"

@laurentdroin
Copy link
Contributor

I think that the following:

"The gateway’s metadata name must be similar to the gateway’s spec name (redis-gateway in this example)."

is misplaced. It would make some sense if the Gateway came second in the list of Istio resources but it comes first.

I believe that what we mean is the following:

The 'gateway listed in the "gateways" spec of the VirtualService resource must correspond to the name of the Gateway resource'
And for this to make sense, we need to have introduced both resources (i.e. we can't put this text in between resources).

Or something similar to the above... We just need to explain the link between the 2 resources.

@laurentdroin
Copy link
Contributor

I don't know if we are making it clear that with the VirtualService example, we are actually creating both a route to contact the API server and a route to contact one of the databases. A reader might wonder where is "db1" coming from so we may want to explain this a bit more.
And if we are just explaining how to access the databases, do we really need to bother with a route for the API?

@bascar
Copy link
Contributor

bascar commented Jan 6, 2022

v1alpha3 is old, let's not use it for the istio resources (I know it's all over the official istio examples but still). Instead, we should go with "networking.istio.io/v1beta1"

Have we tested against v1beta1?

@laurentdroin
Copy link
Contributor

v1alpha3 is old, let's not use it for the istio resources (I know it's all over the official istio examples but still). Instead, we should go with "networking.istio.io/v1beta1"

Have we tested against v1beta1?

Not that it's official, but I have.

@laurentdroin
Copy link
Contributor

laurentdroin commented Jan 6, 2022

@bascar Looks like there is no difference between v1alpha3 and v1beta1 - just better to go with the latest version.

See: https://discuss.istio.io/t/is-api-version-networking-istio-io-v1alpha3-considered-to-be-deprecated-on-1-5-0/7278

And that was 1.5 years ago, so we definitely should use v1beta1 IMO.

@kaitlynmichael
Copy link
Contributor Author

New edits from feedback:

  • new example IPs
  • replaced v1alpha3 with v1beta1
  • moved gateway metadata explanation after virtual service yaml
  • added note after vs example about creating route for API and db

Copy link
Collaborator

@rrelledge rrelledge left a comment

Choose a reason for hiding this comment

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

Looks good to me. I just left a few suggestions.

Co-authored-by: Rachel Elledge <86307637+rrelledge@users.noreply.github.com>
@kaitlynmichael kaitlynmichael merged commit a0f3166 into release-k8s-intrepid Jan 7, 2022
@kaitlynmichael kaitlynmichael deleted the jira-doc-994 branch January 7, 2022 20:43
kaitlynmichael added a commit that referenced this pull request Jan 11, 2022
* update desc and linktitle (#1732)

* DOC-994 Istio ingress configuration (#1734)

* openshift 3 bundle comments

* openshift version changes

* correct external IPs

* DOC-1092 adding Istio section to K8s AA article (#1742)

* adding istio section to AA article

* istio manual CRD warning

* DOC-1091 Intrepid release notes (#1738)

* new version tag, updated known issue

* small line edits, added private repo prereq (#1747)

* Apply suggestions from code review

Co-authored-by: Rachel Elledge <86307637+rrelledge@users.noreply.github.com>

Co-authored-by: Rachel Elledge <86307637+rrelledge@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
K8s Kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants