Bug 1873311: 8 27 2020 merge #243
Conversation
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
Custom VersionPrinter
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Keep `getPodIPs` true to its functional name `getPodIPs` was doing additional operations not related to "getting the pod IPs". Move that to `addPodEgressIP` Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Rename nodeLogicalRouterIP with IP version for egress IP tests Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Fix variable spelling mistake Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
The initial implementation of egress IP in shared gateway mode was dependent on having the bug: https://bugzilla.redhat.com/show_bug.cgi?id=1861294 resolved. This is now the case and the fix is in the latest Fedora 31 release. This commit thus switches to the correct shared gateway mode implementation. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
Tag egress IP related OVN transactions with external_id This patch deals with avoiding inconsistent state when a pod matches multiple egress IPs. Before this patch: the lr-policy-add / lr-nat-add commands failed once the second egress IP setup happened. This wasn't a problem in itself, but if that egress IP was later deleted: it then proceeded to delete the egress setup for the first egress IP. It goes like this: 1) EgressIP-1 is created matching pod X, setup happens correctly 2) EgressIP-2 is created matching pod X, setup fails because that exact same setup (nat and policy rules) already exists 3) EgressIP-2 is deleted, deleting EgressIP-1's setup. 4) OVN state is now screwed up and pod egress functionality will no longer work even though EgressIP-1 still exists. To avoid this we have the possibility to tag the policy and nat objects with the external_id, which in this case is the EgressIP.Name. As such we will only try deleting whatever setup we find for that particular object. The OVN API for doing this is not as straightforward though, and we use an extra OVN transaction for the delete, but we win in correctness. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Expanding on the docstring for policyAlreadyExistsMsg Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Simplify logical router policy filter for egress IP Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Check if nat/lr-policy exists before creating it As to avoid duplicate objects in the OVN database, check that the NAT / logical router policy object which is about to be created does not already exist Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
The policy command in local gateway mode (specifying pkt_mark) was incorrect. It is not a column in the logical_router_policy table, it's an option. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
The egress IP code was modifying the API server object directly. Leading to data races when running with the race detector set to "on". Instead copy the object, modify it and update the API server object with the copied one. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Remove unecessary locking in egress tests. Egress tests were locking eIPAllocatorMutex as to avoid the data races, which have now been fixed by the top level commit. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Encapsulate all assignEgressIPs operations in lock Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> Rename utility functions in egress*_test.go to reflect its new behaviour
This PR introduces a fix for the issue addressed in ovn-org/ovn-kubernetes#1562 As of now, running hybrid-overlay-node as a Windows service relies on os.Exit() to stop the service, this PR removes abrupt exit by instead cancelling the context on svc.Stop. Signed-off-by: mansikulkarni96 <mankulka@redhat.com>
…ip_fixes Egress IP fixes
Moved installation of j2cli via PIP after parsing the arguments. Parsing command line arguments should be the first statement in the script so that if someone is just providing the "-h" flag we print out the help without doing any check. By moving the above statement as mentioned we have ensured "-h" flag works even if say pip is not installed in the system. Signed-off-by: Balaji Varadaraju <bvaradar@redhat.com>
Fix abrupt exit of Windows service
This reverts commit fd47587. We found that this causes problems with network policy ACLs when using multi-node setups where the network policy ACL only targets a portgroup of the destination, and in effect only places the ACL there with allow-related. This causes return traffic destined to another node to be dropped, because that node does not have any allow-related ACL, and thus never able to determine via CT if this is reply traffic. Signed-off-by: Tim Rozet <trozet@redhat.com>
…ange Revert "Fix default allow mgmt ACL using conntrack"
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, trozet 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
lgtm
|
/retest |
1 similar comment
|
/retest |
|
rhel8 RPM mirrors are broken and cannot download necessary RPMs: |
|
/retest |
openshift-ci-robot
added
the
bugzilla/severity-high
|
/retest Please review the full test history for this PR and help us cut down flakes. |
24 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. |
|
/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. |
|
/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. |
|
/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. |
|
@trozet: 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@trozet: All pull requests linked via external trackers have merged: Bugzilla bug 1873311 has been moved to the MODIFIED 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. |
Includes fixes for Network policy and egress IP.
Note the revert for allow-related will lower performance for pod<->pod traffic without network policy.