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

Improve functional tests #816

Merged
merged 1 commit into from
Oct 13, 2017
Merged

Improve functional tests #816

merged 1 commit into from
Oct 13, 2017

Conversation

tomdee
Copy link
Contributor

@tomdee tomdee commented Sep 18, 2017

  • Move to using bash_unit
  • Test pings in both directions
  • Test kubernetes subnet manager
  • Test multiple versions of etcd and k8s during a release

Makefile Outdated
release-tests: bash_unit
# Run the functional tests with different etcd versions. Currently these are the latest point releases.
# This list should be updated during the release process.
ETCD_IMG="quay.io/coreos/etcd:v3.2.7" ./bash_unit dist/functional-test.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

what if we do a latest tag here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea

}

test_vxlan() {
start_flannel vxlan && sleep 2
Copy link
Contributor

Choose a reason for hiding this comment

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

how about instead of all these sleep 2, we put the pings in a loop so it retries for 5-10 times to see if pings work, so the tests don't become flakey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the sleep is actually there to make sure the subnet.env file is written. I'll update the PR to get rid of the sleeps


pings() {
# ping in both directions
assert "docker exec -it --privileged flannel-e2e-test-flannel1 /bin/ping -c 5 $ping_dest2" "Host 1 cannot ping host 2"
Copy link
Contributor

Choose a reason for hiding this comment

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

or we could put this in some sort of retry block, maybe


test_manifest() {
# This just tests that the API server accepts the manifest, not that it actually acts on it correctly.
assert "cat ../Documentation/kube-flannel.yml | docker run -i --rm --net=host gcr.io/google_containers/hyperkube-amd64:v$K8S_VERSION /hyperkube kubectl create -f -"
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, there's a cool project garethr/kubeval that can validate the k8s manifests. Not suggesting we should or shouldn't use it, but just throwing it out there

# sleep 10000
#fi
test_vxlan_ping() {
write_config_etcd vxlan && sleep 1
Copy link
Contributor

Choose a reason for hiding this comment

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

curious, why is this sleep 1 vs 2 in the other test?

Copy link
Contributor

Choose a reason for hiding this comment

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

i'd still advocate for some sort of ping retry instead of sleep

Copy link
Contributor

@gunjan5 gunjan5 left a comment

Choose a reason for hiding this comment

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

looks pretty good. I just have a few comments/suggestions, nothing critical or blocking the merge tho

- Move to using bash_unit
- Test pings in both directions
- Test kubernetes subnet manager
- Test multiple versions of etcd and k8s during a release
@tomdee tomdee merged commit 5d22389 into flannel-io:master Oct 13, 2017
@tomdee tomdee deleted the improve-tests branch October 13, 2017 22:34
alvaroaleman pushed a commit to alvaroaleman/flannel that referenced this pull request Oct 28, 2017
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

2 participants