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

IC/ICNI: Remove the need for k8s.ovn.org/external-gw-pod-ips annotation in #3933

Merged
merged 1 commit into from Sep 28, 2023

Conversation

tssurya
Copy link
Member

@tssurya tssurya commented Sep 26, 2023

- What this PR does and why is it needed
Currently in legacy ICNI, we use the k8s.ovn.org/external-gw-pod-ips as a way for communicating the change in an ICNI gateway pod's status (add/delete/update) from the ovnkube-controller (ovnkube-master in nonIC) to ovnkube-node. In IC multi-zone since the ovnkube-controller and ovnkube-node run as the same process, this is not needed. We can remove dependeny on the extra annotation update we do for IC multizone deployments. We flush the conntrack on ovnkube-controller directly.

On nonIC and IC with single zone we still use the annotation approach for flushing.

- Special notes for reviewers
Review the namespace update bits carefully.

- How to verify it
E2Es present for signalling issues. Ran v6 ones locally since CI does not run it. Need to dust off #3512 because some of the v6 ones are failing on the setup parts :/ so TODO to fix that in future, but as of now v4 ones pass and v6 should follow the same logic as far as code is concerned.

- Description for the changelog
Remove the need for k8s.ovn.org/external-gw-pod-ips annotation in IC/ICNI

@coveralls
Copy link

Coverage Status

Changes unknown when pulling fd2435a on tssurya:skeleton into ** on ovn-org:master**.

go-controller/pkg/ovn/egressgw.go Show resolved Hide resolved
go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
@tssurya tssurya changed the title skeleton IC/ICNI: Remove the need for k8s.ovn.org/external-gw-pod-ips annotation in Sep 27, 2023
go-controller/pkg/node/obj_retry_node.go Outdated Show resolved Hide resolved
Comment on lines +95 to +97
if config.OVNKubernetesFeature.EnableInterconnect && oc.zone != types.OvnDefaultZone {
existingGWs.Insert(nsInfo.routingExternalGWs.gws.UnsortedList()...)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this conditionalized to single node zone IC? Isn't this something generic you are fixing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its conditionalized to multizone IC (not single zone).
because for nonIC and single zone IC we want to use the tranditional legacy method of annotation flush. The annotation k8s.ovn.org/external-gw-pod-ips must only contain ips of the nsInfo.routingExternalPodGWs NOT nsInfo.routingExternalGWs (this is already present as part of k8s.ovn.org/routing-external-gws so we don't want to duplicate it.) So TLDR; this condition is to compliment the

} else {
		// flush here since we know we have added an egressgw pod and we also know the full list of existing gatewayIPs

below this block.

go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
go-controller/pkg/ovn/egressgw.go Outdated Show resolved Hide resolved
@tssurya
Copy link
Member Author

tssurya commented Sep 28, 2023

Summarizing 2 Failures:

[Fail] Services when a nodePort service targeting a pod with hostNetwork:false is created when tests are run towards the agnhost echo service [It] queries to the nodePort service shall work for TCP 
/home/runner/work/ovn-kubernetes/ovn-kubernetes/test/e2e/service.go:254

[Fail] Services when a nodePort service targeting a pod with hostNetwork:false is created when tests are run towards the agnhost echo service [It] queries to the nodePort service shall work for TCP 
/home/runner/work/ovn-kubernetes/ovn-kubernetes/test/e2e/service.go:254

I think we broke something in these tests recently, they are flaking way more than they use to.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
@trozet trozet merged commit e79fdd9 into ovn-org:master Sep 28, 2023
28 of 29 checks passed
@openshift-merge-robot
Copy link

Fix included in accepted release 4.15.0-0.nightly-2023-09-29-095642

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants