From eed389c7822e140741bfda90cd3299ebf6e0ff42 Mon Sep 17 00:00:00 2001 From: Tim Swanson Date: Tue, 19 Feb 2019 11:24:09 -0500 Subject: [PATCH 1/5] Fix gometalinter 3.0 errors - per gometalinter 3.0 notes, replace megacheck with staticcheck - fix errors found by staticcheck (mostly unused funcs/vars) --- cmd/istio-cni/main.go | 5 ++--- cmd/istio-cni/main_test.go | 1 - cmd/istio-cni/redirect.go | 24 ------------------------ lintconfig_base.json | 4 ++-- test/install_k8s_test.go | 12 +++++------- 5 files changed, 9 insertions(+), 37 deletions(-) diff --git a/cmd/istio-cni/main.go b/cmd/istio-cni/main.go index bd615aa8c..ad9f39f5f 100644 --- a/cmd/istio-cni/main.go +++ b/cmd/istio-cni/main.go @@ -58,7 +58,7 @@ type Kubernetes struct { type PluginConf struct { types.NetConf // You may wish to not nest this type RuntimeConfig *struct { - SampleConfig map[string]interface{} `json:"sample"` + // SampleConfig map[string]interface{} `json:"sample"` } `json:"runtimeConfig"` // This is the previous result, when called in the context of a chained @@ -159,8 +159,7 @@ func cmdAdd(args *skel.CmdArgs) error { nsSetupBinDir = conf.Kubernetes.CniBinDir } - var logger *logrus.Entry - logger = logrus.WithFields(logrus.Fields{ + logger := logrus.WithFields(logrus.Fields{ "ContainerID": args.ContainerID, "Pod": string(k8sArgs.K8S_POD_NAME), "Namespace": string(k8sArgs.K8S_POD_NAMESPACE), diff --git a/cmd/istio-cni/main_test.go b/cmd/istio-cni/main_test.go index 8ee0fd9ab..50c885e31 100644 --- a/cmd/istio-cni/main_test.go +++ b/cmd/istio-cni/main_test.go @@ -33,7 +33,6 @@ var ( currentVersion = "0.3.0" k8Args = "K8S_POD_NAMESPACE=istio-system;K8S_POD_NAME=testPodName" invalidVersion = "0.1.0" - excludeNamespace = "testExcludeNS" getKubePodInfoCalled = false nsenterFuncCalled = false diff --git a/cmd/istio-cni/redirect.go b/cmd/istio-cni/redirect.go index a71bbd72f..ab86e0261 100644 --- a/cmd/istio-cni/redirect.go +++ b/cmd/istio-cni/redirect.go @@ -79,22 +79,6 @@ type annotationParam struct { validator annotationValidationFunc } -func (v *annotationParam) getValueOrDefault(annotations map[string]string) string { - if val, ok := annotations[v.key]; ok { - return val - } - return v.defaultVal -} - -func (v *annotationParam) validate(annotations map[string]string) error { - if val, ok := annotations[v.key]; ok { - if err := v.validator(val); err != nil { - return fmt.Errorf("injection failed. Invalid value for annotation %s: %s. Error: %v", v.key, val, err) - } - } - return nil -} - func alwaysValidFunc(value string) error { return nil } @@ -228,14 +212,6 @@ func NewRedirect(ports []string, annotations map[string]string, logger *logrus.E return redir, nil } -func (rdrct *Redirect) setRedirectExcludeUID(noRedirectUID string) { - rdrct.noRedirectUID = noRedirectUID -} - -func (rdrct *Redirect) setRedirectTargetPort(tgtPort string) { - rdrct.targetPort = tgtPort -} - func (rdrct *Redirect) doRedirect(netns string) error { netnsArg := fmt.Sprintf("--net=%s", netns) nsSetupExecutable := fmt.Sprintf("%s/%s", nsSetupBinDir, nsSetupProg) diff --git a/lintconfig_base.json b/lintconfig_base.json index 247443a9a..abdfca6c9 100644 --- a/lintconfig_base.json +++ b/lintconfig_base.json @@ -17,8 +17,8 @@ "interfacer", "lll", "maligned", - "megacheck", "misspell", + "staticcheck", "structcheck", "unconvert", "varcheck", @@ -34,8 +34,8 @@ "interfacer": "error", "lll": "error", "maligned": "error", - "megacheck": "error", "misspell": "error", + "staticcheck": "error", "structcheck": "error", "unconvert": "error", "varcheck": "error", diff --git a/test/install_k8s_test.go b/test/install_k8s_test.go index 1ac037f4a..ffeb5684a 100644 --- a/test/install_k8s_test.go +++ b/test/install_k8s_test.go @@ -26,11 +26,9 @@ import ( ) var ( - PreConfDir = "data/pre" - ExpectedConfDir = "data/expected" - TestWorkDir, _ = os.Getwd() - Hub = "gcr.io/istio-release" - Tag = "master-latest-daily" + TestWorkDir, _ = os.Getwd() + Hub = "gcr.io/istio-release" + Tag = "master-latest-daily" ) type testCase struct { @@ -42,8 +40,8 @@ type testCase struct { } func doTest(testNum int, tc testCase, t *testing.T) { - os.Setenv("HUB", Hub) - os.Setenv("TAG", Tag) + _ = os.Setenv("HUB", Hub) + _ = os.Setenv("TAG", Tag) t.Logf("Running install CNI test with HUB=%s, TAG=%s", Hub, Tag) test.RunInstallCNITest(testNum, tc.preConfFile, tc.resultFileName, tc.expectedOutputFile, tc.expectedPostCleanFile, t) } From ee5e35ce0a12b249c8e407b06587f955327fe39d Mon Sep 17 00:00:00 2001 From: Tim Swanson Date: Tue, 19 Feb 2019 18:34:58 -0500 Subject: [PATCH 2/5] Test helm fetch since istio removal of istio-cni helm dependency --- .circleci/run_e2e.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.circleci/run_e2e.sh b/.circleci/run_e2e.sh index c4fe24cd3..4a3635ce1 100755 --- a/.circleci/run_e2e.sh +++ b/.circleci/run_e2e.sh @@ -3,11 +3,24 @@ CNI_HUB=${CNI_HUB:-istio} CNI_TAG=${CNI_TAG:-$1} +chartdir=$(pwd)/charts +mkdir ${chartdir} + cd /go/src/istio.io/istio HUB=${HUB:-gcr.io/istio-release} TAG=${TAG:-master-latest-daily} +helm init --client-only +helm repo add istio.io https://storage.googleapis.com/istio-release/releases/1.1.0-snapshot.6/charts +helm fetch --untar --untardir ${chartdir} istio.io/istio-cni + +helm template --values ${chartdir}/istio-cni/values.yaml --name=istio-cni --namespace=istio-system --set "excludeNamespaces={}" --set hub=${CNI_HUB} --set tag=${CNI_TAG} --set pullPolicy=IfNotPresent --set logLevel=${CNI_LOGLVL:-debug} ${chartdir}/istio-cni > istio-cni_install.yaml + +kubectl create ns istio-system +kubectl apply -f istio-cni_install.yaml + + HUB=${HUB} TAG=${TAG} make istioctl # Remove any pre-existing charts @@ -15,6 +28,6 @@ HUB=${HUB} TAG=${TAG} make istioctl # /go/out/linux_amd64/release/helm dep update --skip-refresh install/kubernetes/helm/istio # # Error: Unable to move current charts to tmp dir: rename /go/src/istio.io/istio/install/kubernetes/helm/istio/charts /go/src/istio.io/istio/install/kubernetes/helm/istio/tmpcharts: invalid cross-device link -rm -rf /go/src/istio.io/istio/install/kubernetes/helm/istio/charts +#rm -rf /go/src/istio.io/istio/install/kubernetes/helm/istio/charts HUB=${HUB} TAG=${TAG} ENABLE_ISTIO_CNI=true EXTRA_HELM_SETTINGS="--set istio-cni.excludeNamespaces={} --set istio-cni.hub=${CNI_HUB} --set istio-cni.tag=${CNI_TAG} --set istio-cni.pullPolicy=IfNotPresent --set istio-cni.logLevel=${CNI_LOGLVL:-debug}" E2E_ARGS="--kube_inject_configmap=istio-sidecar-injector ${SKIP_CLEAN:+ --skip_cleanup}" make test/local/auth/e2e_simple From a4a9b826711306576cbe36dcc807417a4a6e53bd Mon Sep 17 00:00:00 2001 From: Tim Swanson Date: Thu, 21 Feb 2019 14:01:07 -0500 Subject: [PATCH 3/5] Cleanup run_e2e.sh and install istio-cni in kube-system namespace --- .circleci/run_e2e.sh | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/.circleci/run_e2e.sh b/.circleci/run_e2e.sh index 4a3635ce1..7354624ef 100755 --- a/.circleci/run_e2e.sh +++ b/.circleci/run_e2e.sh @@ -8,26 +8,17 @@ mkdir ${chartdir} cd /go/src/istio.io/istio -HUB=${HUB:-gcr.io/istio-release} -TAG=${TAG:-master-latest-daily} - +# Install istio-cni prior to executing the Istio e2e test. Now that the helm chart for istio/istio no longer +# depends on the istio-cni chart, we need to explicitly do these steps. helm init --client-only helm repo add istio.io https://storage.googleapis.com/istio-release/releases/1.1.0-snapshot.6/charts helm fetch --untar --untardir ${chartdir} istio.io/istio-cni - -helm template --values ${chartdir}/istio-cni/values.yaml --name=istio-cni --namespace=istio-system --set "excludeNamespaces={}" --set hub=${CNI_HUB} --set tag=${CNI_TAG} --set pullPolicy=IfNotPresent --set logLevel=${CNI_LOGLVL:-debug} ${chartdir}/istio-cni > istio-cni_install.yaml - -kubectl create ns istio-system +helm template --values ${chartdir}/istio-cni/values.yaml --name=istio-cni --namespace=kube-system --set "excludeNamespaces={}" --set hub=${CNI_HUB} --set tag=${CNI_TAG} --set pullPolicy=IfNotPresent --set logLevel=${CNI_LOGLVL:-debug} ${chartdir}/istio-cni > istio-cni_install.yaml kubectl apply -f istio-cni_install.yaml +HUB=${HUB:-gcr.io/istio-release} +TAG=${TAG:-master-latest-daily} HUB=${HUB} TAG=${TAG} make istioctl -# Remove any pre-existing charts -# ...This seems to get around an issue seen with 1.1 where helm dep update fails with: -# /go/out/linux_amd64/release/helm dep update --skip-refresh install/kubernetes/helm/istio -# -# Error: Unable to move current charts to tmp dir: rename /go/src/istio.io/istio/install/kubernetes/helm/istio/charts /go/src/istio.io/istio/install/kubernetes/helm/istio/tmpcharts: invalid cross-device link -#rm -rf /go/src/istio.io/istio/install/kubernetes/helm/istio/charts - -HUB=${HUB} TAG=${TAG} ENABLE_ISTIO_CNI=true EXTRA_HELM_SETTINGS="--set istio-cni.excludeNamespaces={} --set istio-cni.hub=${CNI_HUB} --set istio-cni.tag=${CNI_TAG} --set istio-cni.pullPolicy=IfNotPresent --set istio-cni.logLevel=${CNI_LOGLVL:-debug}" E2E_ARGS="--kube_inject_configmap=istio-sidecar-injector ${SKIP_CLEAN:+ --skip_cleanup}" make test/local/auth/e2e_simple +HUB=${HUB} TAG=${TAG} ENABLE_ISTIO_CNI=true E2E_ARGS="--kube_inject_configmap=istio-sidecar-injector ${SKIP_CLEAN:+ --skip_cleanup}" make test/local/auth/e2e_simple From a5a971ac6da3f02ad4b01040f8a635db0ff8fc5c Mon Sep 17 00:00:00 2001 From: Tim Swanson Date: Thu, 21 Feb 2019 15:17:19 -0500 Subject: [PATCH 4/5] Use istio-cni helm chart from changeset --- .circleci/Dockerfile | 1 + .circleci/config.yml | 2 ++ .circleci/run_e2e.sh | 7 +++---- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.circleci/Dockerfile b/.circleci/Dockerfile index 3b09998de..9d5bda8ca 100644 --- a/.circleci/Dockerfile +++ b/.circleci/Dockerfile @@ -5,3 +5,4 @@ RUN git clone --branch $istio_branch https://github.com/istio/istio.git /go/src ADD run_e2e.sh /go/ +ADD helm /go/helm diff --git a/.circleci/config.yml b/.circleci/config.yml index 485cedb72..17c1032bc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -101,6 +101,8 @@ e2e-dind-test: &e2e-dind-test name: Build test-e2e container working_directory: /go/src/istio.io/cni command: | + mkdir .circleci/helm + cp -r deployments/kubernetes/install/helm/istio-cni .circleci/helm/ docker build --build-arg istio_branch=${ISTIO_BRANCH:-master} -t e2e-runner:latest .circleci - run: name: Get KDC diff --git a/.circleci/run_e2e.sh b/.circleci/run_e2e.sh index 7354624ef..a02856bc5 100755 --- a/.circleci/run_e2e.sh +++ b/.circleci/run_e2e.sh @@ -3,16 +3,15 @@ CNI_HUB=${CNI_HUB:-istio} CNI_TAG=${CNI_TAG:-$1} -chartdir=$(pwd)/charts -mkdir ${chartdir} +chartdir=/go/helm cd /go/src/istio.io/istio # Install istio-cni prior to executing the Istio e2e test. Now that the helm chart for istio/istio no longer # depends on the istio-cni chart, we need to explicitly do these steps. helm init --client-only -helm repo add istio.io https://storage.googleapis.com/istio-release/releases/1.1.0-snapshot.6/charts -helm fetch --untar --untardir ${chartdir} istio.io/istio-cni +#helm repo add istio.io https://storage.googleapis.com/istio-release/releases/1.1.0-snapshot.6/charts +#helm fetch --untar --untardir ${chartdir} istio.io/istio-cni helm template --values ${chartdir}/istio-cni/values.yaml --name=istio-cni --namespace=kube-system --set "excludeNamespaces={}" --set hub=${CNI_HUB} --set tag=${CNI_TAG} --set pullPolicy=IfNotPresent --set logLevel=${CNI_LOGLVL:-debug} ${chartdir}/istio-cni > istio-cni_install.yaml kubectl apply -f istio-cni_install.yaml From 5cc7b18ec2b602a88460f910cfebbe9a9569caa0 Mon Sep 17 00:00:00 2001 From: Tim Swanson Date: Thu, 21 Feb 2019 21:37:02 -0500 Subject: [PATCH 5/5] Fix comments in run_e2e.sh --- .circleci/run_e2e.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.circleci/run_e2e.sh b/.circleci/run_e2e.sh index a02856bc5..431c46536 100755 --- a/.circleci/run_e2e.sh +++ b/.circleci/run_e2e.sh @@ -3,15 +3,14 @@ CNI_HUB=${CNI_HUB:-istio} CNI_TAG=${CNI_TAG:-$1} +# Dockerfile copies the PR's istio/cni helm chart into /go/helm/istio-cni chartdir=/go/helm cd /go/src/istio.io/istio # Install istio-cni prior to executing the Istio e2e test. Now that the helm chart for istio/istio no longer -# depends on the istio-cni chart, we need to explicitly do these steps. -helm init --client-only -#helm repo add istio.io https://storage.googleapis.com/istio-release/releases/1.1.0-snapshot.6/charts -#helm fetch --untar --untardir ${chartdir} istio.io/istio-cni +# depends on the istio-cni chart, we need to explicitly do this as a prereq for installing Istio +# (the e2e_simple test installs Istio). helm template --values ${chartdir}/istio-cni/values.yaml --name=istio-cni --namespace=kube-system --set "excludeNamespaces={}" --set hub=${CNI_HUB} --set tag=${CNI_TAG} --set pullPolicy=IfNotPresent --set logLevel=${CNI_LOGLVL:-debug} ${chartdir}/istio-cni > istio-cni_install.yaml kubectl apply -f istio-cni_install.yaml