upstream heptio/kube-conformance #69368

dims · 2018-10-03T13:01:43Z

Pick up some code from https://github.com/heptio/kube-conformance

Command line to build the container:
REGISTRY=staging-k8s.gcr.io VERSION=v1.12.1 hack/dev-push-conformance.sh

Change-Id: I22d2436ad2658bdc889af566beb21f3714d0b0b0

What this PR does / why we need it:

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

Special notes for your reviewer:

Release note:

Added a new container based image for running e2e tests

dims · 2018-10-03T13:02:01Z

In support of #69110

cc @timothysc @jbeda

dims · 2018-10-03T13:04:36Z

cc @brahmaroutu @duglin @bradtopol

timothysc

@dims thanks for doing this!

Minor comment about adding other .test artifacts and possibly changing the name.

timothysc · 2018-10-03T18:29:39Z

build/lib/release.sh

@@ -217,6 +218,12 @@ function kube::release::build_server_images() {
    if [[ "${KUBE_BUILD_HYPERKUBE}" =~ [yY] ]]; then
      cp "${LOCAL_OUTPUT_BINPATH}/${platform}/hyperkube" "${release_stage}/server/bin"
    fi
+    # if we are building conformance, we also need to copy the binaries needed
+    if [[ "${KUBE_BUILD_CONFORMANCE}" =~ [yY] ]]; then
+      cp "${LOCAL_OUTPUT_BINPATH}/${platform}/e2e.test" "${release_stage}/server/bin"


fwiw I think we should call it kube_tests and include the other suites. We had an issue open for a while to include the node tests to evaluate prior to running conformance.

Ack. i want to get the basic skeleton working and hand it off to @brahmaroutu or @duglin who promised to help.

cp "${LOCAL_OUTPUT_BINPATH}/${platform}/*.test" "${release_stage}/server/bin" should work.

Done @timothysc

I guess this works. I'd think of this as part of building the test tarball rather than the server (=control plane) images ... Anyone building those will need to opt out of another image now or lose build time again.

switching back to cp "${LOCAL_OUTPUT_BINPATH}/${platform}/e2e.test" "${release_stage}/server/bin", will revisit later when we add node conformance test etc

Why do you need these test binaries in the server tarball? I don't see them used anywhere.

Ideally, we would want e2e.test and e2e_node.test - When a group is running conformance this gives them the ability to check to see it the node itself passes the local checks.

spiffxp · 2018-10-04T23:49:39Z

/area conformance
/sig testing
/cc @BenTheElder @Katharine

spiffxp · 2018-10-04T23:50:12Z

/kind feature

dims · 2018-10-11T11:06:45Z

/retest

dims · 2018-10-11T11:51:00Z

/test pull-kubernetes-cross

ixdy · 2018-10-11T23:17:29Z

build/lib/release.sh

@@ -217,6 +218,12 @@ function kube::release::build_server_images() {
    if [[ "${KUBE_BUILD_HYPERKUBE}" =~ [yY] ]]; then
      cp "${LOCAL_OUTPUT_BINPATH}/${platform}/hyperkube" "${release_stage}/server/bin"
    fi
+    # if we are building conformance, we also need to copy the binaries needed
+    if [[ "${KUBE_BUILD_CONFORMANCE}" =~ [yY] ]]; then
+      cp "${LOCAL_OUTPUT_BINPATH}/${platform}/e2e.test" "${release_stage}/server/bin"


Why do you need these test binaries in the server tarball? I don't see them used anywhere.

ixdy · 2018-10-11T23:18:46Z

build/lib/release.sh

+  if [[ -n "${save_dir}" ]]; then
+    "${DOCKER[@]}" save "${conformance_tag}" > "${save_dir}/conformance-${arch}.tar"
+  fi
+  if [[ -z "${KUBE_DOCKER_IMAGE_TAG-}" || -z "${KUBE_DOCKER_REGISTRY-}" ]]; then


I'm pretty sure we can always call rmi here - we use the tarballs to push to gcr.io these days, rather than pushing images directly. (The conditionals are basically legacy stuff I forgot to clean up after #47939.)

ok removing the test binaries

cool, removing the if condition around rmi

ixdy · 2018-10-11T23:24:24Z

cluster/images/conformance/Makefile

+E2E_TEST_BIN?=$(shell pwd)/../../../$(OUT_DIR)/dockerized/bin/linux/$(ARCH)/e2e.test
+CLUSTER_DIR?=$(shell pwd)/../../../cluster/
+
+BASEIMAGE=k8s.gcr.io/debian-hyperkube-base-$(ARCH):0.10.2


why hyperkube?

i needed something that had ca-certificates and just reusing the existing image.

https://github.com/heptio/kube-conformance/blob/master/Dockerfile#L20

you could also potentially clean-install ca-certificates in the Dockerfile for conformance. hyperkube-base has a lot of extra stuff you don't really need...

OTOH installing packages at build time is not great, so I'm OK with leaving this as-is for now.

whew! thanks :)

ixdy · 2018-10-11T23:25:14Z

cluster/images/conformance/Makefile

+REGISTRY?=staging-k8s.gcr.io
+ARCH?=amd64
+OUT_DIR?=_output
+GINKGO_BIN?=$(shell pwd)/../../../$(OUT_DIR)/dockerized/bin/linux/$(ARCH)/ginkgo


maybe build/lib/release.sh could set these variables, since it really knows the path?

was just following the pattern from cluster/images/hyperkube/Makefile could we do this in a later PR? (clean up both)

ixdy · 2018-10-11T23:27:04Z

build/release-images.sh

@@ -29,6 +29,9 @@ CMD_TARGETS="${KUBE_SERVER_IMAGE_TARGETS[*]}"
 if [[ "${KUBE_BUILD_HYPERKUBE}" =~ [yY] ]]; then
    CMD_TARGETS="${CMD_TARGETS} cmd/hyperkube"
 fi
+if [[ "${KUBE_BUILD_CONFORMANCE}" =~ [yY] ]]; then
+    CMD_TARGETS="${CMD_TARGETS} vendor/github.com/onsi/ginkgo/ginkgo test/e2e/e2e.test cmd/kubectl"


would it make sense to list these targets in hack/lib/golang.sh rather than repeating them in multiple places?

i was not sure KUBE_SERVER_IMAGE_TARGETS was appropriate to change (see line 28)

I meant to add another variable like KUBE_CONFORMANCE_TEST_TARGETS or something.

perfect. doing that now

ixdy · 2018-10-11T23:27:51Z

cluster/images/conformance/run_e2e.sh

@@ -0,0 +1,53 @@
+#!/bin/bash


should this script have errexit, nounset, and pipefail per usual?

ixdy · 2018-10-11T23:39:06Z

hack/dev-push-conformance.sh

+	exit -1
+fi
+
+IMAGE="${REGISTRY}/conformance-amd64:${VERSION}"


should we be using a manifest-list here instead?

eventually, this is just a helper script. (make release/quick-release will spit out images for all platforms already)

dims · 2018-10-12T00:54:02Z

/retest

dims · 2018-10-12T03:10:49Z

/retest

dims · 2018-10-12T15:12:15Z

@ixdy there's more things to do here which we can tackle while we are getting a job up and go green. i'll open a follow up Issue for @brahmaroutu and myself.

timothysc · 2018-10-12T17:56:28Z

/lgtm

ixdy · 2018-10-12T18:47:04Z

cluster/images/conformance/run_e2e.sh

+
+ginkgo_args=(
+    "--focus=${E2E_FOCUS}"
+    "--skip=${E2E_SKIP}"


E2E_SKIP isn't defined by default - should it be?

It doesn't need to be and it's defined in the dockerfile.

@timothysc i had removed it in one of the edits. yes, should be good with latest changes

ixdy · 2018-10-12T18:47:41Z

cluster/images/conformance/run_e2e.sh

+    [1-9]|[1-9][0-9]*) ginkgo_args+=("--nodes=${E2E_PARALLEL}") ;;
+esac
+
+echo "/usr/local/bin/ginkgo ${ginkgo_args[@]} /usr/local/bin/e2e.test -- --disable-log-dump --repo-root=/kubernetes --provider=\"${E2E_PROVIDER}\" --report-dir=\"${RESULTS_DIR}\" --kubeconfig=\"${KUBECONFIG}\""


similarly for KUBECONFIG

same answer.

y i added it after @ixdy's nudge

dims · 2018-10-12T18:49:02Z

/hold

ixdy · 2018-10-12T18:49:26Z

it'd be worth following up with the bazel rules to build this image, too. I believe this would do it (put in cluster/images/conformance/BUILD:

load("@io_bazel_rules_docker//container:container.bzl", "container_bundle", "container_image", "container_layer")

container_layer(
    name = "cluster-srcs",
    data_path = "/",
    directory = "/kubernetes",
    files = ["//cluster:all-srcs"],
)

container_layer(
    name = "bins",
    directory = "/usr/local/bin",
    files = [
        "//cmd/kubectl",
        "//test/e2e:e2e.test",
        "//vendor/github.com/onsi/ginkgo/ginkgo",
    ],
)

container_image(
    name = "conformance-internal",
    base = "@debian-hyperkube-base-amd64//image",
    cmd = [
        "/bin/bash",
        "-c",
        "/run_e2e.sh",
    ],
    env = {
        "E2E_FOCUS": "\[Conformance\]",
        "E2E_PARALLEL": "1",
        "E2E_PROVIDER": "local",
        "RESULTS_DIR": "/tmp/results",
    },
    files = [
        ":run_e2e.sh",
    ],
    layers = [
        ":cluster-srcs",
        ":bins",
    ],
    stamp = True,
    workdir = "/usr/local/bin",
)

container_bundle(
    name = "conformance",
    images = {"k8s.gcr.io/conformance-amd64:{STABLE_DOCKER_TAG}": "conformance-internal"},
    stamp = True,
    visibility = ["//visibility:public"],
)

ixdy · 2018-10-12T18:54:02Z

(updated my bazel code to add visibility on the container_bundle rule; you'd also need to run hack/update-bazel.sh to update the various all-srcs rules)

dims · 2018-10-12T19:04:04Z

Thanks @ixdy adding the cluster/images/conformance/BUILD

Pick up some code from https://github.com/heptio/kube-conformance Fix up build scripts for the new conformance image Fix Header template and Copyright to make verify job go green update README and add execute permissions for script Change-Id: Ib6509acd816cc2fb3a516bfb8e0ff9e32bff8f79

ixdy · 2018-10-12T20:31:46Z

cluster/images/conformance/Makefile

@@ -0,0 +1,66 @@
+# Copyright 2016 The Kubernetes Authors.


technically this should be 2018 but I don't really care

ixdy · 2018-10-12T20:33:24Z

/lgtm

spiffxp · 2018-10-12T20:57:40Z

/retest

dims · 2018-10-12T22:24:29Z

/hold cancel

k8s-ci-robot requested review from eparis and roberthbailey October 3, 2018 13:01

timothysc reviewed Oct 3, 2018

View reviewed changes

k8s-ci-robot requested review from BenTheElder and Katharine October 4, 2018 23:49

k8s-ci-robot added area/conformance Issues or PRs related to kubernetes conformance tests sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 4, 2018

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 4, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from 513d8bc to 066f512 Compare October 11, 2018 02:05

dims changed the title ~~[WIP] upstream heptio/kube-conformance~~ upstream heptio/kube-conformance Oct 11, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 11, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from 066f512 to 8964386 Compare October 11, 2018 02:06

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 11, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from 8964386 to c6b3985 Compare October 11, 2018 12:58

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from f44bf1d to 0cac408 Compare October 11, 2018 23:12

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2018

ixdy reviewed Oct 11, 2018

View reviewed changes

dims force-pushed the upstream-heptio-kube-conformance branch from 0cac408 to 2090bb8 Compare October 12, 2018 02:07

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2018

ixdy reviewed Oct 12, 2018

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from 2090bb8 to 6881049 Compare October 12, 2018 19:27

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2018

dims force-pushed the upstream-heptio-kube-conformance branch from 6881049 to 6830bad Compare October 12, 2018 19:33

ixdy reviewed Oct 12, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2018

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2018

k8s-ci-robot merged commit 8e4f781 into kubernetes:master Oct 13, 2018

cblecker mentioned this pull request Oct 15, 2018

Switch off KUBE_BUILD_CONFORMANCE to diagnose a ci-cross issue #69817

Merged

ixdy mentioned this pull request Oct 15, 2018

Remove last traces of old registry name #69779

Merged

upstream heptio/kube-conformance #69368

upstream heptio/kube-conformance #69368

Conversation

dims commented Oct 3, 2018 • edited Loading

dims commented Oct 3, 2018

dims commented Oct 3, 2018

timothysc 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

Choose a reason for hiding this comment

spiffxp commented Oct 4, 2018

spiffxp commented Oct 4, 2018

dims commented Oct 11, 2018

dims commented Oct 11, 2018

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dims commented Oct 12, 2018

dims commented Oct 12, 2018

dims commented Oct 12, 2018

timothysc commented Oct 12, 2018

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

dims commented Oct 12, 2018

ixdy commented Oct 12, 2018 • edited Loading

ixdy commented Oct 12, 2018

dims commented Oct 12, 2018

Choose a reason for hiding this comment

ixdy commented Oct 12, 2018

spiffxp commented Oct 12, 2018

dims commented Oct 12, 2018

dims commented Oct 3, 2018 •

edited

Loading

ixdy commented Oct 12, 2018 •

edited

Loading