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

Integration test cleanup #1938

Merged
merged 3 commits into from
Sep 12, 2017
Merged

Conversation

r2d4
Copy link
Contributor

@r2d4 r2d4 commented Sep 9, 2017

A few changes to the integration tests needed for #1903

  • Persistence Test: Don't start up busybox pod in random namespace, since there won't be sufficient RBAC permissions created.

  • Use a randomly generated name for busybox. If the TestFunctional/DNS ends too closely with TestPersistence, it could try to create a busybox pod, while the one from the DNS test hasn't deleted yet. Using a randomly generate name prevents this, but we have to query the name after we create it in TestFunctional/DNS so that we can exec into it afterwards.

  • Move the kubernetes WaitFor... utils to pkg/util and out of the integration tests. I want to use some of them in Kubeadm bootstrapper #1903 and they might be generally useful outside of the integration tests. One of these functions also had to change, since the kubernetes library it used was printing things to stdout.

  • Split up minikube start args from root args. We need to pass certain flags to all commands in the integration tests (verbosity, logtostderr, bootstrapper) which we weren't currently before. Now, all commands will append -minikube-args to the command, while start will additionally use minikube-start-args.

  • Add the NewMinikubeRunner helper function to set the flags on the struct correctly. This removes a lot of code.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 9, 2017
@codecov-io
Copy link

codecov-io commented Sep 9, 2017

Codecov Report

Merging #1938 into master will decrease coverage by 0.9%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1938      +/-   ##
==========================================
- Coverage   31.35%   30.45%   -0.91%     
==========================================
  Files          75       76       +1     
  Lines        4557     4692     +135     
==========================================
  Hits         1429     1429              
- Misses       2949     3084     +135     
  Partials      179      179
Impacted Files Coverage Δ
pkg/util/kubernetes.go 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6049c1...9747484. Read the comment docs.

If we choose a random namespace, the test will fail since RBAC
permissions will not be set up correctly.

This also chooses a randomly generated name, so that if we are running
an integration test while another busybox pod is still cleaning up,
there are no errors
* Separate start args from args passed to every command.  This is so
that we can call `minikube logs` and `minikube status` with the proper
flags (for the bootstrapper)

* Add a NewMinikubeRunner function to make getting a minikube runner
easier.
@r2d4
Copy link
Contributor Author

r2d4 commented Sep 11, 2017

@aaron-prindle @dlorenc PTAL when you get a chance - this is a blocker for #1903 and #1904 (because of RBAC)

@r2d4 r2d4 merged commit 3180e2e into kubernetes:master Sep 12, 2017
@r2d4 r2d4 deleted the integration-test-cleanup- branch September 12, 2017 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants