Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 2095444: EGW: Clean Stale Conntrack Entries #1189

Merged
merged 4 commits into from Jul 15, 2022
Merged

Bug 2095444: EGW: Clean Stale Conntrack Entries #1189

merged 4 commits into from Jul 15, 2022

Conversation

tssurya
Copy link
Contributor

@tssurya tssurya commented Jul 13, 2022

COMMIT 1: Conflict because ovn-org/ovn-kubernetes#3001 is missing

commit b21c93546e86b569228bc81e3aaa9aa3c7e6dce8                                                                                                                              
Author: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                                     
Date:   Thu Jun 16 22:07:02 2022 +0200                                                                                                                                       
    Vendor github.com/j-keck/arping library 
    Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
    (cherry picked from commit f12f04da3f0b82dfe7f0d4d03900ef28a84c24dc)
     Conflicts:
            go-controller/go.sum
            go-controller/vendor/modules.txt
     because https://github.com/ovn-org/ovn-kubernetes/pull/3001 is missing

COMMIT2: CLEAN PICK

commit 153bfe7a5f353911f353352e0a89b08e4e8e72a1                                                                                                                              
Author: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                                     
Date:   Wed Jul 6 21:14:36 2022 +0200                                                                                                                                                                                                                                               
    CARRY VENDOR: TEMP: netlink: Conntrack Label Filter                                                                                                                                                                                                                                                                                     
    This commit can be reverted when                                                                                                                                         
    https://github.com/vishvananda/netlink/pull/784/                                                                                                                         
    merges and we can re-vendor the next netlink release                                                                                                                     
                                                                                                                                                                             
    Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                          
    (cherry picked from commit ecd92a8cedaafd67b485f4be7d55b4c591da2ee2)

COMMIT3: Conflict because ovn-org/ovn-kubernetes#2927 is missing

commit 49ae83eadb5dd8ff84d29ef6760a7982aaf19538                                                                                                                              
Author: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                                     
Date:   Thu Jul 7 13:21:51 2022 +0200                                                                                                                                                                                                                                                                                                   
    Add utils for namespace annotations                                                                                                                                                                                                                                                                                                                  
    Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                          
    (cherry picked from commit 739395d0a181707203b18d720d454edf3a2f9319)                                                                                                                                                                                                                                                                                
     Conflicts:                                                                                                                                                              
            go-controller/pkg/ovn/egressfirewall_test.go                                                                                                                                                                                                                                                                                    
    because https://github.com/ovn-org/ovn-kubernetes/pull/2927 is missing

COMMIT4: CLEAN PICK

commit c3f54d0529833a72b62a1979886bb5686a4f8803                                                                                  
Author: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                                     
Date:   Thu Jun 16 22:09:19 2022 +0200                                                                                                                                                                                                                                                                                                        
    EGW: Delete stale conntrack entries                                                                                                                                                                                                                                                                                                       
    This commit adds logic to delete the conntrack entries                                                                                                                   
    that contain src MAC address in the "labels" field when                                                                                                                  
    using ECMP routes on the GR.                                                                                                                                                                                                                                                                                                                
    Logic:                                                                                                                                                                                                                                                                                                                                           
    1) annotate the namespace each time an exgw is added/deleted with list of ips                                                                                            
    2) add new informer for namespace on node side checking only if gw ip annotation OR external-gws annotation changed                                                      
    3) ovnkube node on namespace change, iterates through all the ips and initiates an arp request via ovnk and collects the MACs                                            
    4) once all the responses come back, we have all the known macs                                                                                                          
    5) we search for ct entries for any pod ip belonging to the namespace, if ct_label is loaded with a mac not in our list we flush it                                      
    6) we run the above in a goroutine as well set which will run every 5mins looping through all relevant namespaces.                                                                                                                                                                                  
    Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>                                                                                                          
    (cherry picked from commit 5a3a8b83ae01c0c2ce4935ec9875785eba309076)

/assign @trozet
/assign @dcbw

@tssurya
Vendor github.com/j-keck/arping library
b21c935 
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
(cherry picked from commit f12f04d)

 Conflicts:
	go-controller/go.sum
	go-controller/vendor/modules.txt

 because ovn-org/ovn-kubernetes#3001 is missing
@tssurya
CARRY VENDOR: TEMP: netlink: Conntrack Label Filter
153bfe7 
This commit can be reverted when
vishvananda/netlink#784
merges and we can re-vendor the next netlink release

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
(cherry picked from commit ecd92a8)
@tssurya
Add utils for namespace annotations
49ae83e 
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
(cherry picked from commit 739395d)

 Conflicts:
	go-controller/pkg/ovn/egressfirewall_test.go

because ovn-org/ovn-kubernetes#2927 is missing
@tssurya
EGW: Delete stale conntrack entries
c3f54d0 
This commit adds logic to delete the conntrack entries
that contain src MAC address in the "labels" field when
using ECMP routes on the GR.

Logic:

1) annotate the namespace each time an exgw is added/deleted with list of ips
2) add new informer for namespace on node side checking only if gw ip annotation OR external-gws annotation changed
3) ovnkube node on namespace change, iterates through all the ips and initiates an arp request via ovnk and collects the MACs
4) once all the responses come back, we have all the known macs
5) we search for ct entries for any pod ip belonging to the namespace, if ct_label is loaded with a mac not in our list we flush it
6) we run the above in a goroutine as well set which will run every 5mins looping through all relevant namespaces.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
(cherry picked from commit 5a3a8b8)
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 13, 2022

@tssurya: Bugzilla bug 2095444 is in a bug group that is not in the allowed groups for this repo.
Allowed groups for this repo are:

  • nec
  • qe_staff
  • redhat

In response to this:

Bug 2095444: EGW: Clean Stale Conntrack Entries

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 openshift-ci bot requested review from abhat and trozet July 13, 2022 18:09
@tssurya tssurya mentioned this pull request Jul 13, 2022
Merged
@tssurya
Copy link
Contributor Author

tssurya commented Jul 13, 2022

/retest

@tssurya
Copy link
Contributor Author

tssurya commented Jul 14, 2022

/test e2e-openstack-ovn

@trozet
Copy link
Contributor

trozet commented Jul 14, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: trozet, tssurya

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2022
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 2 against base HEAD e4c125f and 8 for PR HEAD c3f54d0 in total

@tssurya
Copy link
Contributor Author

tssurya commented Jul 14, 2022

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2022

@tssurya: Bugzilla bug 2095444 is in a bug group that is not in the allowed groups for this repo.
Allowed groups for this repo are:

  • nec
  • qe_staff
  • redhat

In response to this:

/bugzilla refresh

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-robot
Copy link
Contributor

/retest-required

Remaining retests: 1 against base HEAD e4c125f and 7 for PR HEAD c3f54d0 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD e4c125f and 6 for PR HEAD c3f54d0 in total

@tssurya
Copy link
Contributor Author

tssurya commented Jul 15, 2022

/retest-required

1 similar comment
@tssurya
Copy link
Contributor Author

tssurya commented Jul 15, 2022

/retest-required

@tssurya
Copy link
Contributor Author

tssurya commented Jul 15, 2022

/test e2e-metal-ipi-ovn-dualstack

@tssurya
Copy link
Contributor Author

tssurya commented Jul 15, 2022

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 15, 2022

@tssurya: Bugzilla bug 2095444 is in a bug group that is not in the allowed groups for this repo.
Allowed groups for this repo are:

  • nec
  • qe_staff
  • redhat

In response to this:

/bugzilla refresh

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
Copy link
Contributor

openshift-ci bot commented Jul 15, 2022 •
edited

@tssurya: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack-ovn c3f54d0 link false /test e2e-openstack-ovn

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.

@trozet
Copy link
Contributor

trozet commented Jul 15, 2022

/override ci/prow/e2e-metal-ipi-ovn-dualstack

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 15, 2022

@trozet: Overrode contexts on behalf of trozet: ci/prow/e2e-metal-ipi-ovn-dualstack

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-dualstack

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 openshift-ci bot merged commit f691b03 into openshift:master Jul 15, 2022
@anuragthehatter
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 15, 2022
@tssurya
Copy link
Contributor Author

tssurya commented Jul 17, 2022

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 17, 2022

@tssurya: Bugzilla bug 2095444 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state.

In response to this:

/bugzilla refresh

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
Reviewers

@abhat abhat Awaiting requested review from abhat

@trozet trozet Awaiting requested review from trozet

Assignees

@trozet trozet

@dcbw dcbw

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tssurya @trozet @openshift-ci-robot @anuragthehatter @dcbw