New e2e test suite in golang

[istalker2]

New e2e test suite in golang

• Pure golang
• Framework that allows remote execution of commands and other
  operations on DinD k8s node (i.e. what docker exec does),
  k8s pods (like kubectl exec) and created VMs (like vmssh.sh) without
  executing external apps
• ginkgo test framework drop-in replacement that extends the framework
  with ability to have BeforeAll/AfterAll methods that do setup/teardown
  for current context as a workaround for
  BeforeAll/AfterAll? onsi/ginkgo#70
• e2e suite is compiled into a standalone binary that has no system
  dependencies and can be run from outside of the cluster with
  virtlet deployed
• tests are performed in a temporary namespace that is deleted
  afterwards

Also, noticeably decrease project build time for subsequent builds
by adding -i flag to go build

---

This change is 

[jellonek]

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

---

tests/e2e/framework/common.go, line 53 at r1 (raw file):

	if stderr != "" {
		return "", fmt.Errorf("stderr has errors: %s", stderr)
	}

If stderr output is collected after verifying that exit code is equal to zero - we can loose valuable (for debugging) error output. Looks like reverse order (test for stderr output, then for exit code) can be better.

---

Comments from Reviewable

[ivan4th]

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

---

tests/e2e/basic_test.go, line 76 at r1 (raw file):

		})

		It("Check network interface", func(done Done) {

maybe tests should be renamed a bit to make the output more meaningful? It("...") means "verify that it ..." (does something). So if we're testing that foobar must be able to read a file, we should write Context("foobar", func () { ... It ("should be able to read a file", .... See basic Ginkgo example here for instance: https://onsi.github.io/ginkgo/

---

tests/e2e/framework/common.go, line 65 at r1 (raw file):

}

func waitState(f func() error, wait, poll time.Duration, waitFailure bool) error {

waitForState maybe? or just waitFor ?

---

tests/e2e/framework/common.go, line 93 at r1 (raw file):

}

func waitConsistentState(f func() error, timing ...time.Duration) error {

waitForConsistentState?

---

Comments from Reviewable

[istalker2]

A lot of changes from previous WiP-level PR. All shell e2e tests were migrated

[ivan4th]

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

---

tests/e2e/basic_test.go, line 59 at r2 (raw file):

		scheduleWaitSSH(&vm, &ssh)

		It("Should have default route", func() {

sorry for nitpicking, but perhaps the casing should be consistent (there's "Should" in some places and "should" in others -- with the latter perhaps being more suited for Ginkgo output)

---

tests/e2e/basic_test.go, line 137 at r2 (raw file):

					`cat "/var/log/virtlet/vms/%s/%s" | \
					grep "login as 'cirros' user. default password: 'cubswin:)'. use 'sudo' for root." | \
					wc -l`, sandboxID, filename))).To(Equal("1"))

Wouldn't it be a bit cleaner to just grab the content of the log file and use strings.Count() on it?

---

tests/e2e/basic_test.go, line 168 at r2 (raw file):

		do(framework.ExecSimple(executor, "apt", "update"))
		do(framework.ExecSimple(executor, "apt", "install", "-y", "vncsnapshot"))

I think we should just install this vncsnapshot in Dockerfile.build, no need to do it during the test.

---

tests/e2e/ceph_test.go, line 227 at r2 (raw file):

		// Add rbd pool and volume
		`ceph osd pool create libvirt-pool 8 8`,
		`apt-get update && apt-get install -y qemu-utils 2> /dev/null`,

please add qemu-utils to Dockerfile.build instead of installing it during e2e (yes, it was using apt-get in e2e script before, but that's a hack)

---

tests/e2e/common.go, line 91 at r2 (raw file):

}

func do(value interface{}, extra ...interface{}) interface{} {

It would be nice to add a comment here describing what this func does.

---

tests/e2e/framework/controller.go, line 31 at r2 (raw file):

	"k8s.io/client-go/tools/clientcmd"

	// register standard k28 types

s/k28/k8s/

---

tests/e2e/ginkgo-ext/matchers.go, line 24 at r2 (raw file):

type anythingMatcher struct {
}

empty structs usually are written as type anythingMatcher struct {} (single line)

---

Comments from Reviewable

[istalker2]

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

---

tests/e2e/basic_test.go, line 168 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

I think we should just install this vncsnapshot in Dockerfile.build, no need to do it during the test.

here and above: probably you meant Dockerfile because Dockerfile.build because vncsnapshot need to be present in virtlet pod and not in the build container

---

Comments from Reviewable

[istalker2]

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

---

tests/e2e/ceph_test.go, line 227 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

please add qemu-utils to Dockerfile.build instead of installing it during e2e (yes, it was using apt-get in e2e script before, but that's a hack)

quemu-img is run in ceph container, which is based on ceph/demo image, which is not ours. So adding qemu-utils to it would mean to maintain our own derived docker image for the ceph just for this test. I don't think it worth it

---

Comments from Reviewable

[istalker2]

PTAL

[istalker2]

CI is going to fail until we merge #396 because now e2e relies on vncsnapshot to be present in virtlet pod, but CI uses virtlet image from docker hub rather than build it from this commit. Thus changes to Dockerfile must be merged first

[pigmej]

but CI uses virtlet image from docker hub rather than build it from this commit.

That shouldn't be the case, if it is then it's probably broken because: #386

[ivan4th]

---

Reviewed 3 of 15 files at r1, 7 of 19 files at r2, 13 of 13 files at r3.
Review status: all files reviewed at latest revision, 9 unresolved discussions.

---

tests/e2e/basic_test.go, line 168 at r2 (raw file):

Previously, istalker2 (Stan Lagun) wrote…

here and above: probably you meant Dockerfile because Dockerfile.build because vncsnapshot need to be present in virtlet pod and not in the build container

you're right, thanks.

---

tests/e2e/ceph_test.go, line 227 at r2 (raw file):

Previously, istalker2 (Stan Lagun) wrote…

quemu-img is run in ceph container, which is based on ceph/demo image, which is not ours. So adding qemu-utils to it would mean to maintain our own derived docker image for the ceph just for this test. I don't think it worth it

ok then, sorry for misunderstanding, let's keep it as-is for now

---

Comments from Reviewable

[jellonek]

With minor comment -

---

Reviewed 7 of 19 files at r2, 13 of 13 files at r3.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

tests/e2e/ceph_test.go, line 226 at r3 (raw file):

	commands := []string{
		// Adjust ceph configs
		`echo -e "rbd default features = 1\nrbd default format = 2" >> /etc/ceph/ceph.conf`,

Usage of echo there to add something to file looks strange there. I know that this is simply the same what was in shell script, but later in time we can replace that with call to https://golang.org/pkg/io/ioutil/#WriteFile - for now i'm ok with current version.

---

Comments from Reviewable

[istalker2]

Review status: all files reviewed at latest revision, 8 unresolved discussions, all commit checks successful.

---

tests/e2e/ceph_test.go, line 226 at r3 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Usage of echo there to add something to file looks strange there. I know that this is simply the same what was in shell script, but later in time we can replace that with call to https://golang.org/pkg/io/ioutil/#WriteFile - for now i'm ok with current version.

echo is executed in docker container, not on the host. How WriteFile can help here?

---

Comments from Reviewable

[jellonek]

Review status: all files reviewed at latest revision, 8 unresolved discussions, all commit checks successful.

---

tests/e2e/ceph_test.go, line 226 at r3 (raw file):

Previously, istalker2 (Stan Lagun) wrote…

echo is executed in docker container, not on the host. How WriteFile can help here?

Point taken.

---

Comments from Reviewable