Smoke test #145

pavolloffay · 2018-11-30T17:13:10Z

Resolves #143 #142

Use port forwad to query and collector. Report a span to collector and query it.

This proves that cassandra tests incorrectly configures c* resulting in error on reads and writes.

review by commits - checked in vendor dir...

jpkrohling · 2018-11-30T17:13:15Z

This change is

codecov · 2018-11-30T17:16:15Z

Codecov Report

Merging #145 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #145   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files          28       28           
  Lines        1267     1267           
=======================================
  Hits         1219     1219           
  Misses         37       37           
  Partials       11       11

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 43bcee3...38ebcd7. Read the comment docs.

jpkrohling

Could you add a call to the SmokeTest from a test that is known to work? This is just to "prove" that the SmokeTest is working.

I'm in favor of merging this PR once the Cassandra setup gets fixed, with a clean make test, otherwise people might wonder why a test is failing that has nothing to do with their PRs :)

Reviewed 139 of 139 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pavolloffay)

pkg/deployment/collector_test.go, line 8 at r1 (raw file):

	"k8s.io/api/core/v1"
	"k8s.io/apimachinery/pkg/api/resource"
	"testing"

Perhaps you are using a different tooling, but we are (attempting?) to place the stdlib imports first, then external, then internal.

test/e2e/port_forward.go, line 20 at r1 (raw file):

	}

	path := fmt.Sprintf("/api/v1/namespaces/%s/pods/%s/portforward", namespace, pod)

Is it possible to use something similar to what we have in the SDK, WaitFor... ? I like your approach, especially the channel part, but I'd rather have a consistent code across all the object types we have to wait for.

pavolloffay · 2018-12-03T14:08:13Z

Could you add a call to the SmokeTest from a test that is known to work? This is just to "prove" that the SmokeTest is working.

The PR fixes that test and also C* example.

pavolloffay

Reviewable status: 135 of 142 files reviewed, 2 unresolved discussions (waiting on @jpkrohling)

pkg/deployment/collector_test.go, line 8 at r1 (raw file):

Previously, jpkrohling (Juraci Paixão Kröhling) wrote…

Perhaps you are using a different tooling, but we are (attempting?) to place the stdlib imports first, then external, then internal.

I was relying on make format. But it seems it does not format everything..

test/e2e/port_forward.go, line 20 at r1 (raw file):

t possible to use something si

I don't know to what part form WaitFor... are you referring to. Could you please propose API which satisfies used interfaces in this method and is similar to wait...?

jpkrohling

Reviewed 4 of 5 files at r2, 3 of 3 files at r3.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pavolloffay)

a discussion (no related file):

The PR fixes that test and also C* example.

Cool, I saw the changes to the Cassandra example. Looks nice!

The test did not pass for me, though:

goroutine 312 [chan receive, 5 minutes]:
github.com/jaegertracing/jaeger-operator/test/e2e.cassandraTest(0xc000257500, 0xc00029ee80, 0xc000454880, 0x0, 0x0)
	/mnt/storage/jpkroehling/Projects/src/github.com/jaegertracing/jaeger-operator/test/e2e/cassandra.go:80 +0x785
github.com/jaegertracing/jaeger-operator/test/e2e.Cassandra(0xc000257500)
	/mnt/storage/jpkroehling/Projects/src/github.com/jaegertracing/jaeger-operator/test/e2e/cassandra.go:20 +0x82
testing.tRunner(0xc000257500, 0x12f2080)
	/mnt/storage/jpkroehling/Tools/go/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
	/mnt/storage/jpkroehling/Tools/go/src/testing/testing.go:878 +0x353

rax    0xca
rbx    0x1e7afa0
rcx    0x45f013
rdx    0x0
rdi    0x1e7b0e0
rsi    0x80
rbp    0x7ffc128921f8
rsp    0x7ffc128921b0
r8     0x0
r9     0x0
r10    0x0
r11    0x286
r12    0x77
r13    0x8
r14    0xc000078f00
r15    0x0
rip    0x45f011
rflags 0x286
cs     0x33
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (10m0s).
FAIL	github.com/jaegertracing/jaeger-operator/test/e2e	600.006s

deploy/examples/with-cassandra.yaml, line 13 at r3 (raw file):

        servers: cassandra
        keyspace: jaeger_v1_datacenter3
    cassandraCreateSchema:

Good catch!

pkg/deployment/collector_test.go, line 8 at r1 (raw file):

Previously, pavolloffay (Pavol Loffay) wrote…

I was relying on make format. But it seems it does not format everything..

Yeah, it uses go fmt behind the scenes, which is quite lenient in this matter.

test/e2e/port_forward.go, line 20 at r1 (raw file):

Previously, pavolloffay (Pavol Loffay) wrote…

t possible to use something si

I don't know to what part form WaitFor... are you referring to. Could you please propose API which satisfies used interfaces in this method and is similar to wait...?

I mean in a generic way. The way this PR is, the caller of GetPortForward will pass a channel which will receive a message once this gets available. For all other Kubernetes objects (jobs, deployments, ...), the test calls a WaitFor... function that waits for this object to be available. I was hoping to have a consistent approach, so that a job would call WaitForPortForward() instead of passing a channel.

pavolloffay · 2018-12-03T16:20:42Z

I mean in a generic way. The way this PR is, the caller of GetPortForward will pass a channel which will receive a message once this gets available.

The channel is not passed. The method returns channel which is used to close. Before using port forward caller has to call -<portForward.Ready we can call it inside the func itself.

jpkrohling

Right, that's what I mean. The way to wait for a PortForward is different than with other objects.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pavolloffay)

pavolloffay · 2018-12-03T16:30:19Z

The test did not pass for me, though:

did you run dep ensure?

@objectiser could you please also try? On my machine it passes fine (I know we like claims like these)

jpkrohling

did you run dep ensure?

Is it needed? If so, then the vendor changes should be included in this PR.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pavolloffay)

pavolloffay · 2018-12-03T16:42:15Z

Is it needed? If so, then the vendor changes should be included in this PR.

The vendor changes are checked in. What else is needed except adding it to Gopkg and including vendor sources?

jpkrohling

Nothing else should be needed. I ran dep ensure and there were no relevant changes, but the test is still timing out.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pavolloffay)

pavolloffay · 2018-12-04T10:02:16Z

indeed there was an issue with the test. Also note that it's using port forwarding on standard jaeger ports so make sure those ports are free: 16686, 14268.

k8s client 1.13.0 will allow us to use random ports. (It allows now but there is no getter on the forwarded port). I could not update that dep now.

Signed-off-by: Pavol Loffay <[email protected]>

jpkrohling

Reviewed 6 of 6 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):
It works now :-)

$ make test
Running unit tests...
?   	github.com/jaegertracing/jaeger-operator/cmd	[no test files]
?   	github.com/jaegertracing/jaeger-operator/cmd/manager	[no test files]
ok  	github.com/jaegertracing/jaeger-operator/pkg/account	0.060s	coverage: 100.0% of statements
?   	github.com/jaegertracing/jaeger-operator/pkg/apis	[no test files]
ok  	github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1	0.059s	coverage: 18.8% of statements
?   	github.com/jaegertracing/jaeger-operator/pkg/cmd/start	[no test files]
?   	github.com/jaegertracing/jaeger-operator/pkg/cmd/version	[no test files]
ok  	github.com/jaegertracing/jaeger-operator/pkg/config/sampling	0.069s	coverage: 94.1% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/config/ui	0.031s	coverage: 94.1% of statements
?   	github.com/jaegertracing/jaeger-operator/pkg/controller	[no test files]
?   	github.com/jaegertracing/jaeger-operator/pkg/controller/deployment	[no test files]
ok  	github.com/jaegertracing/jaeger-operator/pkg/controller/jaeger	3.027s	coverage: 64.3% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/deployment	0.040s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/ingress	0.020s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/inject	0.060s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/route	0.032s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/service	0.008s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/storage	0.010s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/strategy	0.008s	coverage: 100.0% of statements
ok  	github.com/jaegertracing/jaeger-operator/pkg/util	0.007s	coverage: 100.0% of statements
?   	github.com/jaegertracing/jaeger-operator/pkg/version	[no test files]
Formatting code...
Building...
Sending build context to Docker daemon  80.22MB
Step 1/4 : FROM alpine:3.8
 ---> 196d12cf6ab1
Step 2/4 : USER nobody
 ---> Using cache
 ---> 18305c5b670d
Step 3/4 : ADD build/_output/bin/jaeger-operator /usr/local/bin/jaeger-operator
 ---> 041345996495
Step 4/4 : ENTRYPOINT ["/usr/local/bin/jaeger-operator"]
 ---> Running in 1cfd525d9bd9
Removing intermediate container 1cfd525d9bd9
 ---> de34fadab6de
Successfully built de34fadab6de
Successfully tagged jpkroehling/jaeger-operator:latest
Pushing image jpkroehling/jaeger-operator:latest...
Running end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	232.211s

test/e2e/cassandra.go, line 69 at r4 (raw file):

	}

	jaegerPod, err := GetPod(namespace, "with-cassandra", "jaegertracing/all-in-one", f.KubeClient)

Isn't the image in the tests using the user's namespace, instead of jaegertracing?

edit: apparently not, the tests are passing :-)

jpkrohling reviewed Dec 3, 2018

View reviewed changes

pavolloffay force-pushed the smoke-test branch from 61df808 to 5b0215e Compare December 3, 2018 14:05

pavolloffay changed the title ~~WIP: Smoke test~~ Smoke test Dec 3, 2018

pavolloffay commented Dec 3, 2018

View reviewed changes

jpkrohling reviewed Dec 3, 2018

View reviewed changes

pavolloffay added 7 commits December 4, 2018 11:02

Add vendor changes for port-forward

cbf52d8

Signed-off-by: Pavol Loffay <[email protected]>

Smoke tests

d1c7dd1

Signed-off-by: Pavol Loffay <[email protected]>

Add modeke test to elastic

6eff5d1

Signed-off-by: Pavol Loffay <[email protected]>

Fix cassandra example

8d2566a

Signed-off-by: Pavol Loffay <[email protected]>

Remove print

bb6719b

Signed-off-by: Pavol Loffay <[email protected]>

Manually spring annoying imports

6407d27

Signed-off-by: Pavol Loffay <[email protected]>

Fix issue with e2e tests

38ebcd7

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay force-pushed the smoke-test branch from 1a835f5 to 38ebcd7 Compare December 4, 2018 10:02

pavolloffay mentioned this pull request Dec 4, 2018

Use random ports in port forward and smoketests #154

Closed

jpkrohling approved these changes Dec 4, 2018

View reviewed changes

jpkrohling merged commit 893144d into jaegertracing:master Dec 4, 2018

pavolloffay mentioned this pull request Dec 4, 2018

with-cassandra example is broken #142

Closed

jpkrohling mentioned this pull request Dec 5, 2018

Fix import order for collector-test #158

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smoke test #145

Smoke test #145

pavolloffay commented Nov 30, 2018 •

edited

Loading

jpkrohling commented Nov 30, 2018

codecov bot commented Nov 30, 2018 •

edited

Loading

jpkrohling left a comment

pavolloffay commented Dec 3, 2018

pavolloffay left a comment

jpkrohling left a comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

pavolloffay commented Dec 4, 2018

jpkrohling left a comment

Smoke test #145

Smoke test #145

Conversation

pavolloffay commented Nov 30, 2018 • edited Loading

jpkrohling commented Nov 30, 2018

codecov bot commented Nov 30, 2018 • edited Loading

Codecov Report

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 3, 2018

pavolloffay left a comment

Choose a reason for hiding this comment

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 3, 2018

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 4, 2018

jpkrohling left a comment

Choose a reason for hiding this comment

pavolloffay commented Nov 30, 2018 •

edited

Loading

codecov bot commented Nov 30, 2018 •

edited

Loading