From a13f026fd012859f04467e6007e2cafe4a788927 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Fri, 19 May 2017 16:18:25 -0400 Subject: [PATCH] Panic server on watch errors in test environment This change makes it so that errors during watch decoding panic the server if it is in a test environment. This allows us to catch coder errors related to storing incompatible types at the same location in etcd. Signed-off-by: Monis Khan --- hack/local-up-cluster.sh | 12 ++++--- hack/make-rules/test.sh | 4 +++ .../apiserver/pkg/storage/etcd3/watcher.go | 32 ++++++++++++++++++- .../pkg/storage/etcd3/watcher_test.go | 3 +- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/hack/local-up-cluster.sh b/hack/local-up-cluster.sh index 1188fee113..c59c53b64c 100755 --- a/hack/local-up-cluster.sh +++ b/hack/local-up-cluster.sh @@ -86,6 +86,10 @@ DEFAULT_STORAGE_CLASS=${KUBE_DEFAULT_STORAGE_CLASS:-true} KUBE_CACHE_MUTATION_DETECTOR="${KUBE_CACHE_MUTATION_DETECTOR:-true}" export KUBE_CACHE_MUTATION_DETECTOR +# panic the server on watch decode errors since they are considered coder mistakes +KUBE_PANIC_WATCH_DECODE_ERROR="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}" +export KUBE_PANIC_WATCH_DECODE_ERROR + ADMISSION_CONTROL_CONFIG_FILE=${ADMISSION_CONTROL_CONFIG_FILE:-""} # START_MODE can be 'all', 'kubeletonly', or 'nokubelet' @@ -658,11 +662,11 @@ function start_kubelet { ${KUBELET_FLAGS} >"${KUBELET_LOG}" 2>&1 & KUBELET_PID=$! # Quick check that kubelet is running. - if ps -p $KUBELET_PID > /dev/null ; then + if ps -p $KUBELET_PID > /dev/null ; then echo "kubelet ( $KUBELET_PID ) is running." else cat ${KUBELET_LOG} ; exit 1 - fi + fi else # Docker won't run a container with a cidfile (container id file) # unless that file does not already exist; clean up an existing @@ -733,7 +737,7 @@ function start_kubedns { echo "Creating kube-system namespace" sed -e "s/{{ pillar\['dns_domain'\] }}/${DNS_DOMAIN}/g" "${KUBE_ROOT}/cluster/addons/dns/kubedns-controller.yaml.in" >| kubedns-deployment.yaml sed -e "s/{{ pillar\['dns_server'\] }}/${DNS_SERVER_IP}/g" "${KUBE_ROOT}/cluster/addons/dns/kubedns-svc.yaml.in" >| kubedns-svc.yaml - + # TODO update to dns role once we have one. ${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" create clusterrolebinding system:kube-dns --clusterrole=cluster-admin --serviceaccount=kube-system:default # use kubectl to create kubedns deployment and service @@ -748,7 +752,7 @@ function start_kubedns { function start_kubedashboard { if [[ "${ENABLE_CLUSTER_DASHBOARD}" = true ]]; then - echo "Creating kubernetes-dashboard" + echo "Creating kubernetes-dashboard" # use kubectl to create the dashboard ${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-controller.yaml ${KUBECTL} --kubeconfig="${CERT_DIR}/admin.kubeconfig" create -f ${KUBE_ROOT}/cluster/addons/dashboard/dashboard-service.yaml diff --git a/hack/make-rules/test.sh b/hack/make-rules/test.sh index d577645fd0..fddcc3ec56 100755 --- a/hack/make-rules/test.sh +++ b/hack/make-rules/test.sh @@ -27,6 +27,10 @@ kube::golang::setup_env KUBE_CACHE_MUTATION_DETECTOR="${KUBE_CACHE_MUTATION_DETECTOR:-true}" export KUBE_CACHE_MUTATION_DETECTOR +# panic the server on watch decode errors since they are considered coder mistakes +KUBE_PANIC_WATCH_DECODE_ERROR="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}" +export KUBE_PANIC_WATCH_DECODE_ERROR + # Handle case where OS has sha#sum commands, instead of shasum. if which shasum >/dev/null 2>&1; then SHA1SUM="shasum -a1" diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go index b42ae4b80f..cb172564d3 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go @@ -17,8 +17,11 @@ limitations under the License. package etcd3 import ( + "errors" "fmt" "net/http" + "os" + "strconv" "strings" "sync" @@ -40,6 +43,24 @@ const ( outgoingBufSize = 100 ) +// fatalOnDecodeError is used during testing to panic the server if watcher encounters a decoding error +var fatalOnDecodeError = false + +// errTestingDecode is the only error that testingDeferOnDecodeError catches during a panic +var errTestingDecode = errors.New("sentinel error only used during testing to indicate watch decoding error") + +// testingDeferOnDecodeError is used during testing to recover from a panic caused by errTestingDecode, all other values continue to panic +func testingDeferOnDecodeError() { + if r := recover(); r != nil && r != errTestingDecode { + panic(r) + } +} + +func init() { + // check to see if we are running in a test environment + fatalOnDecodeError, _ = strconv.ParseBool(os.Getenv("KUBE_PANIC_WATCH_DECODE_ERROR")) +} + type watcher struct { client *clientv3.Client codec runtime.Codec @@ -373,9 +394,18 @@ func (wc *watchChan) prepareObjs(e *event) (curObj runtime.Object, oldObj runtim return curObj, oldObj, nil } -func decodeObj(codec runtime.Codec, versioner storage.Versioner, data []byte, rev int64) (runtime.Object, error) { +func decodeObj(codec runtime.Codec, versioner storage.Versioner, data []byte, rev int64) (_ runtime.Object, err error) { obj, err := runtime.Decode(codec, []byte(data)) if err != nil { + if fatalOnDecodeError { + // catch watch decode error iff we caused it on + // purpose during a unit test + defer testingDeferOnDecodeError() + // we are running in a test environment and thus an + // error here is due to a coder mistake if the defer + // does not catch it + panic(err) + } return nil, err } // ensure resource version is set on the object we load from etcd diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go index 00c31735a9..58cbfe4c29 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go @@ -17,7 +17,6 @@ limitations under the License. package etcd3 import ( - "errors" "fmt" "reflect" "strconv" @@ -327,7 +326,7 @@ type testCodec struct { } func (c *testCodec) Decode(data []byte, defaults *schema.GroupVersionKind, into runtime.Object) (runtime.Object, *schema.GroupVersionKind, error) { - return nil, nil, errors.New("Expected decoding failure") + return nil, nil, errTestingDecode } func testCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.Interface) {