Fixing the meaning of "container_prefix"

Currently this API attribute means "exact name of a container" interface, but it was never the original intention. It was just a temporary change when we have moved to the asynchronous provisioning method. Even the name suggest this is just a prefix of the NICs :) Functionally speaking this change was making it hard to attach multiple NICs to the same DanmNet... but nevermore!
nokia · Apr 16, 2019 · 07827c6 · 07827c6
1 parent 57911a5
commit 07827c6
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 22 deletions.
diff --git a/README.md b/README.md
@@ -253,12 +253,13 @@ The hard reality to keep in mind is that you shall always have an interface lite
 If such an interface does not exist after CNI is invoked (also having an IPv4 address), the state of the Pod will be considered "faulty", and it will be re-created in a loop.
 To be able to comply with this Kubernetes limitation, DANM supports both explicit, and implicit interface naming schemes for all NetworkTypes!
 
-An interface connected to a DanmNet containing the container_prefix attribute will be always named accordingly. You can use this API to explicitly set descriptive, unique names to NICs connecting to this network.
-In case container_prefix is not set in an interface's network descriptor, DANM automatically names the interface "ethX", where X is a unique integer number corresponding to the sequence number of the network connection (e.g. the first interface defined in the annotation is called "eth0", second interface "eth1" etc.)
+An interface connected to a DanmNet containing the container_prefix attribute is always named accordingly. You can use this API to explicitly set descriptive, unique names to NICs connecting to this network.
+In case container_prefix is not set in an interface's network descriptor, DANM automatically uses the "eth" as the prefix when naming the interface.
+Regardless which prefix is used, the interface name is also suffixed with an integer number corresponding to the sequence number of the network connection (e.g. the first interface defined in the annotation is called "eth0", second interface "eth1" etc.)
 DANM even supports the mixing of the networking schemes within the same Pod, and it supports the whole naming scheme for all network backends.
-While the feature provides complete control over the name of interfaces, ultimately it is the network administrators' responsibility to:
- - make sure exactly one interface is named eth0 in every Pod
- - don't configure multiple NICs into the same Pod with clashing names (e.g. provisioning two implicitly named interfaces, and then a third one explicitly named "eth0", or "eth1" etc.)
+This enables network administrators to even connect Pods to the same network more than once!
+While the feature provides complete control over the name of interfaces, ultimately it is the network administrators' responsibility to make sure exactly one interface is named eth0 in every Pod.
+
 ##### Provisioning static IP routes
 We recognize that not all networking involves an overlay technology, so provisioning IP routes directly into the Pod's network namespace needs to be generally supported.
 Network administrators can define routing rules for both IPv4, and IPv6 destination subnets under the "routes", and "routes6" attributes respectively.

diff --git a/crd/apis/danm/v1/types.go b/crd/apis/danm/v1/types.go
@@ -119,6 +119,7 @@ type Interface struct {
   Proutes6 map[string]string `json:"proutes6"`
   DefaultIfaceName string
   Device string `json:"Device,omitempty"`
+  SequenceId int
 }
 
 type IpamConfig struct {

diff --git a/pkg/cnidel/cnidel.go b/pkg/cnidel/cnidel.go
@@ -4,6 +4,7 @@ import (
   "errors"
   "log"
   "os"
+  "strconv"
   "strings"
   "path/filepath"
   "github.com/containernetworking/cni/pkg/invoke"
@@ -259,9 +260,9 @@ func GetEnv(key, fallback string) string {
 // CalculateIfaceName decides what should be the name of a container's interface.
 // If a name is explicitly set in the related DanmNet API object, the NIC will be named accordingly.
 // If a name is not explicitly set, then DANM will name the interface ethX where X=sequence number of the interface
-func CalculateIfaceName(chosenName, defaultName string) string {
+func CalculateIfaceName(chosenName, defaultName string, sequenceId int) string {
   if chosenName != "" {
-    return chosenName
+    return chosenName + strconv.Itoa(sequenceId)
   }
-  return defaultName
+  return defaultName + strconv.Itoa(sequenceId)
 }
diff --git a/pkg/metacni/metacni.go b/pkg/metacni/metacni.go
@@ -7,7 +7,6 @@ import (
   "net"
   "os"
   "runtime"
-  "strconv"
   "strings"
   "encoding/json"
   "github.com/satori/go.uuid"
@@ -30,14 +29,18 @@ import (
   checkpoint_utils "github.com/intel/multus-cni/checkpoint"
 )
 
-var (
-  apiHost = os.Getenv("API_SERVERS")
+const (
   danmApiPath = "danm.k8s.io"
   danmIfDefinitionSyntax = danmApiPath + "/interfaces"
   v1Endpoint = "/api/v1/"
   cniVersion = "0.3.1"
-  kubeConf string
   defaultNetworkName = "default"
+  defaultIfName = "eth"
+)
+
+var (
+  apiHost = os.Getenv("API_SERVERS")
+  kubeConf string
 )
 
 type NetConf struct {
@@ -260,7 +263,8 @@ func setupNetworking(args *cniArgs) (*current.Result, error) {
     if err != nil {
       return cniRes, errors.New("failed to get DanmNet due to:" + err.Error())
     }
-    nicParams.DefaultIfaceName = "eth" + strconv.Itoa(nicID)
+    nicParams.SequenceId = nicID
+    nicParams.DefaultIfaceName = defaultIfName
     if isDelegationRequired {
       if cnidel.IsDeviceNeeded(netInfo.Spec.NetworkType) {
         if _, ok := allocatedDevices[netInfo.Spec.Options.DevicePool]; !ok {
@@ -285,7 +289,7 @@ func setupNetworking(args *cniArgs) (*current.Result, error) {
 
 func createDelegatedInterface(syncher *syncher.Syncher, danmClient danmclientset.Interface, iface danmtypes.Interface, netInfo *danmtypes.DanmNet, args *cniArgs) {
   epIfaceSpec := danmtypes.DanmEpIface {
-    Name:        cnidel.CalculateIfaceName(netInfo.Spec.Options.Prefix, iface.DefaultIfaceName),
+    Name:        cnidel.CalculateIfaceName(netInfo.Spec.Options.Prefix, iface.DefaultIfaceName, iface.SequenceId),
     Address:     iface.Ip,
     AddressIPv6: iface.Ip6,
     Proutes:     iface.Proutes,
@@ -322,7 +326,7 @@ func createDanmInterface(syncher *syncher.Syncher, danmClient danmclientset.Inte
     syncher.PushResult(iface.Network, errors.New("IP address reservation failed for network:" + netId + " with error:" + err.Error()), nil)
   }
   epSpec := danmtypes.DanmEpIface {
-    Name: cnidel.CalculateIfaceName(netInfo.Spec.Options.Prefix, iface.DefaultIfaceName),
+    Name: cnidel.CalculateIfaceName(netInfo.Spec.Options.Prefix, iface.DefaultIfaceName, iface.SequenceId),
     Address: ip4,
     AddressIPv6: ip6,
     MacAddress: macAddr,

diff --git a/schema/DanmNet.yaml b/schema/DanmNet.yaml
@@ -62,10 +62,11 @@ spec:
     # Only has an effect with dynamically integrated backends. Ignored for other NetworkTypes.
     # OPTIONAL - IPv6 CIDR FORMAT (e.g. "2001:db8::/45"). Netmask of the subnet cannot be higher than /64 (i.e. /65 and upwards).
     net6: ## SUBNET_CIDR ##
-    # Interfaces connected to this network are renamed to "container_prefix" inside the container.
-    # If not provided, DANM dynamically allocates a name for each container interface belonging to this network in Pod instantiation time.
+    # Interfaces connected to this network are renamed inside the Pod's network namespace to a string starting with "container_prefix".
+    # If not provided, DANM uses "eth" as the prefix.
+    # In both cases DANM dynamically suffixes the interface names in Pod instantiation time with a unique integer number, corresponding to the sequence number of the interface during the specific network creation operation.
+    # Thus it becomes guaranteed no network interfaces will ever receive the same name, even if more than one belongs to the same DanmNet.
     # Generally supported parameter, works with all NetworkTypes (except where the CNI backend itself is not following the CNI standard, such is the case with Flannel).
-    # Do not specify container_prefix in case multiple interfaces are provisioned from the same DanmNet.
     # OPTIONAL - STRING
     container_prefix: ## INTERNAL_IF_NAME ##
     # Policy-based IP routes belonging to this network are installed into this routing table, when a user defines them in her Pod's network allocation annotation.

diff --git a/test/uts/cnidel_test/cnidel_test.go b/test/uts/cnidel_test/cnidel_test.go
@@ -2,6 +2,7 @@ package cnidel_test
 
 import (
   "os"
+  "strconv"
   "strings"
   "testing"
   "io/ioutil"
@@ -254,12 +255,15 @@ func TestGetEnv(t *testing.T) {
 func TestCalculateIfaceName(t *testing.T) {
   testChosenName := "thechosenone"
   testDefaultName := "notthechosenone"
-  ifaceName := cnidel.CalculateIfaceName(testChosenName, testDefaultName)
-  if ifaceName != testChosenName {
+  testSequenceId := 4
+  expChosenName := testChosenName+strconv.Itoa(testSequenceId)
+  expDefName := testDefaultName+strconv.Itoa(testSequenceId)
+  ifaceName := cnidel.CalculateIfaceName(testChosenName, testDefaultName, testSequenceId)
+  if ifaceName != expChosenName {
     t.Errorf("Received value for explicitly set interface name: %s does not match with expected: %s", ifaceName, testChosenName)
   }
-  defIfaceName := cnidel.CalculateIfaceName("", testDefaultName)
-  if defIfaceName != testDefaultName {
+  defIfaceName := cnidel.CalculateIfaceName("", testDefaultName, testSequenceId)
+  if defIfaceName != expDefName {
     t.Errorf("Received value for default interface name: %s does not match with expected: %s", defIfaceName, testChosenName)
   }
 }