Skip to content

Fix HNS policylist error "network not found" during network removal#43502

Merged
thaJeztah merged 3 commits intomoby:masterfrom
olljanat:fix-hns-policylist-error
Apr 20, 2022
Merged

Fix HNS policylist error "network not found" during network removal#43502
thaJeztah merged 3 commits intomoby:masterfrom
olljanat:fix-hns-policylist-error

Conversation

@olljanat
Copy link
Contributor

@olljanat olljanat commented Apr 19, 2022

Fixes #41354 (moby/libnetwork#2620 (comment))

- What I did
Copied commits from moby/libnetwork#2620 as this looks to be missing from here but exists on Mirantis Container Engine (Mirantis/libnetwork#4) afaiu.

- How I did it
git cherry-pick ...

- How to verify it
Hopefully passes CI.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Trapier Marshall added 3 commits April 19, 2022 14:21
Signed-off-by: Trapier Marshall <tmarshall@mirantis.com>
Removal of PolicyLists from Windows VFP must be performed prior to
removing the HNS network. Otherwise PolicyList removal fails with
HNS error "network not found".

Signed-off-by: Trapier Marshall <tmarshall@mirantis.com>
Make the call to cleanupServiceBindings during network deletion
conditional on Windows (where it is required), thereby providing a
performance improvement to network cleanup on Linux.

Signed-off-by: Trapier Marshall <tmarshall@mirantis.com>
Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Seems OK to me, but we probably need an answer to moby/libnetwork#2620 (comment) one way or another.

@olljanat
Copy link
Contributor Author

Assuming that commit messages are correct where that line was added moby/libnetwork@b6b8023 and here then that condition just works as performance improvement for Linux.

@thaJeztah
Copy link
Member

Assuming that commit messages are correct where that line was added moby/libnetwork@b6b8023 and here then that condition just works as performance improvement for Linux.

Thanks; yes don't think that is a blocker for this PR; perhaps we should create a tracking issue to look into cleaning it up a bit. It's a bit confusing (and not a huge fan of) "wishy-washy" "here we call it", and "here we don't". If the code is not needed on Linux, the function itself should ideally take care of that, or we should differentiate some code further to be platform specific.

Anyway; not a blocker (and not your fault 😅)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hybrid-Swarm: Services in Stack can only communicate from Linux to Windows but not vice-versa

3 participants