Allow egress-router to connect to DNS IP #20115
Conversation
openshift-ci-robot
added
the
size/M
|
@openshift/sig-networking @knobunc @danwinship @ironcladlou PTAL |
| @@ -230,6 +230,22 @@ func (p *cniPlugin) CmdAdd(args *skel.CmdArgs) error { | |||
| if err := netlink.RouteAdd(route); err != nil { | |||
| return fmt.Errorf("failed to add route to service network: %v", err) | |||
| } | |||
|
|
|||
| // Add a route to DNS IP via SDN | |||
There was a problem hiding this comment.
should probably have a loop for this and the node IP and the service CIDR rather than having basically the same code three times. (If you just add "/32" to the end of the two IPs you can treat them all as CIDRs).
(And I think it does make sense to keep the node and service special cases, even with the DNS IP being handled explicitly, because there might be other reasons besides DNS that we care about access to those IPs in the future, and they are only available via the SDN interface.)
There was a problem hiding this comment.
Yes, that was the same reason for keeping node IP and service CIDR routes.
Okay, I will create a loop for this.
pravisankar
force-pushed
the
egress-router-add-dns-route
branch
from
c28619b to
594e096
Compare
|
@danwinship @knobunc Updated, PTAL |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, pravisankar 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 |
openshift-ci-robot
added
the
approved
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@pravisankar: The following test failed, say
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. I understand the commands that are listed here. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
This will address use case (2) in #20094 (comment)