Remove "appropriate/nc" image from integration test #34832
Conversation
The image used is 3rd party, and hasn't been updated in over two years. Replacing with a plain Alpine image, and manually installing the software. Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
I think the correct fix here is to delete this test. This is not the appropriate place to be testing this behaviour. Looking at #32505 (the PR which introduced this test) the only change was to update vendor. This case should be tested in libnetwork. |
|
Doesn't seem to work as well; already had my doubt because I suspect there will be a race due to the time it takes for the package to be installed (didn't want to build an image first) |
|
@dnephin libnetwork does not have the infra to create tests using the moby code, so this was the best way to have coverage for this case. |
|
The fix did not change any moby code (only vendor), so I don't see why the moby code would be required to test this case. |
|
@dnephin does not change anything because the code that handle the container disconnection from a network is already there |
|
My point is that the test should exercise the bug. If the bug was fixed without touching the moby code, then it must be possible to exercise the bug (and the fix) without the moby code. |
|
@dnephin true but the real use case that we want to cover is a container that create flows during itsl lifecycle and once destroyed has to cleanup its status. |
Why can't we create a test that makes the same libnetwork API calls as you'd expect from container create/remove? That should exercise the bug. |
|
@dnephin you still need to create a network namespace to simulate a container and create flow inside it that subsequently are going to be flushed. But at the same time you will have to use the same contract that moby is sharing with libnetwork so fake endpoints etc etc. |
|
If libnetwork can't be tested in isolation I think that suggests a problem with the interface or the design of the library. There's no reason that a library should be tested from a consumer of the library. I still think we should remove this test. |
|
As I said at the beginning, it's not that is impossible there is only a lack of infra at the moment |
|
ping @thaJeztah |
|
Let me close this for now; as I didn't have time to look at fixing this approach, and there's no other infra (yet) to test this in libnetwork directly |
7 participants
The image used is 3rd party, and hasn't been updated
in over two years. Replacing with a plain Alpine image,
and manually installing the software.
relates to #32505 (comment)
ping @fcrisciani @seemethere @vieux
(let's see if this works 😄)