install-cni: properly detect CNI conf file (match kubelet approach) (#96

)

* install-cni: properly detect CNI conf file (match kubelet approach)

- Add test cases for skipping invalid .conf and .conflist files

* Allow cniVersion to be unset in .conflist

deployments/kubernetes/install/scripts/install-cni.sh

@@ -20,25 +20,37 @@ function rm_bin_files() {
   echo "Removing existing binaries"
   rm -f /host/opt/cni/bin/istio-cni /host/opt/cni/bin/istio-iptables.sh
 }
-
-# The directory on the host where CNI networks are installed. Defaults to
-# /etc/cni/net.d, but can be overridden by setting CNI_NET_DIR.  This is used
-# for populating absolute paths in the CNI network config to assets
-# which are installed in the CNI network config directory.
-HOST_CNI_NET_DIR=${CNI_NET_DIR:-/etc/cni/net.d}
-MOUNTED_CNI_NET_DIR=${MOUNTED_CNI_NET_DIR:-/host/etc/cni/net.d}
-
-CNI_CONF_NAME_OVERRIDE=${CNI_CONF_NAME:-}
-
-# default to first file in `ls` output
-# if dir is empty, default to a filename that is not likely to be lexicographically first in the dir
-CNI_CONF_NAME=${CNI_CONF_NAME:-$(ls ${MOUNTED_CNI_NET_DIR} | head -n 1)}
-CNI_CONF_NAME=${CNI_CONF_NAME:-YYY-istio-cni.conflist}
-KUBECFG_FILE_NAME=${KUBECFG_FILE_NAME:-ZZZ-istio-cni-kubeconfig}
-CFGCHECK_INTERVAL=${CFGCHECK_INTERVAL:-1}
+# find_cni_conf_file
+#   Finds the CNI config file in the mounted CNI config dir.
+#   - Follows the same semantics as kubelet
+#     https://github.com/kubernetes/kubernetes/blob/954996e231074dc7429f7be1256a579bedd8344c/pkg/kubelet/dockershim/network/cni/cni.go#L144-L184
+#
+function find_cni_conf_file() {
+    cni_cfg=
+    for cfgf in $(ls ${MOUNTED_CNI_NET_DIR}); do
+        if [ "${cfgf: -5}" == ".conf" ]; then
+            # check if it's a valid CNI .conf file
+            type=$(cat ${MOUNTED_CNI_NET_DIR}/${cfgf} | jq 'has("type")' 2>/dev/null)
+            if [[ "${type}" == "true" ]]; then
+                cni_cfg=${cfgf}
+                break
+            fi
+        elif [ "${cfgf: -9}" == ".conflist" ]; then
+            # Check that the file is json and has top level "name" and "plugins" keys
+            # NOTE: "cniVersion" key is not required by libcni (kubelet) to be present
+            name=$(cat ${MOUNTED_CNI_NET_DIR}/${cfgf} | jq 'has("name")' 2>/dev/null)
+            plugins=$(cat ${MOUNTED_CNI_NET_DIR}/${cfgf} | jq 'has("plugins")' 2>/dev/null)
+            if [[ "${name}" == "true" && "${plugins}" == "true" ]]; then
+                cni_cfg=${cfgf}
+                break
+            fi
+        fi
+    done
+    echo "$cni_cfg"
+}
 
 function check_install() {
-  cfgfile_nm=$(ls ${MOUNTED_CNI_NET_DIR} | head -n 1)
+  cfgfile_nm=$(find_cni_conf_file)
   if [[ "${cfgfile_nm}" != "${CNI_CONF_NAME}" ]]; then
     if [[ "${CNI_CONF_NAME_OVERRIDE}" != "" ]]; then
        # Install was run with overridden cni config file so don't error out on the preempt check.
@@ -77,6 +89,23 @@ function cleanup() {
   echo "Exiting."
 }
 
+# The directory on the host where CNI networks are installed. Defaults to
+# /etc/cni/net.d, but can be overridden by setting CNI_NET_DIR.  This is used
+# for populating absolute paths in the CNI network config to assets
+# which are installed in the CNI network config directory.
+HOST_CNI_NET_DIR=${CNI_NET_DIR:-/etc/cni/net.d}
+MOUNTED_CNI_NET_DIR=${MOUNTED_CNI_NET_DIR:-/host/etc/cni/net.d}
+
+CNI_CONF_NAME_OVERRIDE=${CNI_CONF_NAME:-}
+
+# default to first file in `ls` output
+# if dir is empty, default to a filename that is not likely to be lexicographically first in the dir
+CNI_CONF_NAME=${CNI_CONF_NAME:-$(find_cni_conf_file)}
+CNI_CONF_NAME=${CNI_CONF_NAME:-YYY-istio-cni.conflist}
+KUBECFG_FILE_NAME=${KUBECFG_FILE_NAME:-ZZZ-istio-cni-kubeconfig}
+CFGCHECK_INTERVAL=${CFGCHECK_INTERVAL:-1}
+
+
 trap cleanup EXIT
 
 # Clean up any existiang binaries / config / assets.

deployments/kubernetes/install/test/data/pre/bad_minikube_cni.conf

@@ -0,0 +1,18 @@
+{
+  "name": "rkt.kubernetes.io",
+  "bridge": "mybridge",
+  "mtu": 1460,
+  "addIf": "true",
+  "isGateway": true,
+  "ipMasq": true,
+  "ipam": {
+    "type": "host-local",
+    "subnet": "10.1.0.0/16",
+    "gateway": "10.1.0.1",
+    "routes": [
+      {
+        "dst": "0.0.0.0/0"
+      }
+    ]
+  }
+}

deployments/kubernetes/install/test/data/pre/non_json.conf

@@ -0,0 +1,3 @@
+"This is not json"
+
+Maybe it magically parses but shouldn't

deployments/kubernetes/install/test/data/pre/noname_calico.conflist

@@ -0,0 +1,27 @@
+{
+  "cniVersion": "0.3.0",
+  "plugins": [
+    {
+      "type": "calico",
+      "etcd_endpoints": "http://10.110.0.136:6666",
+      "log_level": "info",
+      "mtu": 1500,
+      "ipam": {
+        "type": "calico-ipam"
+      },
+      "policy": {
+        "type": "k8s"
+      },
+      "kubernetes": {
+        "kubeconfig": "/etc/cni/net.d/calico-kubeconfig"
+      }
+    },
+    {
+      "type": "portmap",
+      "snat": true,
+      "capabilities": {
+        "portMappings": true
+      }
+    }
+  ]
+}

deployments/kubernetes/install/test/data/pre/noplugins_calico.conflist

@@ -0,0 +1,28 @@
+{
+  "name": "k8s-pod-network",
+  "cniVersion": "0.3.0",
+  "pugins": [
+    {
+      "type": "calico",
+      "etcd_endpoints": "http://10.110.0.136:6666",
+      "log_level": "info",
+      "mtu": 1500,
+      "ipam": {
+        "type": "calico-ipam"
+      },
+      "policy": {
+        "type": "k8s"
+      },
+      "kubernetes": {
+        "kubeconfig": "/etc/cni/net.d/calico-kubeconfig"
+      }
+    },
+    {
+      "type": "portmap",
+      "snat": true,
+      "capabilities": {
+        "portMappings": true
+      }
+    }
+  ]
+}

deployments/kubernetes/install/test/data/pre/nover_calico.conflist

@@ -0,0 +1,27 @@
+{
+  "name": "k8s-pod-network",
+  "plugins": [
+    {
+      "type": "calico",
+      "etcd_endpoints": "http://10.110.0.136:6666",
+      "log_level": "info",
+      "mtu": 1500,
+      "ipam": {
+        "type": "calico-ipam"
+      },
+      "policy": {
+        "type": "k8s"
+      },
+      "kubernetes": {
+        "kubeconfig": "/etc/cni/net.d/calico-kubeconfig"
+      }
+    },
+    {
+      "type": "portmap",
+      "snat": true,
+      "capabilities": {
+        "portMappings": true
+      }
+    }
+  ]
+}

deployments/kubernetes/install/test/install_cni.go

@@ -16,6 +16,7 @@ package test
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -106,11 +107,12 @@ func rm(dir string, t *testing.T) {
 
 // populateTempDirs populates temporary test directories with golden files and
 // other related configuration.
-func populateTempDirs(wd, tempCNIConfDir, tempK8sSvcAcctDir string, t *testing.T) {
+func populateTempDirs(wd string, cniDirOrderedFiles []string, tempCNIConfDir, tempK8sSvcAcctDir string, t *testing.T) {
 	t.Logf("Pre-populating working dirs")
-	for _, f := range ls(wd+cniConfSubDir, t) {
-		t.Logf("Copying %v into temp config dir %v", f, tempCNIConfDir)
-		cp(wd+cniConfSubDir+f, tempCNIConfDir+"/"+f, t)
+	for i, f := range cniDirOrderedFiles {
+		destFilenm := fmt.Sprintf("0%d-%s", i, f)
+		t.Logf("Copying %v into temp config dir %v/%s", f, tempCNIConfDir, destFilenm)
+		cp(wd+cniConfSubDir+f, tempCNIConfDir+"/"+destFilenm, t)
 	}
 	for _, f := range ls(wd+k8sSvcAcctSubDir, t) {
 		t.Logf("Copying %v into temp k8s serviceaccount dir %v", f, tempK8sSvcAcctDir)
@@ -121,7 +123,7 @@ func populateTempDirs(wd, tempCNIConfDir, tempK8sSvcAcctDir string, t *testing.T
 
 // startDocker starts a test Docker container and runs the install-cni.sh script.
 func startDocker(testNum int, wd, tempCNIConfDir, tempCNIBinDir,
-	tempK8sSvcAcctDir string, t *testing.T) string {
+	tempK8sSvcAcctDir, cniConfFileName string, t *testing.T) string {
 
 	dockerImage := env("HUB", "") + "/install-cni:" + env("TAG", "")
 	errFileName := path.Dir(tempCNIConfDir) + "/docker_run_stderr"
@@ -136,8 +138,8 @@ func startDocker(testNum int, wd, tempCNIConfDir, tempCNIBinDir,
 		"--env-file", wd + "/data/env_vars.sh",
 		"-e", cniNetworkConfigName,
 	}
-	if _, ok := os.LookupEnv(cniConfName); ok {
-		args = append(args, "-e", cniConfName)
+	if cniConfFileName != "" {
+		args = append(args, "-e", cniConfName+"="+cniConfFileName)
 	}
 	args = append(args, dockerImage)
 	args = append(args, "/install-cni.sh")
@@ -232,14 +234,15 @@ func doTest(testNum int, wd, preConfFile, resultFileName, expectedOutputFile,
 	t.Logf("Test %v: prior cni-conf='%v', expected result='%v'", testNum, preConfFile, resultFileName)
 
 	// Don't set the CNI conf file env var if preConfFile is not set
+	var envPreconf string
 	if preConfFile != "NONE" {
-		setEnv(cniConfName, preConfFile, t)
+		envPreconf = preConfFile
 	} else {
 		preConfFile = resultFileName
 	}
 	setEnv(cniNetworkConfigName, cniNetworkConfig, t)
 
-	containerID := startDocker(testNum, wd, tempCNIConfDir, tempCNIBinDir, tempK8sSvcAcctDir, t)
+	containerID := startDocker(testNum, wd, tempCNIConfDir, tempCNIBinDir, tempK8sSvcAcctDir, envPreconf, t)
 	time.Sleep(10 * time.Second)
 
 	compareConfResult(testWorkRootDir, tempCNIConfDir, resultFileName, expectedOutputFile, t)
@@ -267,7 +270,7 @@ func doTest(testNum int, wd, preConfFile, resultFileName, expectedOutputFile,
 // prefix. This func is only meant to be invoked programmatically. A separate
 // install_cni_test.go file exists for executing this test.
 func RunInstallCNITest(testNum int, preConfFile, resultFileName, expectedOutputFile,
-	expectedPostCleanFile string, t *testing.T) {
+	expectedPostCleanFile string, cniConfDirOrderedFiles []string, t *testing.T) {
 
 	wd := pwd(t)
 	// root := filepath.Dir(wd)
@@ -284,7 +287,7 @@ func RunInstallCNITest(testNum int, preConfFile, resultFileName, expectedOutputF
 	t.Logf("conf-dir=%v; bin-dir=%v; k8s-serviceaccount=%v", tempCNIConfDir,
 		tempCNIBinDir, tempK8sSvcAcctDir)
 
-	populateTempDirs(wd, tempCNIConfDir, tempK8sSvcAcctDir, t)
+	populateTempDirs(wd, cniConfDirOrderedFiles, tempCNIConfDir, tempK8sSvcAcctDir, t)
 	doTest(testNum, wd, preConfFile, resultFileName, expectedOutputFile,
 		expectedPostCleanFile, tempCNIConfDir, tempCNIBinDir, tempK8sSvcAcctDir,
 		testWorkRootDir, t)

deployments/kubernetes/install/test/install_cni_test.go

@@ -16,17 +16,20 @@ package test
 
 import (
 	"flag"
+	"strings"
 	"testing"
 )
 
 var (
-	preConfFlag        = flag.String("preconf", "", "pre_conf")
-	resultFileNameFlag = flag.String("resultfilename", "", "result_filename")
-	expectedConfFlag   = flag.String("expectedconf", "", "expected_conf")
-	expectedCleanFlag  = flag.String("expectedclean", "", "expected_clean")
+	preConfFlag         = flag.String("preconf", "", "pre_conf")
+	resultFileNameFlag  = flag.String("resultfilename", "", "result_filename")
+	expectedConfFlag    = flag.String("expectedconf", "", "expected_conf")
+	expectedCleanFlag   = flag.String("expectedclean", "", "expected_clean")
+	confDirOrderedFiles = flag.String("confOrderedFiles", "", "conf_ordered_files")
 )
 
 // TestInstallCNI consumes CLI flags and runs the install CNI test.
 func TestInstallCNI(t *testing.T) {
-	RunInstallCNITest(1, *preConfFlag, *resultFileNameFlag, *expectedConfFlag, *expectedCleanFlag, t)
+	RunInstallCNITest(1, *preConfFlag, *resultFileNameFlag, *expectedConfFlag,
+		*expectedCleanFlag, strings.Split(*confDirOrderedFiles, ","), t)
 }