Switch to prow for e2e testing. Fix intermittent test failures. #122

munnerz · 2017-11-09T18:27:25Z

What this PR does / why we need it:

Attempts to fix requestheader & RBAC related issues with e2e tests

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

TBC

Special notes for your reviewer:

This may not pass due to issues with kubernetes actually starting still

Release note:

NONE

/assign
/cc @wallrj

jetstack-bot · 2017-11-09T18:27:44Z

@munnerz PR needs rebase

munnerz · 2017-11-10T01:34:22Z

/test e2e

wallrj

Thanks @munnerz

The tests are failing, as you say, because minikube start is failing.

And I expect the tests to fail 1.7 anyway, because we're passing exactly the same --requestheader- arguments to Navigator API server as in #116 (comment), where we saw RBAC access control failures such as

Error from server (Forbidden): elasticsearchclusters.navigator.jetstack.io is forbidden: User "system:anonymous" cannot list elasticsearchclusters.navigator.jetstack.io in the namespace "default"

Even for a user with cluster-admin role binding.

Unless kubeadm bootstrapper does something differently.
And if so, can you add a comment or two explaining what's different.

wallrj · 2017-11-10T09:30:01Z

contrib/charts/navigator/templates/rbac.yaml

@@ -87,7 +87,7 @@ items:
    name: "{{ template "fullname" . }}:controller"
  rules:
  - apiGroups: ["navigator.jetstack.io"]
-    resources: ["elasticsearchclusters", "pilots"]
+    resources: ["elasticsearchclusters", "pilots", "elasticsearchclusters/status", "pilots/status"]


❓ Does a controller ever need to modify pilots/status ?

It needs to delegate permission to modify/update pilots/status to each Pilot.

wallrj · 2017-11-10T09:34:41Z

hack/install-e2e-dependencies.sh

-     --extra-config=apiserver.Authorization.Mode=RBAC
+     --extra-config=apiserver.Authorization.Mode=RBAC \
+     --bootstrapper=kubeadm; then
+     sudo journalctl -xu kubelet.service


This is failing on Travis because the VM doesn't have journalctl.
Use minikube logs instead?
Or does kubeadm put the logs somewhere where minikube logs can't find them?

This is still WIP - we can't use the kubeadm bootstrapper with vm-driver=none as kubeadm requires systemctl, but travis don't support anything newer than ubuntu 14.04.

I'm working on getting rid of Travis in favour of minikube-in-go on our own test-infra at the moment.

wallrj · 2017-11-10T09:37:24Z

hack/testdata/values-v1.7.0.yaml

+resources:
+  requests:
+    cpu: 50m
+    memory: 64Mi


helm install says -f, --values valueFiles specify values in a YAML file (can specify multiple) (default []) so perhaps we could put only the apiserver.extraArgs in here?

Yep we could - I'm inclined to keep this nice and explicit though, so it's easy to understand what's being deployed in the e2e tests.

munnerz · 2017-11-10T15:55:56Z

Still TODO:

Get kubernetes/test-infra PR merged: Add support for ProwJob lifecycle hooks kubernetes/test-infra#5443
Update minikube_in_go job to run virsh destroy and clean up minikube data dir in preStop hook
Build Kubernetes 1.7.0 test image (or adjust the existing one to support arbitrary k8s versions)
Rename navigator-e2e context to navigator-e2e-v1.8.0, v1.7.0 etc
Update Prow required contexts to include navigator-e2e and navigator-quick-verify
Resolve SELinux issues
Enable RBAC

munnerz · 2017-11-11T00:32:46Z

RBAC is enabled by default with the kubeadm bootstrapper on both 1.7 and 1.8.

munnerz · 2017-11-11T00:33:11Z

/test e2e

munnerz · 2017-11-11T00:36:10Z

@wallrj this appears to be passing pretty consistently now. The only time I've seen failures is when both the k8s 1.7 and 1.8 tests run in parallel sometimes. I think that's because our build server is maxing out (only 2 cores allocated right now) meaning the kubeadm start timeout is being reached.

We'll be increasing the size of our build infrastructure accordingly. I haven't seen the failures very often however.

I've also made the navigator-e2e-v1-7, navigator-e2e-v1-8 and navigator-quick-verify tests mandatory for merge, however e2e tests will not automatically run for every new commit that is pushed. When you are ready to run e2e tests, comment /test e2e or /test e2e v1.7 etc. - you will need to ensure e2e tests are green before the submit queue will merge your PR.

wallrj

Thanks @munnerz

Looks good. And amazing that you got all this stuff set up in such a short time!

The build logs seem to have disappeared

https://prow.build-infra.jetstack.net/log?job=navigator-e2e-v1-7&id=11 Log not found: no such job found: navigator-e2e-v1-7 (id: 11)
https://prow.build-infra.jetstack.net/log?job=navigator-e2e-v1-8&id=9

The Travis build now seems to be running a bare make command which is a bit pointless.

https://travis-ci.org/jetstack/navigator/builds/300431640?utm_source=github_status&utm_medium=notification

I've left a few additional comments and questions below.

Please address and merge.

wallrj · 2017-11-13T10:01:03Z

Makefile

@@ -58,6 +64,7 @@ verify: .hack_verify go_verify
 DOCKER_BUILD_TARGETS = $(addprefix docker_build_, $(CMDS))
 $(DOCKER_BUILD_TARGETS):
 	$(eval DOCKER_BUILD_CMD := $(subst docker_build_,,$@))
+	eval $$(minikube docker-env --profile $$HOSTNAME --shell sh); \


❓ Is it necessary to specify a profile here? The default is minikube, which should work on a new e2e VM right?

It means that I have to remember to use $HOSTNAME as the profile, when I launch minikube locally.

wallrj · 2017-11-13T10:03:49Z

contrib/charts/navigator/templates/rbac.yaml

@@ -87,7 +87,7 @@ items:
    name: "{{ template "fullname" . }}:controller"
  rules:
  - apiGroups: ["navigator.jetstack.io"]
-    resources: ["elasticsearchclusters", "pilots"]
+    resources: ["elasticsearchclusters", "pilots", "elasticsearchclusters/status", "pilots/status"]


wallrj · 2017-11-13T10:04:30Z

contrib/charts/navigator/values.yaml

+#  - --requestheader-client-ca-file=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt
+#  - --requestheader-username-headers=X-Remote-User
+#  - --requestheader-group-headers=X-Remote-Group
+#  - --requestheader-extra-headers-prefix=X-Remote-Extra  - --proxy-client-cert-file="${CERT_DIR}/client-auth-proxy.crt"


This line needs to be split. But do it in a followup branch if you like.

wallrj · 2017-11-13T10:15:17Z

hack/install-e2e-dependencies.sh

-chmod +x minikube
-sudo mv minikube /usr/local/bin/
-
-docker run -v /usr/local/bin:/hostbin quay.io/jetstack/ubuntu-nsenter cp /nsenter /hostbin/nsenter


❓ I see all this stuff gets installed here: https://github.com/jetstack/test-infra/blob/master/images/minikube-in-go/Dockerfile
Great, that should speed things up and save some network traffic.

How easy is it to get launch a KVM VM from inside a Docker container?

How do you ensure that we're using the correct version of kubectl, if it is installed there but minikube start --kubernetes-version... is specified here?

The KVM VM is launched in a container by passing through the libvirt socket into the container. Anything that works with libvirt should work okay within a container too.

The KUBERNETES_VERSION environment variable is set as part of the minikube-in-go docker image, which helps ensure we use the correct version of kubectl with kubernetes 😄

wallrj · 2017-11-13T10:15:32Z

hack/prepare-e2e.sh

-    apiGroup: rbac.authorization.k8s.io
-  - kind: Group
-    name: system:unauthenticated
-    apiGroup: rbac.authorization.k8s.io


munnerz · 2017-11-13T10:45:20Z

/retest

munnerz · 2017-11-13T10:58:56Z

/approve

jetstack-bot · 2017-11-13T10:59:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: munnerz

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [munnerz]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

jetstack-bot · 2017-11-13T10:59:23Z

/test all [submit-queue is verifying that this PR is safe to merge]

munnerz · 2017-11-13T11:04:12Z

/test all

jetstack-bot · 2017-11-13T11:22:33Z

Automatic merge from submit-queue.

jetstack-bot assigned munnerz Nov 9, 2017

jetstack-bot added the release-note-none label Nov 9, 2017

jetstack-bot requested a review from wallrj November 9, 2017 18:27

jetstack-bot added the size/L label Nov 9, 2017

jetstack-bot added the needs-rebase label Nov 9, 2017

munnerz force-pushed the e2e-patches branch from 27850f0 to 8843df9 Compare November 9, 2017 18:34

jetstack-bot removed the needs-rebase label Nov 9, 2017

munnerz added 4 commits November 9, 2017 18:37

Configure e2e test helm chart with k8s version specific values.yaml

eeeb54e

Allow specifying extraArgs to navigator-apiserver

4eae5b6

Remove global navigator api group clusterrole

d873ea2

Grant navigator-controller access to status subresources

6d89d41

munnerz force-pushed the e2e-patches branch from 8843df9 to 6d89d41 Compare November 9, 2017 18:37

munnerz added 2 commits November 9, 2017 19:08

Use kubeadm bootstrapper

a7a26b9

Print kubelet journal on failure

551087a

wallrj suggested changes Nov 10, 2017

View reviewed changes

munnerz added 3 commits November 10, 2017 12:47

Running navigator e2e on prow

a4ca7a9

Set profile name on minikube start

595389e

Use minikube docker daemon

086f5c4

munnerz force-pushed the e2e-patches branch from 0e631f1 to 086f5c4 Compare November 10, 2017 14:06

munnerz added 4 commits November 10, 2017 14:14

Set minikube profile

942b24f

Hardcode shell name

30d966d

Fix e2e image tag

892b505

Remove travis e2e tests

be7bf4f

munnerz closed this Nov 10, 2017

munnerz reopened this Nov 10, 2017

munnerz added 2 commits November 10, 2017 18:25

Update e2e testdata

00ddc07

Only run travis build on master branch

7c77e2c

munnerz changed the title ~~Fixes for e2e test requestheader failures~~ Switch to prow for e2e testing. Fix intermittent test failures. Nov 11, 2017

Unify testdata values.yaml. Update minikube start comment.

0d6c8aa

wallrj approved these changes Nov 13, 2017

View reviewed changes

munnerz added the lgtm label Nov 13, 2017

jetstack-bot added the approved label Nov 13, 2017

jetstack-bot merged commit 26e8057 into jetstack:master Nov 13, 2017

munnerz mentioned this pull request Nov 13, 2017

RBAC roles and bindings installed by ./hack/prepare-e2e.sh masks errors and missing configuration in the helm RBAC template #110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to prow for e2e testing. Fix intermittent test failures. #122

Switch to prow for e2e testing. Fix intermittent test failures. #122

munnerz commented Nov 9, 2017

jetstack-bot commented Nov 9, 2017

munnerz commented Nov 10, 2017

wallrj left a comment

wallrj Nov 10, 2017

munnerz Nov 10, 2017

wallrj Nov 13, 2017

wallrj Nov 10, 2017

munnerz Nov 10, 2017

wallrj Nov 10, 2017

munnerz Nov 10, 2017

munnerz commented Nov 10, 2017 •

edited

Loading

munnerz commented Nov 11, 2017

munnerz commented Nov 11, 2017

munnerz commented Nov 11, 2017

wallrj left a comment

wallrj Nov 13, 2017

wallrj Nov 13, 2017

wallrj Nov 13, 2017

wallrj Nov 13, 2017

munnerz Nov 13, 2017

wallrj Nov 13, 2017

munnerz commented Nov 13, 2017

munnerz commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

munnerz commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

Switch to prow for e2e testing. Fix intermittent test failures. #122

Switch to prow for e2e testing. Fix intermittent test failures. #122

Conversation

munnerz commented Nov 9, 2017

jetstack-bot commented Nov 9, 2017

munnerz commented Nov 10, 2017

wallrj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

munnerz commented Nov 10, 2017 • edited Loading

munnerz commented Nov 11, 2017

munnerz commented Nov 11, 2017

munnerz commented Nov 11, 2017

wallrj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

munnerz commented Nov 13, 2017

munnerz commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

munnerz commented Nov 13, 2017

jetstack-bot commented Nov 13, 2017

munnerz commented Nov 10, 2017 •

edited

Loading