From aa2a7d001a7ecdebc027719e17359e217674d384 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 24 Aug 2018 15:03:03 -0700
Subject: [PATCH 01/13] Build relevant binaries with coverage.

---
 hack/lib/golang.sh | 138 +++++++++++++++++++++++++++++++++++++++++----
 hack/lib/util.sh   |  14 +++++
 2 files changed, 140 insertions(+), 12 deletions(-)

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index a7650f401e9..3184a6a1286 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -211,6 +211,15 @@ readonly KUBE_STATIC_LIBRARIES=(
   kubectl
 )
 
+# Fully-qualified package names that we want to instrument for coverage information.
+readonly KUBE_COVERAGE_INSTRUMENTED_PACKAGES=(
+  k8s.io/kubernetes/cmd/kube-apiserver
+  k8s.io/kubernetes/cmd/kube-controller-manager
+  k8s.io/kubernetes/cmd/kube-scheduler
+  k8s.io/kubernetes/cmd/kube-proxy
+  k8s.io/kubernetes/cmd/kubelet
+)
+
 # KUBE_CGO_OVERRIDES is a space-separated list of binaries which should be built
 # with CGO enabled, assuming CGO is supported on the target platform.
 # This overrides any entry in KUBE_STATIC_LIBRARIES.
@@ -441,6 +450,104 @@ kube::golang::outfile_for_binary() {
   echo "${output_path}/${bin}"
 }
 
+# Argument: the name of a Kubernetes package.
+# Returns 0 if the binary can be built with coverage, 0 otherwise.
+# NB: this ignores whether coverage is globally enabled or not.
+kube::golang::is_covered_binary() {
+  return $(kube::util::array_contains "$1" "${KUBE_COVERAGE_INSTRUMENTED_PACKAGES[@]}")
+}
+
+# Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler)
+# Echos the path to a dummy test used for coverage information.
+kube::golang::path_for_coverage_dummy_test() {
+	local package=$1
+  local path="${KUBE_GOPATH}/src/$package"
+  local name=$(basename "$package")
+  echo "$path/zz_autogenerated_${name}_test.go"
+}
+
+# Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler).
+# Creates a dummy unit test on disk in the source directory for the given package.
+# This unit test will invoke the package's standard entry point when run.
+kube::golang::create_coverage_dummy_test() {
+  local package="$1"
+  local name="$(basename "$package")"
+  cat <<EOF > $(kube::golang::path_for_coverage_dummy_test "$package")
+package main
+ import (
+  "flag"
+	"os"
+	"strconv"
+	"testing"
+	"time"
+)
+ func TestMain(m *testing.M) {
+  // We need to pass coverage instructions to the unittest framework, so we hijack os.Args
+  original_args := os.Args
+  now := time.Now().UnixNano()
+  test_args := []string{os.Args[0], "-test.coverprofile=/tmp/k8s-${name}-" + strconv.FormatInt(now, 10) + ".cov"}
+  os.Args = test_args
+
+  // This is sufficient for the unit tests to be set up.
+  flag.Parse()
+
+   // Restore the original args for use by the program.
+	os.Args = original_args
+ 	// Go!
+  main()
+
+   // Make sure we actually write the profiling information to disk, if we make it here.
+  // On long-running services, or anything that calls os.Exit(), this is insufficient,
+  // so be sure to call this from inside the binary too.
+  // TODO: actually have some code here.
+}
+EOF
+}
+
+# Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler).
+# Deletes a test generated by kube::golang::create_coverage_dumy_test.
+kube::golang::delete_coverage_dummy_test() {
+  local package=$1
+  rm $(kube::golang::path_for_coverage_dummy_test "$package")
+}
+
+# Arguments: a list of kubernetes packages to build.
+# Expected variables: build_args should be set to an array of Go build arguments.
+# In addition, the standard build globals are assumed.
+#
+# Invokes Go to actually build some packages. If coverage is disabled, simply invokes
+# go install. If coverage is enabled, builds covered binaries using go test, temporarily
+# producing the required unit test files and then cleaning up after itself.
+# Non-covered binaries are then built using go install as usual.
+kube::golang::build_some_binaries() {
+  if [[ -n "${build_with_coverage:-}" ]]; then
+    local -a uncovered=()
+    for package in "$@"; do
+      if kube::golang::is_covered_binary "$package"; then
+        V=2 kube::log::info "Building $package with coverage..."
+        kube::golang::create_coverage_dummy_test "$package"
+        go test -c -o "$(kube::golang::outfile_for_binary "$package" "$platform")" \
+          -covermode count \
+          -coverpkg k8s.io/... \
+          "${build_args[@]}" \
+          "$package"
+        kube::golang::delete_coverage_dummy_test "$package"
+      else
+        uncovered+=("$package")
+      fi
+    done
+    if [[ "${#uncovered[@]}" != 0 ]]; then
+      V=2 kube::log::info "Building ${uncovered[@]} without coverage..."
+      go install "${build_args[@]}" "${uncovered[@]}"
+    else
+      V=2 kube::log::info "Nothing to build without coverage."
+     fi
+   else
+    V=2 kube::log::info "Coverage is disabled."
+    go install "${build_args[@]}" "$@"
+   fi
+}
+
 kube::golang::build_binaries_for_platform() {
   local platform=$1
 
@@ -460,18 +567,24 @@ kube::golang::build_binaries_for_platform() {
     fi
   done
 
+  local -a build_args
   if [[ "${#statics[@]}" != 0 ]]; then
-    CGO_ENABLED=0 go install -installsuffix static "${goflags[@]:+${goflags[@]}}" \
-      -gcflags "${gogcflags}" \
-      -ldflags "${goldflags}" \
-      "${statics[@]:+${statics[@]}}"
+    build_args=(
+      -installsuffix static
+      ${goflags:+"${goflags[@]}"}
+      -gcflags "${gogcflags:-}"
+      -ldflags "${goldflags:-}"
+    )
+    CGO_ENABLED=0 kube::golang::build_some_binaries "${statics[@]}"
   fi
 
   if [[ "${#nonstatics[@]}" != 0 ]]; then
-    go install "${goflags[@]:+${goflags[@]}}" \
-      -gcflags "${gogcflags}" \
-      -ldflags "${goldflags}" \
-      "${nonstatics[@]:+${nonstatics[@]}}"
+    build_args=(
+      ${goflags:+"${goflags[@]}"}
+      -gcflags "${gogcflags:-}"
+      -ldflags "${goldflags:-}"
+    )
+    kube::golang::build_some_binaries "${nonstatics[@]}"
   fi
 
   for test in "${tests[@]:+${tests[@]}}"; do
@@ -480,9 +593,9 @@ kube::golang::build_binaries_for_platform() {
 
     mkdir -p "$(dirname ${outfile})"
     go test -c \
-      "${goflags[@]:+${goflags[@]}}" \
-      -gcflags "${gogcflags}" \
-      -ldflags "${goldflags}" \
+      ${goflags:+"${goflags[@]}"} \
+      -gcflags "${gogcflags:-}" \
+      -ldflags "${goldflags:-}" \
       -o "${outfile}" \
       "${testpkg}"
   done
@@ -535,10 +648,11 @@ kube::golang::build_binaries() {
     host_platform=$(kube::golang::host_platform)
 
     # Use eval to preserve embedded quoted strings.
-    local goflags goldflags gogcflags
+    local goflags goldflags gogcflags build_with_coverage
     eval "goflags=(${GOFLAGS:-})"
     goldflags="${GOLDFLAGS:-} $(kube::version::ldflags)"
     gogcflags="${GOGCFLAGS:-}"
+    build_with_coverage="${KUBE_BUILD_WITH_COVERAGE:-}"
 
     local -a targets=()
     local arg
diff --git a/hack/lib/util.sh b/hack/lib/util.sh
index a24b1093591..db0842b0c13 100755
--- a/hack/lib/util.sh
+++ b/hack/lib/util.sh
@@ -18,6 +18,20 @@ kube::util::sortable_date() {
   date "+%Y%m%d-%H%M%S"
 }
 
+# arguments: target, item1, item2, item3, ...
+# returns 0 if target is in the given items, 1 otherwise.
+kube::util::array_contains() {
+  local search="$1"
+  local element
+  shift
+  for element; do
+    if [[ "$element" == "$search" ]]; then
+      return 0
+     fi
+  done
+  return 1
+}
+
 kube::util::wait_for_url() {
   local url=$1
   local prefix=${2:-}

From da4bbd421c4c0eb75f730619427c2289d05b4f40 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 24 Aug 2018 17:12:49 -0700
Subject: [PATCH 02/13] Add runtime coverage support.

---
 hack/lib/golang.sh                     | 27 +++------
 pkg/util/coverage/coverage.go          | 83 ++++++++++++++++++++++++++
 pkg/util/coverage/coverage_disabled.go | 29 +++++++++
 pkg/util/coverage/fake_test_deps.go    | 54 +++++++++++++++++
 4 files changed, 175 insertions(+), 18 deletions(-)
 create mode 100644 pkg/util/coverage/coverage.go
 create mode 100644 pkg/util/coverage/coverage_disabled.go
 create mode 100644 pkg/util/coverage/fake_test_deps.go

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index 3184a6a1286..65bf3f58ac7 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -474,32 +474,22 @@ kube::golang::create_coverage_dummy_test() {
   local name="$(basename "$package")"
   cat <<EOF > $(kube::golang::path_for_coverage_dummy_test "$package")
 package main
- import (
-  "flag"
-	"os"
-	"strconv"
-	"testing"
-	"time"
+import (
+  "testing"
+  "k8s.io/kubernetes/pkg/util/coverage"
 )
- func TestMain(m *testing.M) {
-  // We need to pass coverage instructions to the unittest framework, so we hijack os.Args
-  original_args := os.Args
-  now := time.Now().UnixNano()
-  test_args := []string{os.Args[0], "-test.coverprofile=/tmp/k8s-${name}-" + strconv.FormatInt(now, 10) + ".cov"}
-  os.Args = test_args
 
-  // This is sufficient for the unit tests to be set up.
-  flag.Parse()
+func TestMain(m *testing.M) {
+  // Get coverage running
+  coverage.InitCoverage("${name}")
 
-   // Restore the original args for use by the program.
-	os.Args = original_args
  	// Go!
   main()
 
-   // Make sure we actually write the profiling information to disk, if we make it here.
+  // Make sure we actually write the profiling information to disk, if we make it here.
   // On long-running services, or anything that calls os.Exit(), this is insufficient,
   // so be sure to call this from inside the binary too.
-  // TODO: actually have some code here.
+  coverage.FlushCoverage()
 }
 EOF
 }
@@ -530,6 +520,7 @@ kube::golang::build_some_binaries() {
           -covermode count \
           -coverpkg k8s.io/... \
           "${build_args[@]}" \
+          -tags coverage \
           "$package"
         kube::golang::delete_coverage_dummy_test "$package"
       else
diff --git a/pkg/util/coverage/coverage.go b/pkg/util/coverage/coverage.go
new file mode 100644
index 00000000000..cfb20c2e025
--- /dev/null
+++ b/pkg/util/coverage/coverage.go
@@ -0,0 +1,83 @@
+// +build coverage
+
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package coverage provides tools for coverage-instrumented binaries to collect and
+// flush coverage information.
+package coverage
+
+import (
+	"flag"
+	"github.com/golang/glog"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"os"
+	"testing"
+	"time"
+)
+
+const flushInterval = 5 * time.Second
+
+var coverageFile string
+
+// tempCoveragePath returns a temporary file to write coverage information to.
+// The file is in the same directory as the destination, ensuring os.Rename will work.
+func tempCoveragePath() string {
+	return coverageFile + ".tmp"
+}
+
+// InitCoverage is called from the dummy unit test to prepare Go's coverage framework.
+// Clients should never need to call it.
+func InitCoverage(name string) {
+	// We read the coverage destination in from the KUBE_COVERAGE_FILE env var,
+	// or if it's empty we just use a default in /tmp
+	coverageFile = os.Getenv("KUBE_COVERAGE_FILE")
+	if coverageFile == "" {
+		coverageFile = "/tmp/k8s-" + name + ".cov"
+	}
+
+	// Set up the unit test framework with the arguments we want.
+	flag.CommandLine.Parse([]string{"-test.coverprofile=" + tempCoveragePath()})
+
+	// Begin periodic logging
+	go wait.Forever(FlushCoverage, flushInterval)
+}
+
+// FlushCoverage flushes collected coverage information to disk.
+// The destination file is configured at startup and cannot be changed.
+// Calling this function also sends a line like "coverage: 5% of statements" to stdout.
+func FlushCoverage() {
+	// We're not actually going to run any tests, but we need Go to think we did so it writes
+	// coverage information to disk. To achieve this, we create a bunch of empty test suites and
+	// have it "run" them.
+	tests := []testing.InternalTest{}
+	benchmarks := []testing.InternalBenchmark{}
+	examples := []testing.InternalExample{}
+
+	var deps fakeTestDeps
+
+	dummyRun := testing.MainStart(deps, tests, benchmarks, examples)
+	dummyRun.Run()
+
+	// Once it writes to the temporary path, we move it to the intended path.
+	// This gets us atomic updates from the perspective of another process trying to access
+	// the file.
+	if err := os.Rename(tempCoveragePath(), coverageFile); err != nil {
+		// This should never fail, because we're in the same directory. There's also little that
+		// we can do if it does.
+		glog.Errorf("Couldn't move coverage file from %s to %s", coverageFile, tempCoveragePath())
+	}
+}
diff --git a/pkg/util/coverage/coverage_disabled.go b/pkg/util/coverage/coverage_disabled.go
new file mode 100644
index 00000000000..c10ba69844f
--- /dev/null
+++ b/pkg/util/coverage/coverage_disabled.go
@@ -0,0 +1,29 @@
+// +build !coverage
+
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package coverage
+
+// InitCoverage is a no-op when not running with coverage.
+func InitCoverage(name string) {
+
+}
+
+// FlushCoverage is a no-op when not running with coverage.
+func FlushCoverage() {
+
+}
diff --git a/pkg/util/coverage/fake_test_deps.go b/pkg/util/coverage/fake_test_deps.go
new file mode 100644
index 00000000000..8ca0b9b0934
--- /dev/null
+++ b/pkg/util/coverage/fake_test_deps.go
@@ -0,0 +1,54 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package coverage
+
+import (
+	"io"
+)
+
+// This is an implementation of testing.testDeps. It doesn't need to do anything, because
+// no tests are actually run. It does need a concrete implementation of at least ImportPath,
+// which is called unconditionally when running tests.
+type fakeTestDeps struct{}
+
+func (fakeTestDeps) ImportPath() string {
+	return ""
+}
+
+func (fakeTestDeps) MatchString(pat, str string) (bool, error) {
+	return false, nil
+}
+
+func (fakeTestDeps) StartCPUProfile(io.Writer) error {
+	return nil
+}
+
+func (fakeTestDeps) StopCPUProfile() {}
+
+func (fakeTestDeps) StartTestLog(io.Writer) {}
+
+func (fakeTestDeps) StopTestLog() error {
+	return nil
+}
+
+func (fakeTestDeps) WriteHeapProfile(io.Writer) error {
+	return nil
+}
+
+func (fakeTestDeps) WriteProfileTo(string, io.Writer, int) error {
+	return nil
+}

From 254618ed613e3babe9ccaa997a7f78061c33d4cf Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Mon, 27 Aug 2018 14:11:29 -0700
Subject: [PATCH 03/13] Pass KUBE_BUILD_WITH_COVERAGE through to docker
 instance.

---
 build/common.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/build/common.sh b/build/common.sh
index fe1ef180970..46336a9dd69 100755
--- a/build/common.sh
+++ b/build/common.sh
@@ -596,6 +596,7 @@ function kube::build::run_build_command_ex() {
     --env "KUBE_FASTBUILD=${KUBE_FASTBUILD:-false}"
     --env "KUBE_BUILDER_OS=${OSTYPE:-notdetected}"
     --env "KUBE_VERBOSE=${KUBE_VERBOSE}"
+    --env "KUBE_BUILD_WITH_COVERAGE=${KUBE_BUILD_WITH_COVERAGE:-}"
     --env "GOFLAGS=${GOFLAGS:-}"
     --env "GOLDFLAGS=${GOLDFLAGS:-}"
     --env "GOGCFLAGS=${GOGCFLAGS:-}"

From 6afc1303407047d6edcb07213b7bbaf87327ed01 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Tue, 28 Aug 2018 16:40:44 -0700
Subject: [PATCH 04/13] Add autogenerated BUILD files.

---
 pkg/util/BUILD          |  1 +
 pkg/util/coverage/BUILD | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 pkg/util/coverage/BUILD

diff --git a/pkg/util/BUILD b/pkg/util/BUILD
index 2cd593108e7..d80a3832de1 100644
--- a/pkg/util/BUILD
+++ b/pkg/util/BUILD
@@ -16,6 +16,7 @@ filegroup(
         "//pkg/util/config:all-srcs",
         "//pkg/util/configz:all-srcs",
         "//pkg/util/conntrack:all-srcs",
+        "//pkg/util/coverage:all-srcs",
         "//pkg/util/dbus:all-srcs",
         "//pkg/util/ebtables:all-srcs",
         "//pkg/util/env:all-srcs",
diff --git a/pkg/util/coverage/BUILD b/pkg/util/coverage/BUILD
new file mode 100644
index 00000000000..30b93dc274c
--- /dev/null
+++ b/pkg/util/coverage/BUILD
@@ -0,0 +1,25 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "coverage_disabled.go",
+        "fake_test_deps.go",
+    ],
+    importpath = "k8s.io/kubernetes/pkg/util/coverage",
+    visibility = ["//visibility:public"],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

From 8fe64670135bfd598a1069279f048934ecc601a2 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Thu, 30 Aug 2018 16:18:15 -0700
Subject: [PATCH 05/13] Improve bash formatting.

---
 hack/lib/golang.sh | 34 +++++++++++++++++-----------------
 hack/lib/util.sh   |  2 +-
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index 65bf3f58ac7..8d74afc047c 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -451,7 +451,7 @@ kube::golang::outfile_for_binary() {
 }
 
 # Argument: the name of a Kubernetes package.
-# Returns 0 if the binary can be built with coverage, 0 otherwise.
+# Returns 0 if the binary can be built with coverage, 1 otherwise.
 # NB: this ignores whether coverage is globally enabled or not.
 kube::golang::is_covered_binary() {
   return $(kube::util::array_contains "$1" "${KUBE_COVERAGE_INSTRUMENTED_PACKAGES[@]}")
@@ -460,9 +460,9 @@ kube::golang::is_covered_binary() {
 # Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler)
 # Echos the path to a dummy test used for coverage information.
 kube::golang::path_for_coverage_dummy_test() {
-	local package=$1
-  local path="${KUBE_GOPATH}/src/$package"
-  local name=$(basename "$package")
+  local package="$1"
+  local path="${KUBE_GOPATH}/src/${package}"
+  local name=$(basename "${package}")
   echo "$path/zz_autogenerated_${name}_test.go"
 }
 
@@ -471,8 +471,8 @@ kube::golang::path_for_coverage_dummy_test() {
 # This unit test will invoke the package's standard entry point when run.
 kube::golang::create_coverage_dummy_test() {
   local package="$1"
-  local name="$(basename "$package")"
-  cat <<EOF > $(kube::golang::path_for_coverage_dummy_test "$package")
+  local name="$(basename "${package}")"
+  cat <<EOF > $(kube::golang::path_for_coverage_dummy_test "${package}")
 package main
 import (
   "testing"
@@ -483,7 +483,7 @@ func TestMain(m *testing.M) {
   // Get coverage running
   coverage.InitCoverage("${name}")
 
- 	// Go!
+  // Go!
   main()
 
   // Make sure we actually write the profiling information to disk, if we make it here.
@@ -495,10 +495,10 @@ EOF
 }
 
 # Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler).
-# Deletes a test generated by kube::golang::create_coverage_dumy_test.
+# Deletes a test generated by kube::golang::create_coverage_dummy_test.
 kube::golang::delete_coverage_dummy_test() {
-  local package=$1
-  rm $(kube::golang::path_for_coverage_dummy_test "$package")
+  local package="$1"
+  rm $(kube::golang::path_for_coverage_dummy_test "${package}")
 }
 
 # Arguments: a list of kubernetes packages to build.
@@ -513,18 +513,18 @@ kube::golang::build_some_binaries() {
   if [[ -n "${build_with_coverage:-}" ]]; then
     local -a uncovered=()
     for package in "$@"; do
-      if kube::golang::is_covered_binary "$package"; then
-        V=2 kube::log::info "Building $package with coverage..."
-        kube::golang::create_coverage_dummy_test "$package"
-        go test -c -o "$(kube::golang::outfile_for_binary "$package" "$platform")" \
+      if kube::golang::is_covered_binary "${package}"; then
+        V=2 kube::log::info "Building ${package} with coverage..."
+        kube::golang::create_coverage_dummy_test "${package}"
+        go test -c -o "$(kube::golang::outfile_for_binary "${package}" "${platform}")" \
           -covermode count \
           -coverpkg k8s.io/... \
           "${build_args[@]}" \
           -tags coverage \
-          "$package"
-        kube::golang::delete_coverage_dummy_test "$package"
+          "${package}"
+        kube::golang::delete_coverage_dummy_test "${package}"
       else
-        uncovered+=("$package")
+        uncovered+=("${package}")
       fi
     done
     if [[ "${#uncovered[@]}" != 0 ]]; then
diff --git a/hack/lib/util.sh b/hack/lib/util.sh
index db0842b0c13..19220c93c3b 100755
--- a/hack/lib/util.sh
+++ b/hack/lib/util.sh
@@ -25,7 +25,7 @@ kube::util::array_contains() {
   local element
   shift
   for element; do
-    if [[ "$element" == "$search" ]]; then
+    if [[ "${element}" == "${search}" ]]; then
       return 0
      fi
   done

From 9d499cd0057f1a9b94da67e07ac902c6bd64ca3a Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Thu, 30 Aug 2018 16:57:10 -0700
Subject: [PATCH 06/13] Be sure we delete the dummy tests.

---
 hack/lib/golang.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index 8d74afc047c..a691194c9a3 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -496,9 +496,10 @@ EOF
 
 # Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler).
 # Deletes a test generated by kube::golang::create_coverage_dummy_test.
+# It is not an error to call this for a nonexistent test.
 kube::golang::delete_coverage_dummy_test() {
   local package="$1"
-  rm $(kube::golang::path_for_coverage_dummy_test "${package}")
+  rm -f $(kube::golang::path_for_coverage_dummy_test "${package}")
 }
 
 # Arguments: a list of kubernetes packages to build.
@@ -515,14 +516,16 @@ kube::golang::build_some_binaries() {
     for package in "$@"; do
       if kube::golang::is_covered_binary "${package}"; then
         V=2 kube::log::info "Building ${package} with coverage..."
+
         kube::golang::create_coverage_dummy_test "${package}"
+        kube::util::trap_add "kube::golang::delete_coverage_dummy_test \"${package}\"" EXIT
+
         go test -c -o "$(kube::golang::outfile_for_binary "${package}" "${platform}")" \
           -covermode count \
           -coverpkg k8s.io/... \
           "${build_args[@]}" \
           -tags coverage \
           "${package}"
-        kube::golang::delete_coverage_dummy_test "${package}"
       else
         uncovered+=("${package}")
       fi

From 0fb4b920b5093a23a42fd5d9bc7f1fbad4156135 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 10:49:36 -0700
Subject: [PATCH 07/13] Address review comments.

---
 hack/lib/golang.sh            | 4 ++--
 pkg/util/coverage/coverage.go | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index a691194c9a3..04ec4874db8 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -453,7 +453,7 @@ kube::golang::outfile_for_binary() {
 # Argument: the name of a Kubernetes package.
 # Returns 0 if the binary can be built with coverage, 1 otherwise.
 # NB: this ignores whether coverage is globally enabled or not.
-kube::golang::is_covered_binary() {
+kube::golang::is_instrumented_package() {
   return $(kube::util::array_contains "$1" "${KUBE_COVERAGE_INSTRUMENTED_PACKAGES[@]}")
 }
 
@@ -514,7 +514,7 @@ kube::golang::build_some_binaries() {
   if [[ -n "${build_with_coverage:-}" ]]; then
     local -a uncovered=()
     for package in "$@"; do
-      if kube::golang::is_covered_binary "${package}"; then
+      if kube::golang::is_instrumented_package "${package}"; then
         V=2 kube::log::info "Building ${package} with coverage..."
 
         kube::golang::create_coverage_dummy_test "${package}"
diff --git a/pkg/util/coverage/coverage.go b/pkg/util/coverage/coverage.go
index cfb20c2e025..bf95ae4379c 100644
--- a/pkg/util/coverage/coverage.go
+++ b/pkg/util/coverage/coverage.go
@@ -49,8 +49,8 @@ func InitCoverage(name string) {
 		coverageFile = "/tmp/k8s-" + name + ".cov"
 	}
 
-	// Set up the unit test framework with the arguments we want.
-	flag.CommandLine.Parse([]string{"-test.coverprofile=" + tempCoveragePath()})
+	// Set up the unit test framework with the required arguments to activate test coverage.
+	flag.CommandLine.Parse([]string{"-test.coverprofile", tempCoveragePath()})
 
 	// Begin periodic logging
 	go wait.Forever(FlushCoverage, flushInterval)
@@ -76,8 +76,6 @@ func FlushCoverage() {
 	// This gets us atomic updates from the perspective of another process trying to access
 	// the file.
 	if err := os.Rename(tempCoveragePath(), coverageFile); err != nil {
-		// This should never fail, because we're in the same directory. There's also little that
-		// we can do if it does.
 		glog.Errorf("Couldn't move coverage file from %s to %s", coverageFile, tempCoveragePath())
 	}
 }

From 2d36e9e8741e1a5500910af31dd6e8b14d0899d2 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 15:52:48 -0700
Subject: [PATCH 08/13] Add KUBE_COVERAGE_FLUSH_INTERVAL to set flush interval.

---
 pkg/util/coverage/coverage.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/pkg/util/coverage/coverage.go b/pkg/util/coverage/coverage.go
index bf95ae4379c..020c30657ac 100644
--- a/pkg/util/coverage/coverage.go
+++ b/pkg/util/coverage/coverage.go
@@ -29,7 +29,7 @@ import (
 	"time"
 )
 
-const flushInterval = 5 * time.Second
+var flushInterval = 5 * time.Second
 
 var coverageFile string
 
@@ -49,6 +49,10 @@ func InitCoverage(name string) {
 		coverageFile = "/tmp/k8s-" + name + ".cov"
 	}
 
+	if duration, err := time.ParseDuration(os.Getenv("KUBE_COVERAGE_FLUSH_INTERVAL")); err == nil {
+		flushInterval = duration
+	}
+
 	// Set up the unit test framework with the required arguments to activate test coverage.
 	flag.CommandLine.Parse([]string{"-test.coverprofile", tempCoveragePath()})
 

From facce197b1882f5d955c0238c6b74db53a73cf0a Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 16:04:48 -0700
Subject: [PATCH 09/13] Update stale comment.

---
 hack/lib/golang.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index 04ec4874db8..e5b3b35425d 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -488,7 +488,8 @@ func TestMain(m *testing.M) {
 
   // Make sure we actually write the profiling information to disk, if we make it here.
   // On long-running services, or anything that calls os.Exit(), this is insufficient,
-  // so be sure to call this from inside the binary too.
+  // so we also flush periodically with a default period of five seconds (configurable by
+  // the KUBE_COVERAGE_FLUSH_INTERVAL environment variable).
   coverage.FlushCoverage()
 }
 EOF

From 13d1961d2b516d984589511913518058329e21cb Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 17:06:20 -0700
Subject: [PATCH 10/13] Improve error behaviour of package coverage.

---
 pkg/util/coverage/coverage.go          | 14 ++++++++++----
 pkg/util/coverage/coverage_disabled.go |  4 ++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/pkg/util/coverage/coverage.go b/pkg/util/coverage/coverage.go
index 020c30657ac..a6cdb2e73d4 100644
--- a/pkg/util/coverage/coverage.go
+++ b/pkg/util/coverage/coverage.go
@@ -22,6 +22,7 @@ package coverage
 
 import (
 	"flag"
+	"fmt"
 	"github.com/golang/glog"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"os"
@@ -29,8 +30,6 @@ import (
 	"time"
 )
 
-var flushInterval = 5 * time.Second
-
 var coverageFile string
 
 // tempCoveragePath returns a temporary file to write coverage information to.
@@ -48,9 +47,16 @@ func InitCoverage(name string) {
 	if coverageFile == "" {
 		coverageFile = "/tmp/k8s-" + name + ".cov"
 	}
+	fmt.Println("Dumping coverage information to " + coverageFile)
 
-	if duration, err := time.ParseDuration(os.Getenv("KUBE_COVERAGE_FLUSH_INTERVAL")); err == nil {
-		flushInterval = duration
+	flushInterval := 5 * time.Second
+	requestedInterval := os.Getenv("KUBE_COVERAGE_FLUSH_INTERVAL")
+	if requestedInterval != "" {
+		if duration, err := time.ParseDuration(requestedInterval); err == nil {
+			flushInterval = duration
+		} else {
+			panic("Invalid KUBE_COVERAGE_FLUSH_INTERVAL value; try something like '30s'.")
+		}
 	}
 
 	// Set up the unit test framework with the required arguments to activate test coverage.
diff --git a/pkg/util/coverage/coverage_disabled.go b/pkg/util/coverage/coverage_disabled.go
index c10ba69844f..2e2f12f6347 100644
--- a/pkg/util/coverage/coverage_disabled.go
+++ b/pkg/util/coverage/coverage_disabled.go
@@ -18,9 +18,9 @@ limitations under the License.
 
 package coverage
 
-// InitCoverage is a no-op when not running with coverage.
+// InitCoverage is illegal when not running with coverage.
 func InitCoverage(name string) {
-
+	panic("Called InitCoverage when not built with coverage instrumentation.")
 }
 
 // FlushCoverage is a no-op when not running with coverage.

From 9b790dab7d5d47daf5b062e1efa341641229ed2c Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 17:07:25 -0700
Subject: [PATCH 11/13] Address shell-related comments.

---
 .gitignore         | 1 +
 hack/lib/golang.sh | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 9aa7a78aa3c..02a2a4b1a78 100644
--- a/.gitignore
+++ b/.gitignore
@@ -111,6 +111,7 @@ kubernetes.tar.gz
 # TODO(thockin): uncomment this when we stop committing the generated files.
 #zz_generated.*
 zz_generated.openapi.go
+zz_generated_*_test.go
 
 # make-related metadata
 /.make/
diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh
index e5b3b35425d..0db5008b1a5 100755
--- a/hack/lib/golang.sh
+++ b/hack/lib/golang.sh
@@ -463,7 +463,7 @@ kube::golang::path_for_coverage_dummy_test() {
   local package="$1"
   local path="${KUBE_GOPATH}/src/${package}"
   local name=$(basename "${package}")
-  echo "$path/zz_autogenerated_${name}_test.go"
+  echo "$path/zz_generated_${name}_test.go"
 }
 
 # Argument: the name of a Kubernetes package (e.g. k8s.io/kubernetes/cmd/kube-scheduler).
@@ -504,8 +504,9 @@ kube::golang::delete_coverage_dummy_test() {
 }
 
 # Arguments: a list of kubernetes packages to build.
-# Expected variables: build_args should be set to an array of Go build arguments.
-# In addition, the standard build globals are assumed.
+# Expected variables: ${build_args} should be set to an array of Go build arguments.
+# In addition, ${package} and ${platform} should have been set earlier, and if
+# ${build_with_coverage} is set, coverage instrumentation will be enabled.
 #
 # Invokes Go to actually build some packages. If coverage is disabled, simply invokes
 # go install. If coverage is enabled, builds covered binaries using go test, temporarily

From c3e08bec7a90bae486b88505035abf516b588011 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 17:51:42 -0700
Subject: [PATCH 12/13] Add owners.

---
 pkg/util/coverage/OWNERS | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 pkg/util/coverage/OWNERS

diff --git a/pkg/util/coverage/OWNERS b/pkg/util/coverage/OWNERS
new file mode 100644
index 00000000000..a21c8bf7230
--- /dev/null
+++ b/pkg/util/coverage/OWNERS
@@ -0,0 +1,8 @@
+approvers:
+  - katharine
+  - spiffxp
+reviewers:
+  - katharine
+  - spiffxp
+labels:
+  - sig/testing

From 9390847bd5b5662beae9288eb91e0bd4c2c0c9a3 Mon Sep 17 00:00:00 2001
From: Katharine Berry <ktbry@google.com>
Date: Fri, 31 Aug 2018 18:01:02 -0700
Subject: [PATCH 13/13] Change owners.

---
 pkg/util/coverage/OWNERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/util/coverage/OWNERS b/pkg/util/coverage/OWNERS
index a21c8bf7230..f517735525c 100644
--- a/pkg/util/coverage/OWNERS
+++ b/pkg/util/coverage/OWNERS
@@ -1,8 +1,8 @@
 approvers:
-  - katharine
+  - bentheelder
   - spiffxp
 reviewers:
-  - katharine
+  - bentheelder
   - spiffxp
 labels:
   - sig/testing