Repository: threefoldtech/zos
Pull request #447 (2019-12-16 14:15:59)

Make sure zdb macvlan always have the same mac
This will make SLAAC assign the same IP always
to this interface.

muhamadazmy: I have tested this and works as expected, the zdb now always has an IP associated with it's volume ID which is persisted on the disk.
codecov[bot]: # [Codecov](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=h1) Report
> Merging [#447](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=desc) into [master](https://codecov.io/gh/threefoldtech/zos/commit/5571c15766cf1c81a8c8c7a642fbd77d8b7b36ad?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/threefoldtech/zos/pull/447/graphs/tree.svg?width=650&token=f7Vmq1OAEm&height=150&src=pr)](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
+ Coverage   39.36%   39.38%   +0.01%     
==========================================
  Files          68       68              
  Lines        4539     4540       +1     
==========================================
+ Hits         1787     1788       +1     
  Misses       2502     2502              
  Partials      250      250
```


| [Impacted Files](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/provision/zdb.go](https://codecov.io/gh/threefoldtech/zos/pull/447/diff?src=pr&el=tree#diff-cGtnL3Byb3Zpc2lvbi96ZGIuZ28=) | `45.27% <100%> (+0.27%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=footer). Last update [5571c15...f5d4f89](https://codecov.io/gh/threefoldtech/zos/pull/447?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/network.go b/pkg/network.go
index e6ebb9d2..f6200d7e 100644
--- a/pkg/network.go
+++ b/pkg/network.go
@@ -37,7 +37,8 @@ type Networker interface {
 	// ZDBPrepare creates a network namespace with a macvlan interface into it
 	// to allow the 0-db container to be publicly accessible
 	// it retusn the name of the network namespace created
-	ZDBPrepare() (string, error)
+	// hw is an optional hardware address that will be set on the new interface
+	ZDBPrepare(hw net.HardwareAddr) (string, error)
 
 	// Addrs return the IP addresses of interface
 	// if the interface is in a network namespace netns needs to be not empty
diff --git a/pkg/network/macvlan/macvlan.go b/pkg/network/macvlan/macvlan.go
index 0e2e56ec..a750cc56 100644
--- a/pkg/network/macvlan/macvlan.go
+++ b/pkg/network/macvlan/macvlan.go
@@ -87,8 +87,14 @@ func Create(name string, master string, netns ns.NetNS) (*netlink.Macvlan, error
 }
 
 // Install configures a macvlan interfaces created with Create method
-func Install(link *netlink.Macvlan, ips []*net.IPNet, routes []*netlink.Route, netns ns.NetNS) error {
+func Install(link *netlink.Macvlan, hw net.HardwareAddr, ips []*net.IPNet, routes []*netlink.Route, netns ns.NetNS) error {
 	return netns.Do(func(_ ns.NetNS) error {
+		if hw != nil && len(hw) != 0 {
+			if err := netlink.LinkSetHardwareAddr(link, hw); err != nil {
+				return err
+			}
+		}
+
 		name := link.Attrs().Name
 
 		for _, ip := range ips {
diff --git a/pkg/network/networker.go b/pkg/network/networker.go
index c64b318d..be89964e 100644
--- a/pkg/network/networker.go
+++ b/pkg/network/networker.go
@@ -184,8 +184,7 @@ func (n *networker) Leave(networkdID pkg.NetID, containerID string) error {
 
 // ZDBPrepare sends a macvlan interface into the
 // network namespace of a ZDB container
-func (n networker) ZDBPrepare() (string, error) {
-
+func (n networker) ZDBPrepare(hw net.HardwareAddr) (string, error) {
 	netNSName, err := ifaceutil.RandomName("zdb-ns-")
 	if err != nil {
 		return "", err
@@ -212,8 +211,9 @@ func (n networker) ZDBPrepare() (string, error) {
 		return "", errors.Wrap(err, "failed to create public mac vlan interface")
 	}
 
+	log.Debug().Str("HW", hw.String()).Str("macvlan", macVlan.Name).Msg("setting hw address on link")
 	// we don't set any route or ip
-	if err := macvlan.Install(macVlan, []*net.IPNet{}, []*netlink.Route{}, netNs); err != nil {
+	if err := macvlan.Install(macVlan, hw, []*net.IPNet{}, []*netlink.Route{}, netNs); err != nil {
 		return "", err
 	}
 
diff --git a/pkg/network/public.go b/pkg/network/public.go
index 7caa229c..97be26b4 100644
--- a/pkg/network/public.go
+++ b/pkg/network/public.go
@@ -25,6 +25,7 @@ func CreatePublicNS(iface *types.PubIface, nodeID pkg.Identifier) error {
 		pubNS    ns.NetNS
 		pubIface *netlink.Macvlan
 		err      error
+		mac      net.HardwareAddr
 	)
 
 	if !namespace.Exists(types.PublicNamespace) {
@@ -43,11 +44,7 @@ func CreatePublicNS(iface *types.PubIface, nodeID pkg.Identifier) error {
 			if err != nil {
 				return errors.Wrap(err, "failed to create public mac vlan interface")
 			}
-			mac := ifaceutil.HardwareAddrFromInputBytes([]byte(nodeID.Identity() + publicNsMACDerivationSuffix))
-			if err = ifaceutil.SetMAC(types.PublicIface, mac, pubNS); err != nil {
-				return errors.Wrap(err, "failed to set mac address on public mac vlan interface")
-			}
-			log.Debug().Str("mac", mac.String()).Msg("Set mac address on public mac vlan interface")
+			mac = ifaceutil.HardwareAddrFromInputBytes([]byte(nodeID.Identity() + publicNsMACDerivationSuffix))
 		default:
 			return fmt.Errorf("unsupported public interface type %s", iface.Type)
 		}
@@ -105,7 +102,7 @@ func CreatePublicNS(iface *types.PubIface, nodeID pkg.Identifier) error {
 		return err
 	}
 
-	if err := macvlan.Install(pubIface, ips, routes, pubNS); err != nil {
+	if err := macvlan.Install(pubIface, mac, ips, routes, pubNS); err != nil {
 		return err
 	}
 
diff --git a/pkg/provision/zdb.go b/pkg/provision/zdb.go
index 7a0c9f23..864bc3ff 100644
--- a/pkg/provision/zdb.go
+++ b/pkg/provision/zdb.go
@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/cenkalti/backoff/v3"
+	"github.com/threefoldtech/zos/pkg/network/ifaceutil"
 	"github.com/threefoldtech/zos/pkg/zdb"
 
 	"github.com/pkg/errors"
@@ -133,6 +134,8 @@ func createZdbContainer(ctx context.Context, allocation pkg.Allocation, mode pkg
 		slog = log.With().Str("containerID", string(name)).Logger()
 	)
 
+	hw := ifaceutil.HardwareAddrFromInputBytes([]byte(allocation.VolumeID))
+
 	slog.Debug().Str("flist", zdbFlistURL).Msg("mounting flist")
 	rootFS, err := flist.Mount(zdbFlistURL, "", pkg.MountOptions{
 		Limit:    10,
@@ -153,7 +156,7 @@ func createZdbContainer(ctx context.Context, allocation pkg.Allocation, mode pkg
 	}
 
 	// create the network namespace and macvlan for the 0-db container
-	netNsName, err := network.ZDBPrepare()
+	netNsName, err := network.ZDBPrepare(hw)
 	if err != nil {
 		if err := flist.Umount(rootFS); err != nil {
 			slog.Error().Err(err).Str("path", rootFS).Msgf("failed to unmount")
diff --git a/pkg/stubs/network_stub.go b/pkg/stubs/network_stub.go
index cf9adf13..68e6c0cd 100644
--- a/pkg/stubs/network_stub.go
+++ b/pkg/stubs/network_stub.go
@@ -96,8 +96,8 @@ func (s *NetworkerStub) Leave(arg0 pkg.NetID, arg1 string) (ret0 error) {
 	return
 }
 
-func (s *NetworkerStub) ZDBPrepare() (ret0 string, ret1 error) {
-	args := []interface{}{}
+func (s *NetworkerStub) ZDBPrepare(arg0 []uint8) (ret0 string, ret1 error) {
+	args := []interface{}{arg0}
 	result, err := s.client.Request(s.module, s.object, "ZDBPrepare", args...)
 	if err != nil {
 		panic(err)


Test patch:
diff --git a/pkg/provision/zdb_test.go b/pkg/provision/zdb_test.go
index 61fd2944..4d204863 100644
--- a/pkg/provision/zdb_test.go
+++ b/pkg/provision/zdb_test.go
@@ -173,8 +173,10 @@ func TestZDBProvisionNoMappingContainerDoesNotExists(t *testing.T) {
 			ReadOnly: false,
 		}).Return("/path/to/volume", nil)
 
+	// The hw address in this mock call, is based on the VolumeID above. changing the VolumeID
+	// will require this HW address to change
 	client.On("Request", "network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
-		"ZDBPrepare").Return("net-ns", nil)
+		"ZDBPrepare", []byte{0x7e, 0xcc, 0xc3, 0x81, 0xa2, 0x2e}).Return("net-ns", nil)
 
 	client.On("Request", "container", zbus.ObjectID{Name: "container", Version: "0.0.1"},
 		"Run",