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

K3s Integration test fixes #4341

Merged
merged 6 commits into from
Oct 28, 2021
Merged

Conversation

dereknola
Copy link
Contributor

Proposed Changes

  • Increase timeout on all integration tests to 3 minutes for server startup time
  • Moved each test under tests/integration into their own subfolder, due to a limitation on how Ginkgo/Gomega can only support one BeforeSuite and AfterSuite per package.
  • Fixed Dualstack test as it was improperly formatted

Types of Changes

Test fixes

Verification

go test ./pkg/... ./tests/integration/... -run Integration Now passes 100%

Linked Issues

#4339

User-Facing Change


Further Comments

Signed-off-by: dereknola <derek.nola@suse.com>
Signed-off-by: dereknola <derek.nola@suse.com>
Signed-off-by: dereknola <derek.nola@suse.com>
Signed-off-by: dereknola <derek.nola@suse.com>
Signed-off-by: dereknola <derek.nola@suse.com>
@dereknola dereknola requested a review from a team as a code owner October 27, 2021 17:30
Comment on lines +16 to +17
"--cluster-cidr 10.42.0.0/16,2001:cafe:42:0::/56",
"--service-cidr 10.43.0.0/16,2001:cafe:42:1::/112",
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this actually work? I thought the node needed to also have an ipv6 --node-ip address assigned in order for dual-stack to work properly.

Copy link
Contributor Author

@dereknola dereknola Oct 27, 2021

Choose a reason for hiding this comment

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

So the test passes, but this is a case of "I didn't write it, I just fixed it". I can sit down with @manuelbuil and improve the test to actually insure that it properly checks dual-stack is working in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and no. Klipper-lb and the upcoming networkPolicy controller require the ipv6 address of the node to be part of --nod-ip. However, flannel is checking the ipv6 address by itself

Flannel requires the main interface to have an ipv6 address to create the vxlan tunnel: https://github.com/k3s-io/k3s/blob/master/pkg/agent/flannel/flannel.go#L107, if the test works, then k3s is being deployed somewhere with an interface having an ipv6 address

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it actually test that the cluster comes up and IPv6 stuff works, or just that the process starts with these args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it just tests that the server doesn't crash with these arguments. This is a barebones integration test at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are also executing this:

return testutil.K3sCmd("kubectl", "exec", podname, "-n", "kube-system", "--", "ip", "a")
			}, "5s", "1s").Should(ContainSubstring("2001:cafe:42:"))

which checks that the pods get an ipv6 address in the range of what was defined in --cluster-cidr, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just verified that flannel/k3s is happy with whatever ipv6 address, even if it is a link-local address:
Oct 28 07:03:15 flannel-master k3s[6902]: I1028 07:03:15.817347 6902 flannel.go:112] Using ipv6 address fe80::5054:1ff:fe06:15fa
So the tests are possibly coming up with an ipv6 link-local address

Signed-off-by: dereknola <derek.nola@suse.com>
Copy link
Contributor

@manuelbuil manuelbuil left a comment

Choose a reason for hiding this comment

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

Thanks Derek!

@dereknola dereknola merged commit 7c3f21e into k3s-io:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants