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

e2e tests for virtlet pod restart #641

Merged
merged 4 commits into from
Apr 4, 2018
Merged

e2e tests for virtlet pod restart #641

merged 4 commits into from
Apr 4, 2018

Conversation

lukaszo
Copy link
Contributor

@lukaszo lukaszo commented Mar 28, 2018

It's build on top of #480

This change is Reviewable

@lukaszo lukaszo force-pushed the virtlet_restart branch 2 times, most recently from 97794f8 to 2932867 Compare March 28, 2018 23:11
@jellonek
Copy link
Contributor

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks failed.


tests/e2e/restart_virtlet_test.go, line 38 at r2 (raw file):

	BeforeAll(func() {
		includeDisruptive()

Such thing in BeforeAll() will not work. It needs to be added to test case (to It()).
That's why now in test logs you have first case skipped, but second executed.


Comments from Reviewable

@lukaszo
Copy link
Contributor Author

lukaszo commented Mar 29, 2018

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


tests/e2e/restart_virtlet_test.go, line 38 at r2 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Such thing in BeforeAll() will not work. It needs to be added to test case (to It()).
That's why now in test logs you have first case skipped, but second executed.

Ah, thx. I was searching for it in a docs


Comments from Reviewable

@jellonek
Copy link
Contributor

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


tests/e2e/restart_virtlet_test.go, line 38 at r2 (raw file):

Previously, lukaszo (Łukasz Oleś) wrote…

Ah, thx. I was searching for it in a docs

https://godoc.org/github.com/Mirantis/virtlet/tests/e2e/ginkgo-ext ;)


Comments from Reviewable

@lukaszo lukaszo force-pushed the virtlet_restart branch 2 times, most recently from 20871bd to 5e7d776 Compare March 29, 2018 14:38
@ivan4th
Copy link
Contributor

ivan4th commented Mar 29, 2018

Reviewed 1 of 2 files at r3.
Review status: 1 of 4 files reviewed at latest revision, 1 unresolved discussion.


deploy/virtlet-ds.yaml, line 196 at r3 (raw file):

            - /bin/sh
            - -c
            - socat - UNIX:/run/virtlet.sock </dev/null

Maybe we should add a similar probe for the libvirt container?


tests/e2e/restart_virtlet_test.go, line 2 at r3 (raw file):

/*
Copyright 2017 Mirantis

2018


tests/e2e/restart_virtlet_test.go, line 69 at r3 (raw file):

	}, 3*60)

	It("Should contain logs from attach session", func() {

"Virtlet restart Should contain logs from another session" doesn't sound good, maybe "Should keep logs from another session"?


Comments from Reviewable

@lukaszo
Copy link
Contributor Author

lukaszo commented Mar 30, 2018

Review status: 1 of 4 files reviewed at latest revision, 4 unresolved discussions.


deploy/virtlet-ds.yaml, line 196 at r3 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Maybe we should add a similar probe for the libvirt container?

Done.


tests/e2e/restart_virtlet_test.go, line 2 at r3 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

2018

Done.


tests/e2e/restart_virtlet_test.go, line 69 at r3 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

"Virtlet restart Should contain logs from another session" doesn't sound good, maybe "Should keep logs from another session"?

Done.


Comments from Reviewable

There is a strange bug on CircleCi which causes docker to crash when
virtler pod is restarted
@ivan4th
Copy link
Contributor

ivan4th commented Apr 4, 2018

Reviewed 5 of 5 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Apr 4, 2018

:lgtm:


Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@jellonek
Copy link
Contributor

jellonek commented Apr 4, 2018

:lgtm:


Reviewed 5 of 5 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@ivan4th ivan4th merged commit 8a13a29 into master Apr 4, 2018
@ivan4th ivan4th deleted the virtlet_restart branch April 4, 2018 13:20
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.

3 participants