Bug 1988425: Change to use mountPath: /host #1169

mccv1r0 · 2021-07-30T13:32:32Z

Change iptables chain names to not conflict with existing workaround(s)

Signed-off-by: Michael Cambria mcambria@redhat.com
(cherry picked from commit 32d72e5)

Change iptables chain names to not conflict with existing workaround(s) Signed-off-by: Michael Cambria <mcambria@redhat.com> (cherry picked from commit 32d72e5)

openshift-ci · 2021-07-30T14:04:08Z

@mccv1r0: This pull request references Bugzilla bug 1988425, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @zhaozhanqi

In response to this:

Bug 1988425: Change to use mountPath: /host

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.

mccv1r0 · 2021-07-30T15:21:09Z

/test e2e-gcp-ovn

mccv1r0 · 2021-07-30T16:21:30Z

/test e2e-metal-ipi-ovn-ipv6
/test e2e-agnostic-upgrade

danwinship

It's confusing that the commit message summary only describes one of the two things the commit does. Either split it into two commits, or else make the summary more generic ("fixes to azure iptables setup script" etc) and describe both fixes in the commit message body, rather than one in the summary and one in the body

danwinship · 2021-07-30T17:00:36Z

bindata/network/openshift-sdn/sdn.yaml

+          iptables -F AZURE_ICMP_ACTION
+          iptables -A AZURE_ICMP_ACTION -j LOG
+          iptables -A AZURE_ICMP_ACTION -j DROP
+          /host/usr/bin/oc observe pods -n openshift-sdn -l app=sdn -a '{ .status.hostIP }' -- /var/run/add_iptables.sh


This seems not safe; is oc statically linked, but is it guaranteed to always be statically linked? I think you have to do chroot /host oc ... though of course then you also have to write the script out to /home/var/run rather than /var/run

I think we've glossed over this so far but due to FIPS needing openssl for Go, we can't rely on these things being fully static.

The other pattern is to use e.g. systemd-run -qPG -- oc observe pods -n openshift-sdn -l app=sdn -a '{ .status.hostIP }' -- /var/run/add_iptables.sh.

@mjudeikis fyi

The commit message already describes:

Change to use mountPath: /host

Change iptables chain names to not conflict with existing workaround(s)

For SDN, I'll probably need to change upstream to use systemd-run (or whatever ends up being used) and back port.

The OVN version of this fix will start with whatever consensus decides is best.

mccv1r0 · 2021-07-30T19:12:23Z

/test e2e-gcp-ovn
/test e2e-metal-ipi-ovn-ipv6

Makdaam · 2021-08-02T07:48:41Z

/retest

mccv1r0 · 2021-08-03T17:31:18Z

/retest

mccv1r0 · 2021-08-03T22:15:24Z

/test e2e-gcp-ovn
/test e2e-agnostic-upgrade

abhat · 2021-08-04T12:44:50Z

/retest-required

mccv1r0 · 2021-08-04T15:37:50Z

/retest-required

mccv1r0 · 2021-08-04T18:07:02Z

/retest-required

mccv1r0 · 2021-08-04T20:21:01Z

/test e2e-gcp-ovn

Makdaam · 2021-08-05T11:37:19Z

/test e2e-gcp-ovn

mccv1r0 · 2021-08-05T14:31:56Z

/test e2e-gcp-ovn

Makdaam · 2021-08-06T10:00:58Z

/test e2e-gcp-ovn

mccv1r0 · 2021-08-06T13:10:15Z

/test e2e-gcp-ovn

mccv1r0 · 2021-08-06T15:23:43Z

@knobunc Can you override this required gcp test? As you know this PR changes yaml that runs on Azure only.

mccv1r0 · 2021-08-06T21:28:58Z

/test e2e-gcp-ovn

mccv1r0 · 2021-08-09T13:02:03Z

/test e2e-gcp-ovn

mccv1r0 · 2021-08-09T18:51:35Z

/test e2e-openstack-ovn
/test e2e-gcp-ovn-upgrade

brenton · 2021-08-10T14:01:37Z

Is this PR opened against the correct branch? I would have expected it to merge into release-4.8. When I look now the master, release-4.10 and release-4.9 branches all already have the fix from https://bugzilla.redhat.com/show_bug.cgi?id=1984449.

abhat · 2021-08-10T16:37:28Z

@mccv1r0 since this is a master branch PR, the bz needs retitling?

mccv1r0 · 2021-08-10T16:43:10Z

@mccv1r0 since this is a master branch PR, the bz needs retitling?

The PR needs to be against 4.8

It's a cherry-pick from master

mccv1r0 · 2021-08-11T12:54:48Z

/test e2e-azure-ovn-dualstack

openshift-bot · 2021-08-11T13:56:00Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

mccv1r0 · 2021-08-11T15:23:54Z

Must we hold up an SDN only change due to an OVN flake?

mccv1r0 · 2021-08-11T15:23:58Z

/test e2e-azure-ovn-dualstack

mccv1r0 · 2021-08-11T15:47:09Z

/test e2e-azure-ovn-dualstack

openshift-bot · 2021-08-11T17:21:03Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T17:33:12Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T18:46:00Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T20:12:05Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-11T21:00:07Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

mffiedler · 2021-08-11T21:30:11Z

Verified as with #1160. 4.8.z cluster-bot Azure cluster built from this PR. ssh into node works fine both before and after boot. Without this PR ssh just hangs.

/lgtm
/label qe-approved

mccv1r0 · 2021-08-11T21:34:32Z

/test e2e-azure-ovn-dualstack

openshift-ci · 2021-08-11T21:37:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, mccv1r0, mffiedler

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:

~~OWNERS~~ [dcbw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-08-11T22:50:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T02:08:07Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T02:20:12Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T06:10:07Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T06:36:14Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T07:15:07Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T07:27:08Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T12:53:36Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T13:18:36Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-12T14:46:35Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

dcbw · 2021-08-12T15:07:46Z

/override ci/prow/e2e-azure-ovn-dualstack

Error on cloud setup:
level=error msg=Error: updating Load Balancer "ci-op-5hhdw15j-f5292-k6tnt" (Resource Group "ci-op-5hhdw15j-f5292-k6tnt-rg") for Probe "api-internal-probe": network.LoadBalancersClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="PrivateIPAddressVersionCannotBeModified" Message="Load balancer frontend IP configuration /subscriptions/d38f1e38-4bed-438e-b227-833f997adf6a/resourceGroups/ci-op-5hhdw15j-f5292-k6tnt-rg/providers/Microsoft.Network/loadBalancers/ci-op-5hhdw15j-f5292-k6tnt/frontendIPConfigurations/public-lb-ip-v6 has an existing private IP address version IPv6 and the request has private IP address version IPv4, which do not match. Private IP address version cannot be modified once the resource is created." Details=[]

openshift-ci · 2021-08-12T15:17:48Z

@dcbw: Overrode contexts on behalf of dcbw: ci/prow/e2e-azure-ovn-dualstack

In response to this:

/override ci/prow/e2e-azure-ovn-dualstack

Error on cloud setup:
level=error msg=Error: updating Load Balancer "ci-op-5hhdw15j-f5292-k6tnt" (Resource Group "ci-op-5hhdw15j-f5292-k6tnt-rg") for Probe "api-internal-probe": network.LoadBalancersClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="PrivateIPAddressVersionCannotBeModified" Message="Load balancer frontend IP configuration /subscriptions/d38f1e38-4bed-438e-b227-833f997adf6a/resourceGroups/ci-op-5hhdw15j-f5292-k6tnt-rg/providers/Microsoft.Network/loadBalancers/ci-op-5hhdw15j-f5292-k6tnt/frontendIPConfigurations/public-lb-ip-v6 has an existing private IP address version IPv6 and the request has private IP address version IPv4, which do not match. Private IP address version cannot be modified once the resource is created." Details=[]

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.

openshift-ci · 2021-08-12T15:18:54Z

@mccv1r0: All pull requests linked via external trackers have merged:

openshift/cluster-network-operator#1169

Bugzilla bug 1988425 has been moved to the MODIFIED state.

In response to this:

Bug 1988425: Change to use mountPath: /host

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.

Change to use mountPath: /host

be30472

Change iptables chain names to not conflict with existing workaround(s) Signed-off-by: Michael Cambria <mcambria@redhat.com> (cherry picked from commit 32d72e5)

openshift-ci bot requested review from danwinship and dcbw July 30, 2021 13:32

mccv1r0 changed the title ~~Change to use mountPath: /host~~ Bug 1988425: Change to use mountPath: /host Jul 30, 2021

openshift-ci bot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 30, 2021

openshift-ci bot requested a review from zhaozhanqi July 30, 2021 14:04

danwinship reviewed Jul 30, 2021

View reviewed changes

bparees added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 11, 2021

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 11, 2021

openshift-ci bot assigned mffiedler Aug 11, 2021

openshift-ci bot merged commit f7e31c5 into openshift:release-4.8 Aug 12, 2021

Bug 1988425: Change to use mountPath: /host #1169

Bug 1988425: Change to use mountPath: /host #1169

Conversation

mccv1r0 commented Jul 30, 2021

openshift-ci bot commented Jul 30, 2021

mccv1r0 commented Jul 30, 2021

mccv1r0 commented Jul 30, 2021

danwinship left a comment

Choose a reason for hiding this comment

danwinship Jul 30, 2021 • edited

Choose a reason for hiding this comment

cgwalters Jul 30, 2021

Choose a reason for hiding this comment

cgwalters Jul 30, 2021

Choose a reason for hiding this comment

mccv1r0 Jul 30, 2021

Choose a reason for hiding this comment

mccv1r0 Aug 3, 2021

Choose a reason for hiding this comment

mccv1r0 commented Jul 30, 2021

Makdaam commented Aug 2, 2021

mccv1r0 commented Aug 3, 2021

mccv1r0 commented Aug 3, 2021

abhat commented Aug 4, 2021

mccv1r0 commented Aug 4, 2021

mccv1r0 commented Aug 4, 2021

mccv1r0 commented Aug 4, 2021

Makdaam commented Aug 5, 2021

mccv1r0 commented Aug 5, 2021

Makdaam commented Aug 6, 2021

mccv1r0 commented Aug 6, 2021

mccv1r0 commented Aug 6, 2021

mccv1r0 commented Aug 6, 2021

mccv1r0 commented Aug 9, 2021

mccv1r0 commented Aug 9, 2021

brenton commented Aug 10, 2021

abhat commented Aug 10, 2021

mccv1r0 commented Aug 10, 2021 • edited

mccv1r0 commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

mccv1r0 commented Aug 11, 2021

mccv1r0 commented Aug 11, 2021

mccv1r0 commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

mffiedler commented Aug 11, 2021 • edited

mccv1r0 commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

openshift-bot commented Aug 11, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

openshift-bot commented Aug 12, 2021

dcbw commented Aug 12, 2021

openshift-ci bot commented Aug 12, 2021

openshift-ci bot commented Aug 12, 2021

danwinship Jul 30, 2021 •

edited

mccv1r0 commented Aug 10, 2021 •

edited

mffiedler commented Aug 11, 2021 •

edited