Add e2e package #35

errordeveloper · 2017-12-18T16:50:32Z

This package contains bare minimum configuration required to run e2e tests.

As seen in linuxkit/linuxkit#2668 earlier.

It still needs a little work (e.g. Job config would be plausible to add), but I wanted to open a PR before this gets lost.

GordonTheTurtle · 2017-12-18T16:50:33Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" [email protected]:errordeveloper/linuxkit-kubernetes.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

ijc · 2017-12-18T16:56:39Z

pkg/e2e/Makefile

+kube_release_artefacts=https://dl.k8s.io/$(kubernetes_version)/bin/linux/amd64
+
+.PHONY: build
+build: _cache/kubernetes-test.tar.gz _cache/kubectl


I'd really like to avoid adding any more Makefile wrappers for package builds at this point.

Are these things large enough to be worth caching or can we just get away with fetching them in the Dockerfile.

If they are big and we think caching is worth while then we should look into enhancing the linuxkit tool to express this in the build.yml, perhaps with a new subsection of depends alongside the docker image cache one I added to get rid of the last Makefiles here? If we are going down this path then probably it would be best to have a design in an issue on the linuxkit/linuxkit repo first.

Note that my preference would be to simply avoid the cache (i.e. just fetch in the Dockerfile) unless the artefacts are really so large as to be problematic.

ijc · 2017-12-18T16:57:34Z

pkg/e2e/build.yml

+org: linuxkit
+image: kube-e2e-test
+network: true
+disable-content-trust: true


Lets setup trust for this repo before we merge this PR. I think everything in the linuxkit org really ought to be signed these days.

ijc · 2017-12-18T17:00:35Z

I think you'll need to add a new job to the .circleci/config.yml, hopefully the pattern will be obvious, ask if not. (another reason to avoid Makefile wrappers is it avoids special cases here...)

errordeveloper · 2017-12-18T20:19:35Z

Ian, thanks for looking at this! I do not like Makefile wrapper also, just checked it in as I had it already. I thought of compiling it, like we do for kubelet package, but then sharing the build logic and caching artefacts comes to mind... If we do lean towards fetching binaries rather then building, I am happy to think of a design for that, but personally I feel like we will build Kubernetes code anyway, so we should just tackle that in a way that allows us to reuse artefacts for different images. I certainly would like to be able to use a local checkout of k/k, or a use fork URL. I am happy to copy build logic from kubelet for now, let me know what you think. By the way, are we looking to eventually use buildkit for building things from source? If so, how long until we might be able to use it here?

…

On Mon, 18 Dec 2017, 5:00 pm Ian Campbell, ***@***.***> wrote: I think you'll need to add a new job to the .circleci/config.yml, hopefully the pattern will be obvious, ask if not. (another reason to avoid Makefile wrappers is it avoids special cases here...) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPWS3aaKgZ1-Of3elF0kGhk9G6P5E_pks5tBpo0gaJpZM4RFwq0> .

ijc · 2017-12-19T10:06:14Z

It should be a relatively simply change to this PR to move the curl invocations into the Dockerfile nowm then we can get this merged (assuming the downloads are 10s to asmall hundred MB should be tolerable given we ought to be building this package reasonably infrequently) and consider whether we want to build from scratch or not (kubelet went through a similar trajectory with getting binaries first and switching to building later on).

WRT building, what is the source repo for this stuff?

ijc · 2017-12-19T10:22:22Z

I missed replying to your final paragraph, #2587 has a bullet point for using buildkit but AFAIK noone is actively working on that so there is no timeline for when it might become possible.

Remind the reader about the other place to update. Structured as a list because I'm anticipating linuxkit#35 adding another. Make the variable name consistent too by updating scripts/mk-image-cache-lst. Signed-off-by: Ian Campbell <[email protected]>

errordeveloper · 2017-12-19T11:40:42Z

WRT building, what is the source repo for this stuff?

kubernetes/kubernetes

Remind the reader about the other place to update. Structured as a list because I'm anticipating linuxkit#35 adding another. Make the variable name consistent too by updating scripts/mk-image-cache-lst. Signed-off-by: Ian Campbell <[email protected]>

errordeveloper · 2017-12-19T17:45:25Z

This now builds all binaries that are needed from source.

ijc · 2018-01-04T16:10:46Z

pkg/e2e/.gitignore

@@ -0,0 +1 @@
+_cache


I don't think this exists in the current version so this whole file is redundant.

ijc · 2018-01-04T16:11:30Z

pkg/e2e/Dockerfile

+RUN mkdir -p /out/etc/apk && cp -r /etc/apk/* /out/etc/apk/
+#coreutils needed for du -B for disk image checks made by kubelet
+# example: $ du -s -B 1 /var/lib/kubelet/pods/...
+#          du: unrecognized option: B


Is this relevant in the context of building the tests?

ijc · 2018-01-04T16:12:16Z

pkg/e2e/README.md

+```
+
+This will result in `linuxkit/kube-e2e-test:current` image that
+you can use. See `e2e.sh` for supported environment variables.


The section is out of date I think.

errordeveloper · 2018-01-04T21:25:00Z

@ijc I've updated this, and included YAML in the top-level directory, as I didn't know where it maybe be better to put it. Inside the package it would affect the hash and that doesn't seem right.
I've also cleaned-up a few things, please take a look and let me know what you think.

Running the e2e tests in CI would be a long job (I think it took 40min on my laptop last time I tried it), what do you think?
I certainly should add package build job to the CI, as discussed earlier.

ijc · 2018-01-05T11:03:04Z

About to take a look, but came here to ask you to rebase onto #36 and to update the two comments I added and add a similar comment to the place you've added another Kube version definition.

Running the e2e tests in CI would be a long job (I think it took 40min on my laptop last time I tried it), what do you think?
I certainly should add package build job to the CI, as discussed earlier.

40 mins is certainly too long, the current set of tests takes ~20 mins in the worst case (all packages needing to be rebuilt) and can be much faster if packages don't need building.

We could consider adding a daily/nightly job which ran on master and ran these tests, but lets defer that until these basics are in.

ijc · 2018-01-05T11:12:17Z

included YAML in the top-level directory, as I didn't know where it maybe be better to put it. Inside the package it would affect the hash and that doesn't seem right.

Do you mean test/kube-e2e-test-job.yaml? Since it includes linuxkit/kube-e2e-test:260c4467ab0a752b5369e73d4f2da13eb2da7bbf I think it can't go in pkg/e2e-test even if you wanted to (since updating the hash would change the hash, rinse and repeat).

Actually that does highlight that this is not actually a "linuxkit package" (i.e. something you would feed to linuxkit build via a yml), it's an image intended to be run as a pod. It's not too bad to reuse linuxkit pkg to rebuild such a thing but perhaps it should be in test/pkg rather than pkg and then having test/foo-job.yaml which consumes it seems entirely reasonable.

Wherever it ends up you should include it in the update-hashes rune in the top-level Makefile.

What about also adding yml/e2e-test.yml similar to yml/weave.yml to pull in kube-e2e-test-job.yaml into /etc/kubeadm/kube-system.init/99-e2e-test.yaml so one can easily build an image which runs the tests automatically. Or are there issues like needing to wait after kubeadm init for things to stabilise before running the tests in order to get useful results?

ijc · 2018-01-05T11:14:46Z

I think your CI issues are down to the thing which will be fixed by #39. Once we have the code in shape I can bash the retry button till it passes (or maybe #39 will get merged in time for you to rebase onto it)

errordeveloper · 2018-01-05T11:15:07Z

We could consider adding a daily/nightly job which ran on master and ran these tests, but lets defer that until these basics are in.

💯 👍

ijc · 2018-01-05T15:01:36Z

.circleci/config.yml

            - lint
            - pkg-kubelet
            - pkg-cri-containerd
+            - pkg-kube-e2e-test


You are missing a linuxkit pkg push --nobuild for this new package in the push-pkgs-to-hub step.

Well spotted!

ijc · 2018-01-05T15:02:23Z

pkg/kube-e2e-test/build.yml

+org: linuxkit
+image: kube-e2e-test
+network: true
+disable-content-trust: true


I setup content trust for this repo so you can now drop this line.

ijc · 2018-01-05T15:05:28Z

scripts/run-e2e-test.sh

+#!/bin/sh
+
+set -o errexit
+set -o pipefail


pipefail (at least) is a bash-ism so you need #! /bin/bash shebang. Not sure about errexit or nounset, the short names (-e and -u) are portable I think.

Given how awful it is to replicate pipefail in pure POSIX shell I think just using /bin/bash for the shebang is the right answer in this case.

Ah, I didn't realise this... It's okay, we aren't relying on pipefail here. I'll switch to /bin/sh and use set -eu, same goes for pkg/kube-e2e-test/e2e.sh (which currently has an explicit set +o pipefail).

ijc · 2018-01-05T15:26:56Z

scripts/run-e2e-test.sh

+echo "$0: log save in ${PWD}/e2e.log, cleaning up the resources..."
+cleanup 2> /dev/null || true
+grep -q '^Test Suite Passed$' e2e.log && exit 0
+grep -q '^Test Suite Failed$' e2e.log && exit 1


If the log contains neither the pass nor fail messages that is a failure IMHO (perhaps the file is empty because pod logging is b0rked), I think you can just drop the grep from this last line and exit 1 unconditionally.

errordeveloper · 2018-01-05T16:07:26Z

I found an issue with the script, testing a fix right now.

errordeveloper · 2018-01-08T08:29:33Z

@ijc I'm quite happy with the runner script now, PTAL.

Signed-off-by: Ilya Dmitrichenko <[email protected]>

ijc · 2018-01-08T13:22:10Z

.circleci/config.yml

-            linuxkit pkg push --nobuild pkg/cri-containerd
            linuxkit pkg push --nobuild pkg/kubelet
+            linuxkit pkg push --nobuild pkg/cri-containerd
+            linuxkit pkg push --nobuild pkg/kube-e2e-test


Why did you move cri-containerd below kubelet? They were alphabetic before. Suppose it isn't a big deal.

They were in different order in some places, so I was trying to list them in the same order everywhere...

I guess it was the other places which were buggy then ;-)

Lets not worry about it, if someone enthusiastic soul decides they want to sort them later (and add comments to keep them sorted) then they can.

ijc

LGTM

GordonTheTurtle added the dco/no label Dec 18, 2017

GordonTheTurtle added the status/0-triage label Dec 18, 2017

ijc suggested changes Dec 18, 2017

View reviewed changes

ijc mentioned this pull request Dec 19, 2017

Build cni and cri tools #36

Merged

errordeveloper force-pushed the master branch from bdc2376 to 3741a07 Compare December 19, 2017 14:55

GordonTheTurtle added the dco/no label Dec 19, 2017

errordeveloper force-pushed the master branch from 3741a07 to 76d117b Compare December 21, 2017 14:48

GordonTheTurtle removed the dco/no label Dec 21, 2017

errordeveloper force-pushed the master branch from 76d117b to 5083219 Compare December 21, 2017 14:50

ijc reviewed Jan 4, 2018

View reviewed changes

errordeveloper force-pushed the master branch from 5083219 to 9cb3be5 Compare January 4, 2018 21:19

errordeveloper force-pushed the master branch 2 times, most recently from 09067af to bd2dafb Compare January 5, 2018 01:25

errordeveloper force-pushed the master branch 6 times, most recently from 3b7ee1e to 0297ffa Compare January 5, 2018 14:48

ijc reviewed Jan 5, 2018

View reviewed changes

errordeveloper force-pushed the master branch 2 times, most recently from beae7cd to 349b23a Compare January 5, 2018 15:22

ijc reviewed Jan 5, 2018

View reviewed changes

errordeveloper force-pushed the master branch from 349b23a to 720019b Compare January 5, 2018 15:34

errordeveloper mentioned this pull request Jan 5, 2018

[WIP] Booting/testing something in CI #19

Closed

errordeveloper force-pushed the master branch 5 times, most recently from 8e4c750 to 7bd6c72 Compare January 5, 2018 16:54

errordeveloper mentioned this pull request Jan 5, 2018

e2e tests make apiserver crash on d4m #40

Closed

errordeveloper force-pushed the master branch 2 times, most recently from 0edd1f3 to 3d2fad9 Compare January 8, 2018 08:29

errordeveloper force-pushed the master branch from 3d2fad9 to dc79b97 Compare January 8, 2018 08:42

Add kube-e2e-test package

9b6fd9f

Signed-off-by: Ilya Dmitrichenko <[email protected]>

errordeveloper force-pushed the master branch from dc79b97 to 9b6fd9f Compare January 8, 2018 12:04

ijc reviewed Jan 8, 2018

View reviewed changes

ijc approved these changes Jan 8, 2018

View reviewed changes

ijc merged commit 4e6f520 into linuxkit:master Jan 8, 2018

		@@ -0,0 +1 @@
		_cache

Add e2e package #35

Add e2e package #35

Uh oh!

Conversation

errordeveloper commented Dec 18, 2017

Uh oh!

GordonTheTurtle commented Dec 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijc commented Dec 18, 2017

Uh oh!

errordeveloper commented Dec 18, 2017 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijc commented Dec 19, 2017

Uh oh!

ijc commented Dec 19, 2017

Uh oh!

errordeveloper commented Dec 19, 2017

Uh oh!

errordeveloper commented Dec 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

errordeveloper commented Jan 4, 2018

Uh oh!

ijc commented Jan 5, 2018

Uh oh!

ijc commented Jan 5, 2018

Uh oh!

ijc commented Jan 5, 2018

Uh oh!

errordeveloper commented Jan 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

errordeveloper commented Jan 5, 2018

Uh oh!

errordeveloper commented Jan 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

errordeveloper Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

errordeveloper commented Dec 18, 2017 via email •

edited

Loading

errordeveloper Jan 8, 2018 •

edited

Loading