Skip to content
This repository was archived by the owner on Jan 23, 2022. It is now read-only.

Fix gometalinter 3.0 errors and update e2e bringup with istio/istio helm changes #88

Merged
john-a-joyce merged 5 commits into from
Feb 22, 2019
Merged

Fix gometalinter 3.0 errors and update e2e bringup with istio/istio helm changes #88

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eed389c
Fix gometalinter 3.0 errors
tiswanso Feb 19, 2019
ee5e35c
Test helm fetch since istio removal of istio-cni helm dependency
tiswanso Feb 19, 2019
a4a9b82
Cleanup run_e2e.sh and install istio-cni in kube-system namespace
tiswanso Feb 21, 2019
a5a971a
Use istio-cni helm chart from changeset
tiswanso Feb 21, 2019
5cc7b18
Fix comments in run_e2e.sh
tiswanso Feb 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .circleci/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 11 additions & 8 deletions .circleci/run_e2e.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@
CNI_HUB=${CNI_HUB:-istio}

@john-a-joyce john-a-joyce Feb 20, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a good default value

CNI_TAG=${CNI_TAG:-$1}

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

@john-a-joyce john-a-joyce Feb 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should remove the comments here. Thinking about this more the tests here should be only about using the istio source code not also validating if the the CNI charts have been properly pushed to the istio charts repo. The tests on the istio side can confirm that. In this repo it is probably cleaner if it tests the CNI code AND the CNI charts at the tip of the branch but pull in the Istio code to confirm that.

@tiswanso tiswanso Feb 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment just to remove lines 13 & 14 or to remove all the other comments above? Lines 10 & 11 are still valid comments. I could reword to explain that we're testing with the istio-cni chart from the PR under-test.

#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

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
5 changes: 2 additions & 3 deletions cmd/istio-cni/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
1 change: 0 additions & 1 deletion cmd/istio-cni/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions cmd/istio-cni/redirect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lintconfig_base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"interfacer",
"lll",
"maligned",
"megacheck",
"misspell",
"staticcheck",
"structcheck",
"unconvert",
"varcheck",
Expand All @@ -34,8 +34,8 @@
"interfacer": "error",
"lll": "error",
"maligned": "error",
"megacheck": "error",
"misspell": "error",
"staticcheck": "error",
"structcheck": "error",
"unconvert": "error",
"varcheck": "error",
Expand Down
12 changes: 5 additions & 7 deletions test/install_k8s_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down