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

Fix/remove broken integration-cli IPv6 tests #47788

Merged
merged 3 commits into from May 10, 2024

Conversation

robmry
Copy link
Contributor

@robmry robmry commented May 1, 2024

- What I did

Found two integration-cli tests that didn't run, because the "requires IPv6" test they used was wrong (it'd only return true on a host with no IPv6), and sorted that out.

- How I did it

Fixed one test, deleted the other, deleted associated/broken helper functions - see the individual commit messages.

- How to verify it

It's tests.

- Description for the changelog

robmry added 3 commits May 1, 2024 19:09
The test hadn't been running, because it used testRequires(c, IPv6)
and predicate "IPv6" returns the opposite of the expected result.

If the test had run, it'd have failed because:
- it used "--listen-add", but the option is "--listen-addr"
  - so, the daemon wouldn't have started
- it tried to use "--join ::1"
  - address "::1" was interpreted as host:port so the Dial() failed,
    it needed to be "[::1]".
  - it didn't supply a  join token

Signed-off-by: Rob Murray <rob.murray@docker.com>
The test hadn't been running, because it used testRequires(c, IPv6)
and predicate "IPv6" returns the opposite of the expected result.

TestDaemonIPv6Enabled tried to run with IPv6 on the default bridge,
but didn't set up a "fixed-cidr-v6" - so the daemon wouldn't start.

It then tried to check the bridge had address "fe80::1", which it
expected to work because it had just used setupV6() to add that
address.

Then it  checked that "LinkLocalIPv6Address" was set in container
inspect output, but it wouldn't be (the field is deprecated).

There are working IPv6 tests in the suite (TestDaemonIPv6FixedCIDR,
TestDaemonIPv6FixedCIDRAndMac, TestDaemonIPv6HostMode) - and there's
more coverage in the network integration tests.

So, deleted the test as it didn't seem worth salvaging.

Also deleted now-unused helper functions setupV6(), teardownV6().

Signed-off-by: Rob Murray <rob.murray@docker.com>
It'd only return true on a host with no IPv6 in its kernel.

So, removed, having fixed the two tests that used it.

Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry self-assigned this May 1, 2024
@robmry robmry added this to the 27.0.0 milestone May 1, 2024
@robmry robmry marked this pull request as ready for review May 2, 2024 08:12
@robmry robmry requested review from corhere and akerouanton May 2, 2024 08:12
Copy link
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

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

LGTM

@akerouanton akerouanton merged commit a9ded90 into moby:master May 10, 2024
144 checks passed
@robmry robmry deleted the bad_integration-cli_ipv6_tests branch May 15, 2024 09:14
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.

None yet

3 participants