Bug 1881143: Backport handle azure vips for pods #2111
Conversation
The original introduction of this service probably used `gcpRoutesController` which happens to be the same as the MCO image because we didn't have a reference to it, and plumbing the image substitution through all the abstraction layers in the code is certainly not obvious. Prep for openshift#2011 which wants to abstract the GCP work to also handle Azure and it was confusing that `machine-config-daemon-pull.service` was referencing an image with a GCP name.
This PR does the following things: - Rename gcp-routes-controller to apiserver-watcher, since it is generic - Remove obsolete service-management mode from gcp-routes-controller - Change downfile directory to /run/cloud-routes from /run/gcp-routes - Write $VIP.up as well as $VIP.down - Add an azure routes script that fixes hairpin. Background: Azure hosts cannot hairpin back to themselves over a load balancer. Thus, we need to redirect traffic to the apiserver vip to ourselves via iptables. However, we should only do this when our local apiserver is running. The apiserver-watcher drops a $VIP.up and $VIP.down file, accordingly, depending on the state of the apiserver. Then, we add or remove iptables rules that short-circuit the load balancer. Unlike GCP, we don't need to do this for external traffic, only local clients.
Azure hosts cannot hairpin back to themselves over a load balancer. Thus, we need to redirect traffic to the apiserver vip to ourselves via iptables. However, we should only do this when our local apiserver is running. Previously it was added support for host network only, adding the same iptables rules in PREROUTING we apply the same rules for pods. Signed-off-by: Antonio Ojea <aojea@redhat.com>
openshift-ci-robot
added
bugzilla/severity-high
|
@aojea: This pull request references Bugzilla bug 1881143, which is invalid:
Comment In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aojea The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test e2e-azure |
|
/retest |
|
@deads2k this does not seem an easy backport, can you tag somebody with better understanding of the machine operator that can guide me through? |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
@cgwalters I'm finding some difficult with this backport, it seems to depend on other commits and I'm afraid of breaking something, can you advice on how to proceed? |
|
Hm...this is a large change to land in a released version. I would prefer we ship 4.6 first and only backport once we have clear signal there's no other issues. Anyone running 4.5 and is affected can then also upgrade to 4.6 anyways. |
The issue we're having is that upgrades from levels that don't have this fix on azure are unreliable. this means that a lack of a backport to 4.5 means that upgrades to 4.6 don't go smoothly. In fact, they go worse than upgrades from 4.5.z to 4.5.z+1 because the internal LB is expected to work in 4.6 and the MCO goes last. They don't get stuck, but they do face multi-minute gaps in endpoint establishment. CI and upgrades on 4.6 look good, the endpoint checker has reported no failures after the changes were merged. How much more signal you looking for? |
|
@cgwalters Your reasoning seems sound, what your preferred timeline look like? Merge at 4.6 GA? 1 week after GA? Keep in mind that the pipeline introduces some lag so it's about 14 days from the time this merges until it's shipped. @aojea Can you link to all of the upstream PRs so it's easier to determine how long these changes have soaked in master branch? |
|
/retest |
|
@aojea: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
It seems that this is breaking the azure and gcp jobs, they are failing consistently at the installation time https://prow.ci.openshift.org/pr-history/?org=openshift&repo=machine-config-operator&pr=2111 I think that the backport is not straight forward, it seems that these changes depend on some changes introduced in 4.6 ... and I don't have enough knowledge of the MCO to be able to track them down 😓 |
|
/cc @squeed |
|
@aojea: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
|
/close |
|
@aojea: Closed this PR. In response to this:
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. |
|
@aojea: This pull request references Bugzilla bug 1881143. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. In response to this:
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. |
This PR does the following things:
Rename gcp-routes-controller to apiserver-watcher, since it is generic
Remove obsolete service-management mode from gcp-routes-controller
Change downfile directory to /run/cloud-routes from /run/gcp-routes
Write $VIP.up as well as $VIP.down
Add an azure routes script that fixes hairpin.
Background: Azure hosts cannot hairpin back to themselves over a load balancer. Thus, we need to redirect traffic to the apiserver vip to ourselves via iptables. However, we should only do this when our local apiserver is running.
The apiserver-watcher drops a $VIP.up and $VIP.down file, accordingly, depending on the state of the apiserver. Then, we add or remove iptables rules that short-circuit the load balancer.
Unlike GCP, we don't need to do this for external traffic, only local clients.
Install on azure, ensure connections to the internal API load balancer are reliable - both when the local apiserver process is running and stopped.
- Description for the changelog