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

Fixing iface deletion bug when netIds don't match #80

Merged
merged 1 commit into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,15 @@ __Such action will undoubtedly lead to ruin and DANMation!__
Generally speaking, you need to care about how the network interfaces of your Pods are named inside their respective network namespaces.
The hard reality to keep in mind is that you shall always have an interface literally called "eth0" created within all your Kubernetes Pods, because Kubelet will always search for the existence of such an interface at the end of Pod instantiation.
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!
To be able to comply with this Kubernetes limitation, DANM always names the first container interface "eth0", regardless of your intention.
Sorry, but they made us do it :)

However, DANM also supports both explicit, and implicit interface naming schemes for all NetworkTypes to help you flexibly name the other interfaces!
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.
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.
Expand Down
3 changes: 1 addition & 2 deletions crd/apis/danm/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,14 @@ type DanmEp struct {

type DanmEpSpec struct {
NetworkID string `json:"NetworkID"`
NetworkName string `json:"NetworkName"`
NetworkType string `json:"NetworkType"`
EndpointID string `json:"EndpointID"`
Iface DanmEpIface `json:"Interface"`
Host string `json:"Host,omitempty"`
Pod string `json:"Pod"`
CID string `json:"CID,omitempty"`
Netns string `json:"netns,omitempty"`
Creator string `json:"Creator,omitempty"`
Expires string `json:"Expires,omitempty"`
}

type DanmEpIface struct {
Expand Down
4 changes: 4 additions & 0 deletions pkg/cnidel/cnidel.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ func GetEnv(key, fallback string) string {
// 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, sequenceId int) string {
//Kubelet expects the first interface to be literally named "eth0", so...
if sequenceId == 0 {
return "eth0"
}
if chosenName != "" {
return chosenName + strconv.Itoa(sequenceId)
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/metacni/metacni.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func createDelegatedInterface(syncher *syncher.Syncher, danmClient danmclientset
Proutes6: iface.Proutes6,
DeviceID: iface.Device,
}
ep, err := createDanmEp(epIfaceSpec, netInfo.Spec.NetworkID, netInfo.Spec.NetworkType, args)
ep, err := createDanmEp(epIfaceSpec, netInfo, args)
if err != nil {
syncher.PushResult(iface.Network, errors.New("DanmEp object could not be created due to error:" + err.Error()), nil)
return
Expand Down Expand Up @@ -333,8 +333,7 @@ func createDanmInterface(syncher *syncher.Syncher, danmClient danmclientset.Inte
Proutes: iface.Proutes,
Proutes6: iface.Proutes6,
}
networkType := "ipvlan"
ep, err := createDanmEp(epSpec, netId, networkType, args)
ep, err := createDanmEp(epSpec, netInfo, args)
if err != nil {
ipam.GarbageCollectIps(danmClient, netInfo, ip4, ip6)
syncher.PushResult(iface.Network, errors.New("DanmEp object could not be created due to error:" + err.Error()), nil)
Expand Down Expand Up @@ -371,7 +370,7 @@ func createDanmInterface(syncher *syncher.Syncher, danmClient danmclientset.Inte
syncher.PushResult(iface.Network, nil, danmResult)
}

func createDanmEp(epInput danmtypes.DanmEpIface, netId string, neType string, args *cniArgs) (danmtypes.DanmEp, error) {
func createDanmEp(epInput danmtypes.DanmEpIface, netInfo *danmtypes.DanmNet, args *cniArgs) (danmtypes.DanmEp, error) {
epidInt, err := uuid.NewV4()
if err != nil {
return danmtypes.DanmEp{}, errors.New("uuid.NewV4 returned error during EP creation:" + err.Error())
Expand All @@ -381,16 +380,19 @@ func createDanmEp(epInput danmtypes.DanmEpIface, netId string, neType string, ar
if err != nil {
return danmtypes.DanmEp{}, errors.New("OS.Hostname returned error during EP creation:" + err.Error())
}
if netInfo.Spec.NetworkType == "" {
netInfo.Spec.NetworkType = "ipvlan"
}
epSpec := danmtypes.DanmEpSpec {
NetworkID: netId,
NetworkType: neType,
NetworkID: netInfo.Spec.NetworkID,
NetworkName: netInfo.ObjectMeta.Name,
NetworkType: netInfo.Spec.NetworkType,
EndpointID: epid,
Iface: epInput,
Host: host,
Pod: args.podId,
CID: args.containerId,
Netns: args.netns,
Creator: "danm",
}
meta := meta_v1.ObjectMeta {
Name: epid,
Expand Down Expand Up @@ -476,7 +478,7 @@ func deleteInterface(args *cniArgs, syncher *syncher.Syncher, ep danmtypes.DanmE
syncher.PushResult(ep.Spec.NetworkID, errors.New("failed to create danmClient:" + err.Error()), nil)
return
}
netInfo, err := danmClient.DanmV1().DanmNets(args.nameSpace).Get(ep.Spec.NetworkID, meta_v1.GetOptions{})
netInfo, err := danmClient.DanmV1().DanmNets(args.nameSpace).Get(ep.Spec.NetworkName, meta_v1.GetOptions{})
if err != nil {
syncher.PushResult(ep.Spec.NetworkID, errors.New("failed to get DanmNet:"+ err.Error()), nil)
return
Expand Down
7 changes: 6 additions & 1 deletion test/uts/cnidel_test/cnidel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,20 @@ func TestCalculateIfaceName(t *testing.T) {
testDefaultName := "notthechosenone"
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)
}
expDefName := testDefaultName+strconv.Itoa(testSequenceId)
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)
}
expFirstNicName := "eth0"
firstIfaceName := cnidel.CalculateIfaceName(testChosenName, testDefaultName, 0)
if firstIfaceName != expFirstNicName {
t.Errorf("The first interface shall always be named eth0, regardless what the user wants")
}
}

func TestDelegateInterfaceSetup(t *testing.T) {
Expand Down