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.

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

Bug 2092567: [Downstream Merge] 16/11/2022 #1381

Merged

Conversation

tssurya
Copy link
Contributor

@tssurya tssurya commented Nov 16, 2022

Tagging commit authors
@ricky-rav for lgtm since you have lots of small fixes coming in
@andreaskaris : PTAL your commits
@JacobTanenbaum @dcbw : Please note that we are not cherry-picking individual commits any more for bug fixes since 4.13 branch has already opened (in context of #1377 ), we are back to bulk merges.
@pperiyasamy: Please check your commit.

CLEAN MERGE, NO CONFLICTS. cc @trozet @dcbw for merge.

andreaskaris and others added 28 commits November 2, 2022 19:27
Improve kubectl_wait_pods:
* use kubectl rollout to wait for specific DaemonSets
* use kubectl wait pods to wait for specific pods
* respect total timeout

Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Apply retry to namespace controller in ovnk node that, upon a namespace update, syncs UDP conntrack entries.

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
…ange

Command used:
$ mockery --name NodeWatchFactory    --dir ~/go/src/github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory/ --output ~/go/src/github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory/mocks/

04 Oct 22 19:09 CEST INF Starting mockery dry-run=false version=v2.14.0
04 Oct 22 19:09 CEST INF Walking dry-run=false version=v2.14.0
04 Oct 22 19:09 CEST INF Generating mock dry-run=false interface=NodeWatchFactory qualified-name=github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory version=v2.14.0

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Instead of exiting with an error, exit gracefully with an info message.

Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
general fixes to the windows node_linux.go testing to make the testing
more meaningful. The tests are not ideal because there is no interface
into fakeCmd to get the bundled ovs commands but we can compare the
cache to what we believe it should be.

I also removed tests that where no longer meaningful

test "removes stale node flows on initial sync" and "removes stale pod
flows on initial sync" are no longer needed because we sync to the
flowCache on every sync and that stale nodes/pods will be taken care
of then.

test "sets up local pod flows" is a duplicate of "sets up local linux
pod" so I removed the former

Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Add the option to deploy IPsec encryption on kind.

Signed-off-by: Andreas Karis <ak.karis@gmail.com>
When changing the hybrid overlay to dynamically allocated IP addresses
there where instanses where the .3 address was blindly returned. Correct
that mistake

Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
ovnkube-trace: Fail gracefully on failed ovn-detrace dependency check
correct the hybrid overlay port MAC address
apply retry logic to ovnk node controllers
This PR reverts the commits 82094e2 and df3c905.
This will pave the way for using 'options:hairpin_snat_ip'
feature of OVN loadbalancers.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Co-authored-by: Andrew Stoycos <astoycos@redhat.com>
This commit adds a new masqueradeIP which can be
used to tell OVN to explicitly SNAT towards that
masqueradeIP for hairpin service traffic.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Co-authored-by: Andrew Stoycos <astoycos@redhat.com>
This commit adds the hairpin SNAT masqueradeIP to
all the load balancers via the 'options:hairpin_snat_ip'
field in OVN load balancers. This will help us in
handling the NP service hairpin traffic correctly.

Example:

_uuid               : c64e4c7b-66f7-4739-a633-06791efe835a
external_ids        : {"k8s.ovn.org/kind"=Service, "k8s.ovn.org/owner"="surya5/hello-world-net-pol"}
health_check        : []
ip_port_mappings    : {}
name                : "Service_surya5/hello-world-net-pol_TCP_cluster"
options             : {event="false", hairpin_snat_ip="169.254.169.5 fd69::5", reject="true", skip_snat="false"}
protocol            : tcp
selection_fields    : []
vips                : {"10.96.1.35:80"="10.244.1.3:8080,10.244.2.3:8080", "[fd00:10:96::a6a6]:80"="[fd00:10:244:2::3]:8080,[fd00:10:244:3::3]:8080"}

NOTE: This might not be needed in the GR LBs, but no harm
in adding it to all, so that we continue to leverage LBGs.

An e2e test is added that demonstrates that the hairpin traffic
will have the fixed masqueradeIP as the srcIP in its response packet.
This commit also consolidates pokeEndpointHostName and pokeEndpointClientIP
into a single function since they were both doing more or less the
same logic.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Co-authored-by: Andrew Stoycos <astoycos@redhat.com>
This PR adds the logic to allow or deny traffic correctly
in case of podA->svc->podA traffic. If allow-from-podA
is specified then the hairpin traffic should be allowed
and if podA is not in the whitelist then we shouldn't be
allowing hairpin traffic to come back in.

Logic is to add an extra match to all existing ingress
ACLs that match on namespace or pod selectors. This will
look like this:

_uuid               : 6f2566ce-9236-4bee-896c-be56a91aba16
action              : allow-related
direction           : to-lport
external_ids        : {Ingress_num="0", ipblock_cidr="false", l4Match=None, namespace=surya5, policy=allow-ingress-to-foo4-from-foo2, policy_type=Ingress}
label               : 0
log                 : false
match               : "((ip4.src == {$a6612447164629805900} || (ip4.src == 169.254.169.5 && ip4.dst == {$a6612447164629805900})) || (ip6.src == {$a6612449363653062322} || (ip6.src == fd69::5 && ip6.dst == {$a6612449363653062322}))) && outport == @a14627396333488653719"
meter               : acl-logging
name                : surya5_allow-ingress-to-foo4-from-foo2_0
options             : {}
priority            : 1001
severity            : []

The match here tries to check if:
1) traffic is from the allowed-list-address-set OR
2) traffic is hairpin traffic destined towards allowed-list-address-set
for both v4 and v6 cases.

second point is what enables service hairpinning to behave correctly
since we know destination will be same as source for hairpin.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Co-authored-by: Andrew Stoycos <astoycos@redhat.com>
Co-authored-by: Nadia Pinaeva <npinaeva@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
OCPBUGS-2868: Ignore addresses in masquerade subnet when retrieving gateway IPs
Don't log in iterateRetryResources when there are no retry entries
When nextQueryTime for dns entry is already expired and update goroutine is trying
to use negative duration to reset ticker. This causes process crash on the ovnk
master. This change avoids proces crash and ensures to trigger ticker in shortest
possible time (1 ms) so that update happens immediately on expired dns entry.

Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
…anic

Handle expired entry while handling dns update
Fix Network Policy Service Hairpin Traffic
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Hold lock when deleting completed pod during update event
@tssurya tssurya changed the title [Downstream Merge] 16/11/2022 Bug 2092567: [Downstream Merge] 16/11/2022 Nov 16, 2022
@openshift-ci openshift-ci bot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Nov 16, 2022
@pperiyasamy
Copy link
Member

/lgtm for 3784254. Thanks @tssurya !

@ricky-rav
Copy link
Contributor

/LGTM
Thank you, Surya!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2022
node: mock conntrack delete operations for service and EgressIP testcases
kind.sh: Add IPsec option to kind deployments
@trozet
Copy link
Contributor

trozet commented Nov 17, 2022

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2022
@tssurya
Copy link
Contributor Author

tssurya commented Nov 18, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2022
@dcbw
Copy link
Contributor

dcbw commented Nov 18, 2022

/retest

@dcbw
Copy link
Contributor

dcbw commented Nov 18, 2022

/lgtm

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

openshift-ci bot commented Nov 18, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, pperiyasamy, ricky-rav, 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

@tssurya
Copy link
Contributor Author

tssurya commented Nov 18, 2022

/retest

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 5e1ed9e and 2 for PR HEAD 039f756 in total

@tssurya
Copy link
Contributor Author

tssurya commented Nov 19, 2022

/retest

@tssurya
Copy link
Contributor Author

tssurya commented Nov 21, 2022

/retest-required

@tssurya
Copy link
Contributor Author

tssurya commented Nov 21, 2022

/retest

@tssurya
Copy link
Contributor Author

tssurya commented Nov 21, 2022

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 21, 2022

@tssurya: The following tests 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-aws-ovn-hypershift 039f756 link false /test e2e-aws-ovn-hypershift
ci/prow/e2e-openstack-ovn 039f756 link false /test e2e-openstack-ovn
ci/prow/e2e-vsphere-ovn 039f756 link false /test e2e-vsphere-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.

@openshift-merge-robot openshift-merge-robot merged commit 83126e4 into openshift:master Nov 21, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 21, 2022

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

Bugzilla bug 2092567 has been moved to the MODIFIED state.

In response to this:

Bug 2092567: [Downstream Merge] 16/11/2022

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.

@vrutkovs
Copy link
Member

/cherry-pick release-4.12

@openshift-cherrypick-robot

@vrutkovs: #1381 failed to apply on top of branch "release-4.12":

Applying: kind: Improve kubectl_wait_pods
Applying: Apply retry to namespace controller in ovnk node
Applying: Apply retry logic to endpointslice controller in pkg/node/node.go
Applying: Apply retry logic to services and endpointslices in gateway.go
Applying: Regenerated /pkg/factory/mocks/NodeWatchFactory.go after interface change
Using index info to reconstruct a base tree...
A	go-controller/pkg/factory/mocks/NodeWatchFactory.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): go-controller/pkg/factory/mocks/NodeWatchFactory.go deleted in HEAD and modified in Regenerated /pkg/factory/mocks/NodeWatchFactory.go after interface change. Version Regenerated /pkg/factory/mocks/NodeWatchFactory.go after interface change of go-controller/pkg/factory/mocks/NodeWatchFactory.go left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0005 Regenerated /pkg/factory/mocks/NodeWatchFactory.go after interface change
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.12

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
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium 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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.