Skip to content

Conversation

@ZetaoZhuang
Copy link
Contributor

Reason for Change:

cni should not invoke cns to release ips for muiltitenancy

Issue Fixed:

Requirements:

Notes:

@ZetaoZhuang ZetaoZhuang requested a review from a team as a code owner November 6, 2023 22:17
@ZetaoZhuang ZetaoZhuang marked this pull request as draft November 6, 2023 22:24
@ZetaoZhuang ZetaoZhuang marked this pull request as ready for review November 7, 2023 00:00
if err != nil {
plugin.cleanupAllocationOnError(ipamAddResult.defaultInterfaceInfo.ipResult, nwCfg, args, options)
// for multi-tenancies scenario, CNI is not supposed to invoke CNS for cleaning Ips
if !(nwCfg.MultiTenancy && nwCfg.IPAM.Type == network.AzureCNS) {
Copy link
Member

Choose a reason for hiding this comment

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

can we add UT for this ?

@rbtr rbtr added release/1.4 Change affects v1.4 release train release/latest Change affects latest release train needs-backport Change needs to be backported to previous release trains labels Nov 7, 2023
@rbtr rbtr force-pushed the fix_cni_ip_cleanup branch from 5289f1f to 1c4259c Compare November 8, 2023 00:26
@rbtr rbtr force-pushed the fix_cni_ip_cleanup branch from 1c4259c to 7491932 Compare November 8, 2023 15:51
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

@ZetaoZhuang can you add UT in separate PR?

@rbtr rbtr enabled auto-merge (squash) November 8, 2023 19:58
@rbtr rbtr merged commit 3e5526e into master Nov 8, 2023
@rbtr rbtr deleted the fix_cni_ip_cleanup branch November 8, 2023 19:59
@ZetaoZhuang
Copy link
Contributor Author

@ZetaoZhuang can you add UT in separate PR?

Sure. I have those already in my local branch

@ZetaoZhuang ZetaoZhuang mentioned this pull request Nov 8, 2023
4 tasks
matmerr pushed a commit that referenced this pull request Jan 17, 2024
cni should not invoke cns to release ips for muiltitenancy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-backport Change needs to be backported to previous release trains release/latest Change affects latest release train release/1.4 Change affects v1.4 release train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants