From 03504cab659f5b4cfe57b24103b6d1288025b28b Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Sun, 14 Jun 2015 09:00:27 -0700 Subject: [PATCH] Few changes to the UI and API implementation 1. replaced --net option for service UI with SERVICE.[NETWORK] format 2. Making using of the default network/driver backend support 3. NetworkName and NetworkType from the UI/API can be empty string and it will be replaced with DefaultNetwork and DefaultDriver As per the design goals, we wanted to keep libnetwork core free of handling defaults. Rather, the clients (docker & dnet) must handle the defaultness of these entities. Also, since there is no API to get these Default values from the backend, UI will not handle the default values either. Hence, this falls under the responsibility of the API layer to handle this specific case. Signed-off-by: Madhu Venugopal --- libnetwork/api/api.go | 34 +++++++++ libnetwork/client/client_service_test.go | 12 +-- libnetwork/client/network.go | 25 +------ libnetwork/client/service.go | 94 +++++++++++------------- libnetwork/cmd/dnet/dnet.go | 39 +++++++++- libnetwork/cmd/dnet/libnetwork.toml | 11 +++ libnetwork/config/config.go | 8 ++ libnetwork/config/config_test.go | 12 +++ libnetwork/controller.go | 5 +- libnetwork/network.go | 3 +- 10 files changed, 160 insertions(+), 83 deletions(-) create mode 100755 libnetwork/cmd/dnet/libnetwork.toml diff --git a/libnetwork/api/api.go b/libnetwork/api/api.go index 273be0e6ba..641f10db4d 100644 --- a/libnetwork/api/api.go +++ b/libnetwork/api/api.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/docker/libnetwork" + "github.com/docker/libnetwork/netlabel" "github.com/docker/libnetwork/types" "github.com/gorilla/mux" ) @@ -39,6 +40,11 @@ const ( urlEpID = "endpoint-id" urlEpPID = "endpoint-partial-id" urlCnID = "container-id" + + // BridgeNetworkDriver is the built-in default for Network Driver + BridgeNetworkDriver = "bridge" + // BridgeDefaultNetwork is the built-in default for network name + BridgeDefaultNetwork = "bridge" ) // NewHTTPHandler creates and initialize the HTTP handler to serve the requests for libnetwork @@ -246,6 +252,30 @@ func (ej *endpointJoin) parseOptions() []libnetwork.EndpointOption { Process functions *******************/ +func processCreateDefaults(c libnetwork.NetworkController, nc *networkCreate) { + if nc.NetworkType == "" { + nc.NetworkType = c.Config().Daemon.DefaultDriver + } + if nc.NetworkType == BridgeNetworkDriver { + if nc.Options == nil { + nc.Options = make(map[string]interface{}) + } + genericData, ok := nc.Options[netlabel.GenericData] + if !ok { + genericData = make(map[string]interface{}) + } + gData := genericData.(map[string]interface{}) + + if _, ok := gData["BridgeName"]; !ok { + gData["BridgeName"] = nc.Name + } + if _, ok := gData["AllowNonDefaultBridge"]; !ok { + gData["AllowNonDefaultBridge"] = "true" + } + nc.Options[netlabel.GenericData] = genericData + } +} + /*************************** NetworkController interface ****************************/ @@ -256,6 +286,7 @@ func procCreateNetwork(c libnetwork.NetworkController, vars map[string]string, b if err != nil { return "", &responseStatus{Status: "Invalid body: " + err.Error(), StatusCode: http.StatusBadRequest} } + processCreateDefaults(c, &create) nw, err := c.NewNetwork(create.NetworkType, create.Name, create.parseOptions()...) if err != nil { @@ -665,6 +696,9 @@ func findNetwork(c libnetwork.NetworkController, s string, by int) (libnetwork.N case byID: nw, err = c.NetworkByID(s) case byName: + if s == "" { + s = c.Config().Daemon.DefaultNetwork + } nw, err = c.NetworkByName(s) default: panic(fmt.Sprintf("unexpected selector for network search: %d", by)) diff --git a/libnetwork/client/client_service_test.go b/libnetwork/client/client_service_test.go index 52b4c0cd53..fb10fd9fd7 100644 --- a/libnetwork/client/client_service_test.go +++ b/libnetwork/client/client_service_test.go @@ -21,7 +21,7 @@ func TestClientServiceCreate(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "publish", "-net="+mockNwName, mockServiceName) + err := cli.Cmd("docker", "service", "publish", mockServiceName+"."+mockNwName) if err != nil { t.Fatal(err) } @@ -31,7 +31,7 @@ func TestClientServiceRm(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "unpublish", "-net="+mockNwName, mockServiceName) + err := cli.Cmd("docker", "service", "unpublish", mockServiceName+"."+mockNwName) if err != nil { t.Fatal(err) } @@ -51,7 +51,7 @@ func TestClientServiceInfo(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "info", "-net="+mockNwName, mockServiceName) + err := cli.Cmd("docker", "service", "info", mockServiceName+"."+mockNwName) if err != nil { t.Fatal(err) } @@ -61,7 +61,7 @@ func TestClientServiceInfoById(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "info", "-net="+mockNwName, mockServiceID) + err := cli.Cmd("docker", "service", "info", mockServiceID+"."+mockNwName) if err != nil { t.Fatal(err) } @@ -71,7 +71,7 @@ func TestClientServiceJoin(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "attach", "-net="+mockNwName, mockContainerID, mockServiceName) + err := cli.Cmd("docker", "service", "attach", mockContainerID, mockServiceName+"."+mockNwName) if err != nil { t.Fatal(err) } @@ -81,7 +81,7 @@ func TestClientServiceLeave(t *testing.T) { var out, errOut bytes.Buffer cli := NewNetworkCli(&out, &errOut, callbackFunc) - err := cli.Cmd("docker", "service", "detach", "-net="+mockNwName, mockContainerID, mockServiceName) + err := cli.Cmd("docker", "service", "detach", mockContainerID, mockServiceName+"."+mockNwName) if err != nil { t.Fatal(err) } diff --git a/libnetwork/client/network.go b/libnetwork/client/network.go index 33e3674456..9d9ce71c3f 100644 --- a/libnetwork/client/network.go +++ b/libnetwork/client/network.go @@ -9,11 +9,6 @@ import ( flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/pkg/stringid" - "github.com/docker/libnetwork/netlabel" -) - -const ( - defaultDriverType = "bridge" ) type command struct { @@ -45,28 +40,16 @@ func (cli *NetworkCli) CmdNetwork(chain string, args ...string) error { // CmdNetworkCreate handles Network Create UI func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error { cmd := cli.Subcmd(chain, "create", "NETWORK-NAME", "Creates a new network with a name specified by the user", false) - flDriver := cmd.String([]string{"d", "-driver"}, "bridge", "Driver to manage the Network") - cmd.Require(flag.Min, 1) + flDriver := cmd.String([]string{"d", "-driver"}, "", "Driver to manage the Network") + cmd.Require(flag.Exact, 1) err := cmd.ParseFlags(args, true) if err != nil { return err } - if *flDriver == "" { - *flDriver = defaultDriverType - } // Construct network create request body ops := make(map[string]interface{}) nc := networkCreate{Name: cmd.Arg(0), NetworkType: *flDriver, Options: ops} - - // Set driver specific options - if *flDriver == "bridge" { - ops[netlabel.GenericData] = map[string]string{ - "BridgeName": cmd.Arg(0), - "AllowNonDefaultBridge": "true", - } - } - obj, _, err := readBody(cli.call("POST", "/networks", nc, nil)) if err != nil { return err @@ -83,7 +66,7 @@ func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error { // CmdNetworkRm handles Network Delete UI func (cli *NetworkCli) CmdNetworkRm(chain string, args ...string) error { cmd := cli.Subcmd(chain, "rm", "NETWORK", "Deletes a network", false) - cmd.Require(flag.Min, 1) + cmd.Require(flag.Exact, 1) err := cmd.ParseFlags(args, true) if err != nil { return err @@ -155,7 +138,7 @@ func (cli *NetworkCli) CmdNetworkLs(chain string, args ...string) error { // CmdNetworkInfo handles Network Info UI func (cli *NetworkCli) CmdNetworkInfo(chain string, args ...string) error { cmd := cli.Subcmd(chain, "info", "NETWORK", "Displays detailed information on a network", false) - cmd.Require(flag.Min, 1) + cmd.Require(flag.Exact, 1) err := cmd.ParseFlags(args, true) if err != nil { return err diff --git a/libnetwork/client/service.go b/libnetwork/client/service.go index 3822874106..e2285d7e6f 100644 --- a/libnetwork/client/service.go +++ b/libnetwork/client/service.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "text/tabwriter" flag "github.com/docker/docker/pkg/mflag" @@ -36,6 +37,18 @@ func lookupServiceID(cli *NetworkCli, nwName, svNameID string) (string, error) { return "", fmt.Errorf("Network %s does not exist", nwName) } + if nwName == "" { + obj, _, err := readBody(cli.call("GET", "/networks/"+nwList[0].ID, nil, nil)) + if err != nil { + return "", err + } + networkResource := &networkResource{} + if err := json.NewDecoder(bytes.NewReader(obj)).Decode(networkResource); err != nil { + return "", err + } + nwName = networkResource.Name + } + // Query service by name obj, statusCode, err := readBody(cli.call("GET", fmt.Sprintf("/services?name=%s", svNameID), nil, nil)) if err != nil { @@ -95,24 +108,30 @@ func (cli *NetworkCli) CmdService(chain string, args ...string) error { return err } +// Parse service name for "SERVICE[.NETWORK]" format +func parseServiceName(name string) (string, string) { + s := strings.Split(name, ".") + var sName, nName string + if len(s) > 1 { + nName = s[len(s)-1] + sName = strings.Join(s[:len(s)-1], ".") + } else { + sName = s[0] + } + return sName, nName +} + // CmdServicePublish handles service create UI func (cli *NetworkCli) CmdServicePublish(chain string, args ...string) error { - cmd := cli.Subcmd(chain, "publish", "SERVICE", "Publish a new service on a network", false) - flNetwork := cmd.String([]string{"net", "-network"}, "", "Network where to publish the service") - cmd.Require(flag.Min, 1) - + cmd := cli.Subcmd(chain, "publish", "SERVICE[.NETWORK]", "Publish a new service on a network", false) + cmd.Require(flag.Exact, 1) err := cmd.ParseFlags(args, true) if err != nil { return err } - // Default network changes will come later - nw := "docker0" - if *flNetwork != "" { - nw = *flNetwork - } - - sc := serviceCreate{Name: cmd.Arg(0), Network: nw} + sn, nn := parseServiceName(cmd.Arg(0)) + sc := serviceCreate{Name: sn, Network: nn} obj, _, err := readBody(cli.call("POST", "/services", sc, nil)) if err != nil { return err @@ -130,22 +149,15 @@ func (cli *NetworkCli) CmdServicePublish(chain string, args ...string) error { // CmdServiceUnpublish handles service delete UI func (cli *NetworkCli) CmdServiceUnpublish(chain string, args ...string) error { - cmd := cli.Subcmd(chain, "unpublish", "SERVICE", "Removes a service", false) - flNetwork := cmd.String([]string{"net", "-network"}, "", "Network where to publish the service") - cmd.Require(flag.Min, 1) - + cmd := cli.Subcmd(chain, "unpublish", "SERVICE[.NETWORK]", "Removes a service", false) + cmd.Require(flag.Exact, 1) err := cmd.ParseFlags(args, true) if err != nil { return err } - // Default network changes will come later - nw := "docker0" - if *flNetwork != "" { - nw = *flNetwork - } - - serviceID, err := lookupServiceID(cli, nw, cmd.Arg(0)) + sn, nn := parseServiceName(cmd.Arg(0)) + serviceID, err := lookupServiceID(cli, nn, sn) if err != nil { return err } @@ -167,8 +179,6 @@ func (cli *NetworkCli) CmdServiceLs(chain string, args ...string) error { return err } - cmd.Require(flag.Min, 1) - var obj []byte if *flNetwork == "" { obj, _, err = readBody(cli.call("GET", "/services", nil, nil)) @@ -237,8 +247,7 @@ func getBackendID(cli *NetworkCli, servID string) (string, error) { // CmdServiceInfo handles service info UI func (cli *NetworkCli) CmdServiceInfo(chain string, args ...string) error { - cmd := cli.Subcmd(chain, "info", "SERVICE", "Displays detailed information about a service", false) - flNetwork := cmd.String([]string{"net", "-network"}, "", "Network where to publish the service") + cmd := cli.Subcmd(chain, "info", "SERVICE[.NETWORK]", "Displays detailed information about a service", false) cmd.Require(flag.Min, 1) err := cmd.ParseFlags(args, true) @@ -246,13 +255,8 @@ func (cli *NetworkCli) CmdServiceInfo(chain string, args ...string) error { return err } - // Default network changes will come later - nw := "docker0" - if *flNetwork != "" { - nw = *flNetwork - } - - serviceID, err := lookupServiceID(cli, nw, cmd.Arg(0)) + sn, nn := parseServiceName(cmd.Arg(0)) + serviceID, err := lookupServiceID(cli, nn, sn) if err != nil { return err } @@ -276,27 +280,20 @@ func (cli *NetworkCli) CmdServiceInfo(chain string, args ...string) error { // CmdServiceAttach handles service attach UI func (cli *NetworkCli) CmdServiceAttach(chain string, args ...string) error { - cmd := cli.Subcmd(chain, "attach", "CONTAINER SERVICE", "Sets a container as a service backend", false) - flNetwork := cmd.String([]string{"net", "-network"}, "", "Network where to publish the service") + cmd := cli.Subcmd(chain, "attach", "CONTAINER SERVICE[.NETWORK]", "Sets a container as a service backend", false) cmd.Require(flag.Min, 2) - err := cmd.ParseFlags(args, true) if err != nil { return err } - // Default network changes will come later - nw := "docker0" - if *flNetwork != "" { - nw = *flNetwork - } - containerID, err := lookupContainerID(cli, cmd.Arg(0)) if err != nil { return err } - serviceID, err := lookupServiceID(cli, nw, cmd.Arg(1)) + sn, nn := parseServiceName(cmd.Arg(1)) + serviceID, err := lookupServiceID(cli, nn, sn) if err != nil { return err } @@ -311,26 +308,19 @@ func (cli *NetworkCli) CmdServiceAttach(chain string, args ...string) error { // CmdServiceDetach handles service detach UI func (cli *NetworkCli) CmdServiceDetach(chain string, args ...string) error { cmd := cli.Subcmd(chain, "detach", "CONTAINER SERVICE", "Removes a container from service backend", false) - flNetwork := cmd.String([]string{"net", "-network"}, "", "Network where to publish the service") cmd.Require(flag.Min, 2) - err := cmd.ParseFlags(args, true) if err != nil { return err } - // Default network changes will come later - nw := "docker0" - if *flNetwork != "" { - nw = *flNetwork - } - + sn, nn := parseServiceName(cmd.Arg(1)) containerID, err := lookupContainerID(cli, cmd.Arg(0)) if err != nil { return err } - serviceID, err := lookupServiceID(cli, nw, cmd.Arg(1)) + serviceID, err := lookupServiceID(cli, nn, sn) if err != nil { return err } diff --git a/libnetwork/cmd/dnet/dnet.go b/libnetwork/cmd/dnet/dnet.go index 32920c8b85..18a2b1b999 100644 --- a/libnetwork/cmd/dnet/dnet.go +++ b/libnetwork/cmd/dnet/dnet.go @@ -20,6 +20,8 @@ import ( "github.com/docker/libnetwork/api" "github.com/docker/libnetwork/client" "github.com/docker/libnetwork/config" + "github.com/docker/libnetwork/netlabel" + "github.com/docker/libnetwork/options" "github.com/gorilla/mux" ) @@ -63,11 +65,20 @@ func processConfig(cfg *config.Config) []config.Option { if cfg == nil { return options } + dn := "bridge" if strings.TrimSpace(cfg.Daemon.DefaultNetwork) != "" { - options = append(options, config.OptionDefaultNetwork(cfg.Daemon.DefaultNetwork)) + dn = cfg.Daemon.DefaultNetwork } + options = append(options, config.OptionDefaultNetwork(dn)) + + dd := "bridge" if strings.TrimSpace(cfg.Daemon.DefaultDriver) != "" { - options = append(options, config.OptionDefaultDriver(cfg.Daemon.DefaultDriver)) + dd = cfg.Daemon.DefaultDriver + } + options = append(options, config.OptionDefaultDriver(dd)) + + if cfg.Daemon.Labels != nil { + options = append(options, config.OptionLabels(cfg.Daemon.Labels)) } if strings.TrimSpace(cfg.Datastore.Client.Provider) != "" { options = append(options, config.OptionKVProvider(cfg.Datastore.Client.Provider)) @@ -136,6 +147,29 @@ func dnetCommand(stdout, stderr io.Writer) error { return nil } +func createDefaultNetwork(c libnetwork.NetworkController) { + nw := c.Config().Daemon.DefaultNetwork + d := c.Config().Daemon.DefaultDriver + createOptions := []libnetwork.NetworkOption{} + genericOption := options.Generic{} + + if nw != "" && d != "" { + // Bridge driver is special due to legacy reasons + if d == "bridge" { + genericOption[netlabel.GenericData] = map[string]interface{}{ + "BridgeName": nw, + "AllowNonDefaultBridge": "true", + } + networkOption := libnetwork.NetworkOptionGeneric(genericOption) + createOptions = append(createOptions, networkOption) + } + _, err := c.NewNetwork(d, nw, createOptions...) + if err != nil { + logrus.Errorf("Error creating default network : %s : %v", nw, err) + } + } +} + type dnetConnection struct { // proto holds the client protocol i.e. unix. proto string @@ -154,6 +188,7 @@ func (d *dnetConnection) dnetDaemon() error { fmt.Println("Error starting dnetDaemon :", err) return err } + createDefaultNetwork(controller) httpHandler := api.NewHTTPHandler(controller) r := mux.NewRouter().StrictSlash(false) post := r.PathPrefix("/{.*}/networks").Subrouter() diff --git a/libnetwork/cmd/dnet/libnetwork.toml b/libnetwork/cmd/dnet/libnetwork.toml new file mode 100755 index 0000000000..cbc605f53c --- /dev/null +++ b/libnetwork/cmd/dnet/libnetwork.toml @@ -0,0 +1,11 @@ +title = "LibNetwork Configuration file" + +[daemon] + debug = false + DefaultNetwork = "bridge" + DefaultDriver = "bridge" +[cluster] + discovery = "token://22aa23948f4f6b31230687689636959e" + Address = "1.1.1.1" +[datastore] + embedded = false diff --git a/libnetwork/config/config.go b/libnetwork/config/config.go index 637703f1eb..602f5c33e4 100644 --- a/libnetwork/config/config.go +++ b/libnetwork/config/config.go @@ -101,3 +101,11 @@ func (c *Config) ProcessOptions(options ...Option) { } } } + +// IsValidName validates configuration objects supported by libnetwork +func IsValidName(name string) bool { + if name == "" || strings.Contains(name, ".") { + return false + } + return true +} diff --git a/libnetwork/config/config_test.go b/libnetwork/config/config_test.go index 4820db040f..cc8a911c94 100644 --- a/libnetwork/config/config_test.go +++ b/libnetwork/config/config_test.go @@ -41,3 +41,15 @@ func TestOptionsLabels(t *testing.T) { } } } + +func TestValidName(t *testing.T) { + if !IsValidName("test") { + t.Fatal("Name validation fails for a name that must be accepted") + } + if IsValidName("") { + t.Fatal("Name validation succeeds for a case when it is expected to fail") + } + if IsValidName("name.with.dots") { + t.Fatal("Name validation succeeds for a case when it is expected to fail") + } +} diff --git a/libnetwork/controller.go b/libnetwork/controller.go index 2f41bac5e8..a2ab5046ef 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -188,6 +188,9 @@ func (c *controller) ConfigureNetworkDriver(networkType string, options map[stri func (c *controller) RegisterDriver(networkType string, driver driverapi.Driver, capability driverapi.Capability) error { c.Lock() defer c.Unlock() + if !config.IsValidName(networkType) { + return ErrInvalidName(networkType) + } if _, ok := c.drivers[networkType]; ok { return driverapi.ErrActiveRegistration(networkType) } @@ -198,7 +201,7 @@ func (c *controller) RegisterDriver(networkType string, driver driverapi.Driver, // NewNetwork creates a new network of the specified network type. The options // are network specific and modeled in a generic way. func (c *controller) NewNetwork(networkType, name string, options ...NetworkOption) (Network, error) { - if name == "" { + if !config.IsValidName(name) { return nil, ErrInvalidName(name) } // Check if a network already exists with the specified network name diff --git a/libnetwork/network.go b/libnetwork/network.go index 7e6c343b1a..28369656f4 100644 --- a/libnetwork/network.go +++ b/libnetwork/network.go @@ -6,6 +6,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/stringid" + "github.com/docker/libnetwork/config" "github.com/docker/libnetwork/datastore" "github.com/docker/libnetwork/driverapi" "github.com/docker/libnetwork/netlabel" @@ -274,7 +275,7 @@ func (n *network) addEndpoint(ep *endpoint) error { func (n *network) CreateEndpoint(name string, options ...EndpointOption) (Endpoint, error) { var err error - if name == "" { + if !config.IsValidName(name) { return nil, ErrInvalidName(name) }