projectcalico / calico

Cloud native networking and network security
https://docs.tigera.io/calico/latest/about/
Apache License 2.0
6.04k stars 1.35k forks source link

Fix: announcement of /32 and /128 entries for serviceExternalIPs #9422

Closed tanujd11 closed 1 day ago

tanujd11 commented 3 weeks ago

Description

Similar to https://github.com/projectcalico/calico/pull/6282 but for externalIP, I kind of ran into this and it would be great if externalIPs populated as single IP addresses stop advertising itself as global routes and only expose itself for services with externalTrafficPolicy as Cluster.

Related issues/PRs

Fixes https://github.com/projectcalico/calico/issues/7558

Todos

Release Note

Fix that single-IP entries on BGPConfiguration serviceExternalIPs were not advertised according to external traffic policy. 

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

Every PR needs one release-note-* label.

Other optional labels:

CLAassistant commented 3 weeks ago

CLA assistant check
All committers have signed the CLA.

caseydavenport commented 3 weeks ago

@tanujd11 thanks for the PR - before we can do anything you'll need to sign the CLA in order to make the bot happy :pray:

tanujd11 commented 3 weeks ago

Done @caseydavenport.

caseydavenport commented 2 weeks ago

/sem-approve

caseydavenport commented 2 weeks ago

@tanujd11 thanks for the PR! This looks pretty good to me - I left one comment, and I also think we should add unit tests for this scenario much like the original PR for LBs did.

caseydavenport commented 6 days ago

/sem-approve

marvin-tigera commented 6 days ago

Removing "merge-when-ready" label due to new commits

caseydavenport commented 2 days ago

/sem-approve