Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests and update to quickstart guide #354

Merged
merged 26 commits into from Aug 5, 2019
Merged
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
df3a495
Added a test for GetLoggingLevel
nicklesimba Jun 10, 2019
536e2d6
Added up to 96% coverage for checkpoint.go
nicklesimba Jun 10, 2019
a340aa7
Improved coverage of checkpoint.go to 96.4%
nicklesimba Jun 20, 2019
41af2c8
Improved coverage of checkpoint.go to 96.4%
nicklesimba Jun 20, 2019
762f34f
Adding changes so i will have them saved for my remote fork thingy
nicklesimba Jun 20, 2019
f71a4fb
Fixed unit tests in checkpoint_test.go and conf_test.go
nicklesimba Jun 21, 2019
12f9918
Removed unnecessary comments
nicklesimba Jun 27, 2019
78d32b6
improved conf code coverage by an amount that is greater than 0!
nicklesimba Jun 27, 2019
86e9f81
improved coverage, but line 144 of conf.go needs a look
nicklesimba Jul 2, 2019
ae332ad
Added unit tests to multus and types, also fixed a bug in conf.go
nicklesimba Jul 9, 2019
a67676e
added label to import types/020 in types.go
nicklesimba Jul 10, 2019
fc5fd6a
hopefully resolved merge conflicts
nicklesimba Jul 11, 2019
03164f2
increased code coverage in multus.go and conf.go, also added bug fixe…
nicklesimba Jul 11, 2019
78b69b9
review comments addressed
nicklesimba Jul 12, 2019
d3c1124
addressed all comments in review
nicklesimba Jul 12, 2019
5bd3cad
Updated testing.go with better comments
nicklesimba Jul 15, 2019
3ae61bd
changed 'thejohn' to '_not_type' for readability
nicklesimba Jul 17, 2019
4630f85
added additional unit tests
nicklesimba Jul 25, 2019
f9a1444
fixing merge errors
nicklesimba Jul 26, 2019
afeed8c
updating tests branch with most recent additions
nicklesimba Jul 26, 2019
850065e
added tests to kubeletclient.go
nicklesimba Jul 26, 2019
98ba6dc
added more unit tests to k8sclient.go and kubeletclient.go
nicklesimba Jul 30, 2019
ca2c053
Merge branch 'master' into tests
nicklesimba Jul 30, 2019
f95ae1c
Added network status annotations section to quickstart and added more…
nicklesimba Aug 1, 2019
6b43573
Merge branch 'master' into tests
nicklesimba Aug 1, 2019
b4a60cf
made changes to tests based on code review
nicklesimba Aug 5, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -176,6 +176,33 @@ You should note that there's 3 interfaces:
* `eth0` our default network
* `net1` the new interface we created with the macvlan configuration.

### Network Status Annotations

For additional confirmation, use `kubectl describe pod samplepod` and there will be an annotations section, similar to the following:

```
Annotations: k8s.v1.cni.cncf.io/networks: macvlan-conf
k8s.v1.cni.cncf.io/networks-status:
[{
"name": "cbr0",
"ips": [
"10.244.1.73"
],
"default": true,
"dns": {}
},{
"name": "macvlan-conf",
"interface": "net1",
"ips": [
"192.168.1.205"
],
"mac": "86:1d:96:ff:55:0d",
"dns": {}
}]
```

This metadata tells us that we have two CNI plugins running successfully.

### What if I want more interfaces?

You can add more interfaces to a pod by creating more custom resources and then referring to them in pod's annotation. You can also reuse configurations, so for example, to attach two macvlan interfaces to a pod, you could create a pod like so:
@@ -198,4 +225,4 @@ EOF

Note that the annotation now reads `k8s.v1.cni.cncf.io/networks: macvlan-conf,macvlan-conf`. Where we have the same configuration used twice, separated by a comma.

If you were to create another custom resource with the name `foo` you could use that such as: `k8s.v1.cni.cncf.io/networks: foo,macvlan-conf`, and use any number of attachments.
If you were to create another custom resource with the name `foo` you could use that such as: `k8s.v1.cni.cncf.io/networks: foo,macvlan-conf`, and use any number of attachments.
@@ -22,6 +22,8 @@ import (
"path/filepath"
"testing"

types020 "github.com/containernetworking/cni/pkg/types/020"
testhelpers "github.com/intel/multus-cni/testing"
testutils "github.com/intel/multus-cni/testing"

"github.com/containernetworking/cni/pkg/skel"
@@ -611,4 +613,187 @@ var _ = Describe("k8sclient operations", func() {
Expect(err).To(MatchError("GetPodNetwork: namespace isolation violation: podnamespace: test / target namespace: kube-system"))

})

It("Returns proper error message", func() {
// getPodNetwork will give us the error, then this test will use that error on the top func boi
err := &NoK8sNetworkError{"no kubernetes network found"}
Expect(err.Error()).To(Equal("no kubernetes network found"))
})

It("Sets pod network annotations without error", func() {
fakePod := testutils.NewFakePod("testpod", "kube-system/net1", "")

net1 := `{
"name": "net1",
"type": "mynet",
"cniVersion": "0.2.0"
}`

args := &skel.CmdArgs{
Args: fmt.Sprintf("K8S_POD_NAME=%s;K8S_POD_NAMESPACE=%s", fakePod.ObjectMeta.Name, fakePod.ObjectMeta.Namespace),
}

fKubeClient := testutils.NewFakeKubeClient()
fKubeClient.AddPod(fakePod)
fKubeClient.AddNetConfig("kube-system", "net1", net1)

kubeClient, err := GetK8sClient("", fKubeClient)
Expect(err).NotTo(HaveOccurred())
k8sArgs, err := GetK8sArgs(args)
Expect(err).NotTo(HaveOccurred())

pod, err := kubeClient.GetPod(string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_NAME))
Expect(err).NotTo(HaveOccurred())

networkstatus := "test status"
_, err = setPodNetworkAnnotation(kubeClient, "test", pod, networkstatus)
Expect(err).NotTo(HaveOccurred())
})

/* Still figuring this one out. need to make "setPodNetworkAnnotation throw an error */

This comment has been minimized.

Copy link
@s1061123

s1061123 Aug 2, 2019

Collaborator

Golang prefers to use '//' than '/* */' for comments. Could you please change it?

It("Fails to set pod network annotations without error", func() {
fakePod := testutils.NewFakePod("testpod", "kube-system/net1", "")

net1 := `{
"name": "net1",
"type": "mynet",
"cniVersion": "0.2.0"
}`

args := &skel.CmdArgs{
Args: fmt.Sprintf("K8S_POD_NAME=%s;K8S_POD_NAMESPACE=%s", fakePod.ObjectMeta.Name, fakePod.ObjectMeta.Namespace),
}

fKubeClient := testutils.NewFakeKubeClient()
fKubeClient.AddPod(fakePod)
fKubeClient.AddNetConfig("kube-system", "net1", net1)

kubeClient, err := GetK8sClient("", fKubeClient)
Expect(err).NotTo(HaveOccurred())
k8sArgs, err := GetK8sArgs(args)
Expect(err).NotTo(HaveOccurred())

pod, err := kubeClient.GetPod(string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_NAME))
Expect(err).NotTo(HaveOccurred())

networkstatus := "test status"
_, err = setPodNetworkAnnotation(kubeClient, "test", pod, networkstatus)
Expect(err).NotTo(HaveOccurred())
})

It("Sets network status without error", func() {
result := &types020.Result{
CNIVersion: "0.2.0",
IP4: &types020.IPConfig{
IP: *testhelpers.EnsureCIDR("1.1.1.2/24"),
},
}

conf := `{
"name": "node-cni-network",
"type": "multus",
"kubeconfig": "/etc/kubernetes/node-kubeconfig.yaml",
"delegates": [{
"type": "weave-net"
}],
"runtimeConfig": {
"portMappings": [
{"hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
}
}`

delegate, err := types.LoadDelegateNetConf([]byte(conf), nil, "0000:00:00.0")
Expect(err).NotTo(HaveOccurred())

delegateNetStatus, err := types.LoadNetworkStatus(result, delegate.Conf.Name, delegate.MasterPlugin)
GinkgoT().Logf("delegateNetStatus %+v\n", delegateNetStatus)
Expect(err).NotTo(HaveOccurred())

netstatus := []*types.NetworkStatus{delegateNetStatus}

fakePod := testutils.NewFakePod("testpod", "kube-system/net1", "")

netConf, err := types.LoadNetConf([]byte(conf))
Expect(err).NotTo(HaveOccurred())

net1 := `{
"name": "net1",
"type": "mynet",
"cniVersion": "0.2.0"
}`

args := &skel.CmdArgs{
Args: fmt.Sprintf("K8S_POD_NAME=%s;K8S_POD_NAMESPACE=%s", fakePod.ObjectMeta.Name, fakePod.ObjectMeta.Namespace),
}

fKubeClient := testutils.NewFakeKubeClient()
fKubeClient.AddPod(fakePod)
fKubeClient.AddNetConfig("kube-system", "net1", net1)

kubeClient, err := GetK8sClient("", fKubeClient)
Expect(err).NotTo(HaveOccurred())
k8sArgs, err := GetK8sArgs(args)
Expect(err).NotTo(HaveOccurred())

err = SetNetworkStatus(kubeClient, k8sArgs, netstatus, netConf)
Expect(err).NotTo(HaveOccurred())
})

It("Fails to set network status without error", func() {
result := &types020.Result{
CNIVersion: "0.2.0",
IP4: &types020.IPConfig{
IP: *testhelpers.EnsureCIDR("1.1.1.2/24"),
},
}

conf := `{
"name": "node-cni-network",
"type": "multus",
"kubeconfig": "/etc/kubernetes/node-kubeconfig.yaml",
"delegates": [{
"type": "weave-net"
}],
"runtimeConfig": {
"portMappings": [
{"hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
}
}`

delegate, err := types.LoadDelegateNetConf([]byte(conf), nil, "0000:00:00.0")

This comment has been minimized.

Copy link
@s1061123

s1061123 Aug 2, 2019

Collaborator

In case of non-sriov case, 3rd argument of LoadDelegateNetConf() could be "" (empty) instead of "0000:00:00.0".

This comment has been minimized.

Copy link
@nicklesimba

nicklesimba Aug 5, 2019

Author Contributor

I changed this in my most recent commit, but I'm not sure what "non-sriov case" means. Can you explain so I will know where/why such changes should be made for future? Thank you

Expect(err).NotTo(HaveOccurred())

delegateNetStatus, err := types.LoadNetworkStatus(result, delegate.Conf.Name, delegate.MasterPlugin)
GinkgoT().Logf("delegateNetStatus %+v\n", delegateNetStatus)
Expect(err).NotTo(HaveOccurred())

netstatus := []*types.NetworkStatus{delegateNetStatus}

fakePod := testutils.NewFakePod("testpod", "kube-system/net1", "")

netConf, err := types.LoadNetConf([]byte(conf))
Expect(err).NotTo(HaveOccurred())

net1 := `{
"name": "net1",
"type": "mynet",
"cniVersion": "0.2.0"
}`

args := &skel.CmdArgs{
Args: fmt.Sprintf("K8S_POD_NAME=%s;K8S_POD_NAMESPACE=%s", fakePod.ObjectMeta.Name, fakePod.ObjectMeta.Namespace),
}

fKubeClient := testutils.NewFakeKubeClient()
fKubeClient.AddPod(fakePod)
fKubeClient.AddNetConfig("kube-system", "net1", net1)

k8sArgs, err := GetK8sArgs(args)
Expect(err).NotTo(HaveOccurred())

err = SetNetworkStatus(nil, k8sArgs, netstatus, netConf)
Expect(err).To(HaveOccurred())
})
})
@@ -115,6 +115,15 @@ var _ = Describe("Kubelet resource endpoint data read operations", func() {
_, err := GetResourceClient()
Expect(err).NotTo(HaveOccurred())
})

It("should fail with missing file", func() {
// os.Mkdir("tempdir", 0644)

This comment has been minimized.

Copy link
@s1061123

s1061123 Aug 2, 2019

Collaborator

Could you remove commented out code (if it is not necessary)?

kubeletSocket = "Andrew Bays"

This comment has been minimized.

Copy link
@s1061123

s1061123 Aug 2, 2019

Collaborator

Good example to us but not so universal 😆 We may use simple string for that, like "sampleSocketString".

// os.Rename(socketName, "tempdir/tempname.sock")
_, err := GetResourceClient()
Expect(err).To(HaveOccurred())
// os.Rename("tempdir/tempname.sock", socketName)
})
})

Context("GetPodResourceMap() with valid pod name and namespace", func() {
@@ -146,6 +155,41 @@ var _ = Describe("Kubelet resource endpoint data read operations", func() {
Expect(resourceMap).ShouldNot(BeNil())
Expect(resourceMap).To(Equal(outputRMap))
})

It("should return no error with empty socket", func() {
podUID := k8sTypes.UID("970a395d-bb3b-11e8-89df-408d5c537d23")
fakePod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod-name",
Namespace: "pod-namespace",
UID: podUID,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "container-name",
},
},
},
}
kubeletSocket = ""
client, err := getKubeletClient()
Expect(err).NotTo(HaveOccurred())

outputRMap := map[string]*mtypes.ResourceInfo{
"resource": &mtypes.ResourceInfo{DeviceIDs: []string{"dev0", "dev1"}},
}
resourceMap, err := client.GetPodResourceMap(fakePod)
Expect(err).NotTo(HaveOccurred())
Expect(resourceMap).ShouldNot(BeNil())
Expect(resourceMap).To(Equal(outputRMap))
})

It("should return an error with garbage socket value", func() {
kubeletSocket = "/badfilepath!?//"
_, err := getKubeletClient()
Expect(err).To(HaveOccurred())
})
})

Context("GetPodResourceMap() with empty podname", func() {
@@ -45,6 +45,12 @@ var _ = Describe("logging operations", func() {
// check file existance
})

It("Check file setter with bad filepath", func() {
SetLogFile("/invalid/filepath")
Expect(loggingFp).NotTo(Equal(nil))
// check file existance
})

It("Check loglevel setter", func() {
SetLogLevel("debug")
Expect(loggingLevel).To(Equal(DebugLevel))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.