Skip to content
This repository has been archived by the owner on Sep 24, 2021. It is now read-only.

Cni network plugin for frakti #61

Closed
wants to merge 2 commits into from

Conversation

YaoZengzeng
Copy link
Contributor

@YaoZengzeng YaoZengzeng commented Jan 4, 2017

Cni network plugin for frakti

Signed-off-by: Yao Zengzeng <yaozengzeng@zju.edu.cn>
Signed-off-by: Yao Zengzeng <yaozengzeng@zju.edu.cn>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 4, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@resouer
Copy link
Contributor

resouer commented Jan 5, 2017

Seems vendor dose not updated properly? @YaoZengzeng

@resouer resouer requested a review from feiskyer January 5, 2017 01:59
@feiskyer feiskyer self-assigned this Jan 5, 2017
return nil, nil, err
}

return &Runtime{client: hyperClient, streamingServer: streamingServer, networkPlugin: networkPlugin}, streamingServer, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: lines too long, consider format codes

const (
DefaultInterfaceName = "eth0"
DefaultNetDir = "/etc/cni/net.d"
DefaultCNIDir = "/opt/cni/bin"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make these two path configurable?

func InitCNI() (*CNINetworkPlugin, error) {
plugin := &CNINetworkPlugin{}

plugin.sandboxNs = make(map[string]*sandboxNetNS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Read and check network config here? So we could fail fast and don't need to read and check again and again later.

}

rt := &libcni.RuntimeConf{
ContainerID: "cni",
Copy link
Contributor

Choose a reason for hiding this comment

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

Use sandboxID here?

}

for _, link := range links {
if link.Type() != "veth" {
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to support other interface types, it's better to omit only lo here, e.g. interface name == lo

}

type sandboxNetNS struct {
netns ns.NetNS
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to persist sandbox netns, e.g. in pod labels or write to local files.

@@ -118,6 +156,11 @@ func (h *Runtime) DeletePodSandbox(podSandboxID string) error {
return err
}

err = h.networkPlugin.TeardownPodNetwork(podSandboxID)
Copy link
Contributor

Choose a reason for hiding this comment

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

network should also be cleared up in StopPodSandbox

@YaoZengzeng
Copy link
Contributor Author

This PR's code quality is not good enough, so close it and open another one #62 @resouer @feiskyer

@YaoZengzeng YaoZengzeng closed this Jan 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants