Skip to content

Conversation

@jboxman
Copy link
Contributor

@jboxman jboxman commented Aug 28, 2020

@jboxman jboxman added this to the Future Release milestone Aug 28, 2020
@jboxman jboxman self-assigned this Aug 28, 2020
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 28, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@jboxman jboxman changed the title Add OVN-Kubernetes egress IP support Add OVN-Kubernetes egress IP support [4.6] Aug 31, 2020
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 29, 2020
@jboxman jboxman force-pushed the OSDOCS-1246 branch 3 times, most recently from d66a903 to 821010d Compare September 30, 2020 01:35
Copy link

@alexanderConstantinescu alexanderConstantinescu left a comment

Choose a reason for hiding this comment

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

@jboxman : I left these comments on the docs, in general it looks good though.

Choose a reason for hiding this comment

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

This is not quite true, due to bug: https://bugzilla.redhat.com/show_bug.cgi?id=1877273 (which will not be fixed in the 4.6 release and will have to be Z-streamed). The impact of that bug is: the egress IP will not fail over if it simply stops functioning for whatever reason. It must be deleted by the user by doing a oc delete node $NODE for that to happen. I would thus re-phrase for the moment and say: If a node is deleted by the cluster admin, any egress IP addresses...

Choose a reason for hiding this comment

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

And possibly add a note for this corner case of having a "silent network failure" for a node, which is not supported today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexanderConstantinescu, what might constitute the class of silent network failures? Is there a class of noisy network failures that OCP does detect? I can say:

NOTE: An egress IP address is not reassigned if a connectivity interruption is undetected.

But I'm not sure how to elaborate on that.

Choose a reason for hiding this comment

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

You bring up a good point. "Silently" to me is if a node can't be reached anymore networking wise, but Kubernetes/OpenShift seems to think everything is running fine and indicates such. But actually, if a node X's networking is down (even silently), the kubelet will stop reporting the node's status and thus a cluster admin of Kubernetes / OpenShift will know. We will still not be able to handle that today for egress IP, so I would say: If a node is deleted by the cluster admin, any egress IP addresses... because even if it's unavailable (silently or not) we still don't handle that case. But this will need to be fixed in the docs once that bug is solved, but I will contact you then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexanderConstantinescu, so I've updated this to:

  • If a node is deleted by a cluster administrator, any egress IP addresses assigned to it are automatically reassigned, subject to the previously described conditions.
    NOTE: An egress IP address is not reassigned if a connectivity interruption is undetected.

But maybe I don't need the note?

Thanks!

Choose a reason for hiding this comment

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

Yeah, I would say the NOTE adds confusion. The text is already explicit IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jboxman jboxman added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 5, 2020
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I have some questions and suggestions, but this is looking good.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anything else that a user might want to know about how that IP address is reserved? (I've seen enough about conflicting IP addresses and ranges that it might be worth saying more.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Later in this module, it sounds like you need an IP address range, but here it sounds like there's just one egress IP address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexanderConstantinescu, does it make sense to elaborate on this?

Choose a reason for hiding this comment

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

Yes, I would maybe add that An egress IP address is implemented as an additional IP address on the primary network interface of a node and must be in the same subnet as the primary IP address of the node, but it cannot be the IP address of any cluster node.. It needs to be clear to the user that if they do that, bad things will happen. Normally none would ever do such a thing...but you never know how courageous some people are ?

They never configure IP address ranges either, for OVN-Kubernetes it will only accept one or multiple IP address defined in the .spec.egressIPs section of the EgressIP object stanza.

Hope that's clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kalexand-rh kalexand-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 Oct 7, 2020
@jboxman
Copy link
Contributor Author

jboxman commented Oct 7, 2020

@kalexand-rh thanks!

@jboxman
Copy link
Contributor Author

jboxman commented Oct 8, 2020

@alexanderConstantinescu,

I don't know if this ever came up, but for OpenShift SDN questions regarding whether and how it works on cloud providers come up frequently; Is there anything of note with EgressIP under OVN-Kubernetes related to cloud vs non-cloud deployments?

Thanks!

@jboxman jboxman force-pushed the OSDOCS-1246 branch 3 times, most recently from 64bd90b to c833eaa Compare October 8, 2020 16:27
@alexanderConstantinescu
Copy link

@alexanderConstantinescu,

I don't know if this ever came up, but for OpenShift SDN questions regarding whether and how it works on cloud providers come up frequently; Is there anything of note with EgressIP under OVN-Kubernetes related to cloud vs non-cloud deployments?

Thanks!

GCP, AWS, Azure, etc all require manual assignment of additional IPs to VM NICs through the cloud provider's console. This is something we on OpenShift's side cannot do (or at least don't do currently).

So yes, we should mention that this will work "as is" for bare metal. For vSphere and Openstack the user will need to configure their deployment to allow automatic assignment by OpenShift, so it is possible with this additional step. I don't know exactly what that step is though, but the customer's who do this, will probably know.

@jboxman
Copy link
Contributor Author

jboxman commented Oct 9, 2020

Hi @huiran0826,

Can you confirm this PR for OVN-Kubernetes egress IP support?

Thanks!

@jboxman
Copy link
Contributor Author

jboxman commented Oct 9, 2020

@alexanderConstantinescu, added the following to the diagram discussion, which I think is correct?

When the external service receives traffic from any of the pods selected by the example EgressIP object, the source IP address is either 192.168.126.10 or 192.168.126.102.

@huiran0826
Copy link

lgtm

@jboxman
Copy link
Contributor Author

jboxman commented Oct 12, 2020

@danielmellado, do you know whether automatic assignment of additional IP addresses is possible on RHOSP?

Thanks!

@alexanderConstantinescu

@alexanderConstantinescu, added the following to the diagram discussion, which I think is correct?

When the external service receives traffic from any of the pods selected by the example EgressIP object, the source IP address is either 192.168.126.10 or 192.168.126.102.

That's correct

@jboxman jboxman force-pushed the OSDOCS-1246 branch 2 times, most recently from dc0e420 to f8be6e0 Compare October 13, 2020 19:48
- https://issues.redhat.com/browse/OSDOCS-1246

Co-authored-by: Kathryn Alexander <37149781+kalexand-rh@users.noreply.github.com>
@jboxman jboxman merged commit 3841343 into openshift:master Oct 13, 2020
@jboxman jboxman deleted the OSDOCS-1246 branch October 13, 2020 20:05
@jboxman
Copy link
Contributor Author

jboxman commented Oct 13, 2020

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 13, 2020

@jboxman: new pull request created: #26423

In response to this:

/cherry-pick enterprise-4.6

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.

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

Labels

branch/enterprise-4.6 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

Development

Successfully merging this pull request may close these issues.

8 participants