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

Epif cleanup #16

Merged
merged 4 commits into from
Feb 21, 2015
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 1 addition & 3 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ type EndpointDriver interface {
CreateEndpoint(id string) error
DeleteEndpoint(value string) error
MakeEndpointAddress() (*Address, error)
GetEndpointContainerContext(id string) (*ContainerEpContext, error)
GetContainerEpContextByContName(contName string) ([]ContainerEpContext, error)
UpdateContainerId(id string, contId string) error
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to move the UpdateContainerId() out as well? Please ignore, if you were planning it for later.

Copy link
Author

Choose a reason for hiding this comment

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

Update ContainerID is just a name of the function, there is nothing specific to container runtime there. I was hoping to not do updates to Ep record from oustide drivers code.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, it does imply that Endpoint driver Interface has something to do with containers, which is not the case.

Looks like this API updates the container-id in the EP oper-state in current implementation, should we just do it as part of attach/detach endpoint in the crt's logic? this looks more like some config meta for container runtime than network programming driver.

}

Expand Down Expand Up @@ -118,7 +116,7 @@ type ContainerEpContext struct {
DefaultGw string
}

type ContainerDriver interface {
type ContainerIf interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to move ContainerIf interface out of core as well (may be move it to crt.go?), so that core just contains the APIs the external code either uses (i.e. Plugin interface) or implements (Driver interface(s)).

Also suggest to add some doc-comment strings explaining the use of this interface, as in how is it called; how will it differ between runtimes etc.

Copy link
Author

Choose a reason for hiding this comment

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

This one was interesting. I did have it outside in crt/crt.go but circular dependency between crt/docker and crt made me move this to core.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thinking more, crt is different from docker dir contents in a way that crt defines something like Plugin that makes use of implementation of a containerIf. May be we should add a separate directory for container-clients and move the ContainerIf (to a file like containerc/containerc.go and docker directory there (i.e. container/docker). Since the interface is specific to container-clients we can also name is ContainerC or ContainerClient instead of ContainerIf.

Also while doing this may be we should also move our current implementation of netd under docker/directory as well, since the netplugin binary is docker specific. We can name the directory docker-net or something in that case.

// Container driver provides a mechanism to interface with container
// runtime to handle events create, start, die, stop, pause, etc.
Driver
Expand Down
107 changes: 107 additions & 0 deletions crt/crt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/***
Copyright 2014 Cisco Systems Inc. All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package crt

import (
"encoding/json"
"errors"
"reflect"

"github.com/contiv/netplugin/core"
"github.com/contiv/netplugin/crt/docker"
)

type Crt struct {
ContainerIf core.ContainerIf
}

type CrtConfig struct {
Crt struct {
Type string
}
}

type ContainerIfTypes struct {
CrtType reflect.Type
CrtConfigType reflect.Type
}

var ContainerIfRegistry = map[string]ContainerIfTypes{
"docker": ContainerIfTypes{
CrtType: reflect.TypeOf(docker.Docker{}),
CrtConfigType: reflect.TypeOf(docker.DockerConfig{}),
},
}

type ContainerEpContext struct {
NewContName string
CurrContName string
InterfaceId string
IpAddress string
SubnetLen uint
DefaultGw string
}

func (c *Crt) AttachEndpoint(contEpContext *core.ContainerEpContext) error {
return c.ContainerIf.AttachEndpoint(contEpContext)
}

func (c *Crt) DetachEndpoint(contEpContext *core.ContainerEpContext) error {
return c.ContainerIf.DetachEndpoint(contEpContext)
}

func (c *Crt) GetContainerId(contName string) string {
return c.ContainerIf.GetContainerId(contName)
}

func (c *Crt) GetContainerName(contId string) (string, error) {
return c.ContainerIf.GetContainerName(contId)
}

func (c *Crt) Deinit() {
c.ContainerIf.Deinit()
}

func (c *Crt) Init(configStr string) error {

cfg := &CrtConfig{}
err := json.Unmarshal([]byte(configStr), cfg)
if err != nil {
return err
}

if _, ok := ContainerIfRegistry[cfg.Crt.Type]; !ok {
return errors.New("unregistered container run time")
}

crtConfigType := ContainerIfRegistry[cfg.Crt.Type].CrtConfigType
crtConfig := reflect.New(crtConfigType).Interface()
err = json.Unmarshal([]byte(configStr), crtConfig)
if err != nil {
return err
}

config := &core.Config{V: crtConfig}
crtType := ContainerIfRegistry[cfg.Crt.Type].CrtType
crtif := reflect.New(crtType).Interface()
c.ContainerIf = crtif.(core.ContainerIf)
err = c.ContainerIf.Init(config)
if err != nil {
return err
}

return nil
}
76 changes: 76 additions & 0 deletions crt/crt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/***
Copyright 2014 Cisco Systems Inc. All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package crt

import (
"testing"
)

func TestCrtInit(t *testing.T) {
configStr := `{
"docker" : {
"socket" : "unix:///var/run/docker.sock"
},
"crt" : {
"type": "docker"
}
}`
crt := Crt{}
err := crt.Init(configStr)
if err != nil {
t.Fatalf("crt init failed: Error: %s", err)
}
defer func() { crt.Deinit() }()
}

func TestCrtInitInvalidConfigMissingCrt(t *testing.T) {
configStr := `{
"docker" : {
"socket" : "unix:///var/run/docker.sock"
}
}`
crt := Crt{}
err := crt.Init(configStr)
if err == nil {
t.Fatalf("crt init succeeded!")
}
}

func TestCrtInitInvalidConfigMissingCrtIf(t *testing.T) {
configStr := `{
"crt" : {
"type": "docker"
}
}`
crt := Crt{}
err := crt.Init(configStr)
if err == nil {
t.Fatalf("crt init succeeded!")
}
}

func TestCrtInitInvalidConfigInvalidCrt(t *testing.T) {
configStr := `{
"crt" : {
"type": "rocket"
}
}`
crt := Crt{}
err := crt.Init(configStr)
if err == nil {
t.Fatalf("crt init succeeded!")
}
}
36 changes: 20 additions & 16 deletions drivers/dockerdriver.go → crt/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package drivers
package docker

import (
"errors"
Expand All @@ -30,37 +30,41 @@ import (
// "github.com/vishvananda/netns"
)

type DockerDriverConfig struct {
type DockerConfig struct {
Docker struct {
Socket string
}
}

type DockerDriver struct {
type Docker struct {
Client *dockerclient.DockerClient
}

func (d *DockerDriver) Init(config *core.Config) error {
func (d *Docker) Init(config *core.Config) error {
if config == nil {
return &core.Error{Desc: fmt.Sprintf("Invalid arguments. cfg: %v", config)}
return errors.New("null config!")
}

cfg, ok := config.V.(*DockerDriverConfig)
cfg, ok := config.V.(*DockerConfig)

if !ok {
return &core.Error{Desc: "Invalid config type passed!"}
}

if cfg.Docker.Socket == "" {
return errors.New(fmt.Sprintf("Invalid arguments. cfg: %v", config))
}

// TODO: ADD TLS support
d.Client, _ = dockerclient.NewDockerClient(cfg.Docker.Socket, nil)

return nil
}

func (d *DockerDriver) Deinit() {
func (d *Docker) Deinit() {
}

func (d *DockerDriver) getContPid(contName string) (string, error) {
func (d *Docker) getContPid(contName string) (string, error) {
contInfo, err := d.Client.InspectContainer(contName)
if err != nil {
return "", errors.New("couldn't obtain container info")
Expand Down Expand Up @@ -93,7 +97,7 @@ func setIfNs(ifname string, pid int) error {
// Note: most of the work in this function is a temporary workaround for
// what docker daemon would eventually do; in the meanwhile
// the essense of the logic is borrowed from pipework
func (d *DockerDriver) moveIfToContainer(ifId string, contName string) error {
func (d *Docker) moveIfToContainer(ifId string, contName string) error {

// log.Printf("Moving interface '%s' into container '%s' \n", ifId, contName)

Expand Down Expand Up @@ -137,7 +141,7 @@ func (d *DockerDriver) moveIfToContainer(ifId string, contName string) error {
return err
}

func (d *DockerDriver) cleanupNetns(contName string) error {
func (d *Docker) cleanupNetns(contName string) error {
contPid, err := d.getContPid(contName)
if err != nil {
return err
Expand All @@ -153,7 +157,7 @@ func (d *DockerDriver) cleanupNetns(contName string) error {
}

/*
func (d *DockerDriver) configureIfAddress(ctx *core.ContainerEpContext) error {
func (d *Docker) configureIfAddress(ctx *core.ContainerEpContext) error {

log.Printf("configuring ip: addr -%s/%d- on if %s for container %s\n",
ctx.IpAddress, ctx.SubnetLen, ctx.InterfaceId, ctx.NewContName)
Expand Down Expand Up @@ -228,7 +232,7 @@ func (d *DockerDriver) configureIfAddress(ctx *core.ContainerEpContext) error {
return err
}
*/
func (d *DockerDriver) configureIfAddress(ctx *core.ContainerEpContext) error {
func (d *Docker) configureIfAddress(ctx *core.ContainerEpContext) error {

log.Printf("configuring ip: addr -%s/%d- on if %s for container %s\n",
ctx.IpAddress, ctx.SubnetLen, ctx.InterfaceId, ctx.NewContName)
Expand Down Expand Up @@ -268,7 +272,7 @@ func (d *DockerDriver) configureIfAddress(ctx *core.ContainerEpContext) error {

// performs funtion to configure the network access and policies
// before the container becomes active
func (d *DockerDriver) AttachEndpoint(ctx *core.ContainerEpContext) error {
func (d *Docker) AttachEndpoint(ctx *core.ContainerEpContext) error {

err := d.moveIfToContainer(ctx.InterfaceId, ctx.NewContName)
if err != nil {
Expand All @@ -289,7 +293,7 @@ func (d *DockerDriver) AttachEndpoint(ctx *core.ContainerEpContext) error {
}

// uninstall the policies and configuration during container attach
func (d *DockerDriver) DetachEndpoint(ctx *core.ContainerEpContext) error {
func (d *Docker) DetachEndpoint(ctx *core.ContainerEpContext) error {
var err error

// log.Printf("Detached called for container %s with %s interface\n",
Expand All @@ -303,7 +307,7 @@ func (d *DockerDriver) DetachEndpoint(ctx *core.ContainerEpContext) error {
return err
}

func (d *DockerDriver) GetContainerId(contName string) string {
func (d *Docker) GetContainerId(contName string) string {
contInfo, err := d.Client.InspectContainer(contName)
if err != nil {
log.Printf("could not get contId for container %s, err '%s' \n",
Expand All @@ -319,7 +323,7 @@ func (d *DockerDriver) GetContainerId(contName string) string {
return contInfo.Id
}

func (d *DockerDriver) GetContainerName(contId string) (string, error) {
func (d *Docker) GetContainerName(contId string) (string, error) {
contInfo, err := d.Client.InspectContainer(contId)
if err != nil {
log.Printf("could not get contName for container %s, err '%s' \n",
Expand Down
Loading