Repository: threefoldtech/zos
Pull request #423 (2019-11-28 11:21:21)

include nodeID into the Reservation type

codecov[bot]: # [Codecov](https://codecov.io/gh/threefoldtech/zos/pull/423?src=pr&el=h1) Report
> Merging [#423](https://codecov.io/gh/threefoldtech/zos/pull/423?src=pr&el=desc) into [master](https://codecov.io/gh/threefoldtech/zos/commit/87be79a8ae9278d612c5e8655ae85a1993cf4087?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `50%`.

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

```diff
@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
- Coverage   41.06%   41.02%   -0.05%     
==========================================
  Files          64       64              
  Lines        3986     3993       +7     
==========================================
+ Hits         1637     1638       +1     
- Misses       2130     2134       +4     
- Partials      219      221       +2
```


| [Impacted Files](https://codecov.io/gh/threefoldtech/zos/pull/423?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/provision/reservation.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL3Byb3Zpc2lvbi9yZXNlcnZhdGlvbi5nbw==) | `16% <ø> (ø)` | :arrow_up: |
| [pkg/provision/remote\_store.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL3Byb3Zpc2lvbi9yZW1vdGVfc3RvcmUuZ28=) | `0% <0%> (ø)` | :arrow_up: |
| [.../types/provision/tfgrid\_reservation\_container\_1.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL2dlZGlzL3R5cGVzL3Byb3Zpc2lvbi90ZmdyaWRfcmVzZXJ2YXRpb25fY29udGFpbmVyXzEuZ28=) | `100% <100%> (ø)` | :arrow_up: |
| [.../gedis/types/provision/tfgrid\_reservation\_zdb\_1.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL2dlZGlzL3R5cGVzL3Byb3Zpc2lvbi90ZmdyaWRfcmVzZXJ2YXRpb25femRiXzEuZ28=) | `82.35% <50%> (ø)` | :arrow_up: |
| [pkg/provision/crypto.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL3Byb3Zpc2lvbi9jcnlwdG8uZ28=) | `51.35% <50%> (-3.49%)` | :arrow_down: |
| [pkg/gedis/commands\_provision.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL2dlZGlzL2NvbW1hbmRzX3Byb3Zpc2lvbi5nbw==) | `45.24% <57.14%> (-0.25%)` | :arrow_down: |
| [...dis/types/provision/tfgrid\_reservation\_volume\_1.go](https://codecov.io/gh/threefoldtech/zos/pull/423/diff?src=pr&el=tree#diff-cGtnL2dlZGlzL3R5cGVzL3Byb3Zpc2lvbi90ZmdyaWRfcmVzZXJ2YXRpb25fdm9sdW1lXzEuZ28=) | `83.33% <66.66%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/threefoldtech/zos/pull/423?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/423?src=pr&el=footer). Last update [87be79a...7d410c0](https://codecov.io/gh/threefoldtech/zos/pull/423?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

zaibon: Why only in the result ?
Does it make more sense to have it in the reservation itself? A result is anyhow linked to a reservation.

Patch:
diff --git a/pkg/gedis/commands_provision.go b/pkg/gedis/commands_provision.go
index 9046040b..f02514e3 100644
--- a/pkg/gedis/commands_provision.go
+++ b/pkg/gedis/commands_provision.go
@@ -25,7 +25,7 @@ var provisionOrder = map[provision.ReservationType]int{
 }
 
 // Reserve provision.Reserver
-func (g *Gedis) Reserve(r *provision.Reservation, nodeID pkg.Identifier) (string, error) {
+func (g *Gedis) Reserve(r *provision.Reservation) (string, error) {
 	res := types.TfgridReservation1{
 		DataReservation: types.TfgridReservationData1{},
 		// CustomerTid:     r.User, //TODO: wrong type.
@@ -35,20 +35,19 @@ func (g *Gedis) Reserve(r *provision.Reservation, nodeID pkg.Identifier) (string
 	if err != nil {
 		return "", err
 	}
-	nID := nodeID.Identity()
 
 	switch r.Type {
 	case provision.ContainerReservation:
 		res.DataReservation.Containers = []types.TfgridReservationContainer1{
-			containerReservation(w, nID),
+			containerReservation(w, r.NodeID),
 		}
 	case provision.VolumeReservation:
 		res.DataReservation.Volumes = []types.TfgridReservationVolume1{
-			volumeReservation(w, nID),
+			volumeReservation(w, r.NodeID),
 		}
 	case provision.ZDBReservation:
 		res.DataReservation.Zdbs = []types.TfgridReservationZdb1{
-			zdbReservation(w, nID),
+			zdbReservation(w, r.NodeID),
 		}
 	case provision.NetworkReservation:
 		res.DataReservation.Networks = []types.TfgridReservationNetwork1{
@@ -200,7 +199,7 @@ func reservationFromSchema(w types.TfgridReservationWorkload1) (*provision.Reser
 			return nil, err
 		}
 
-		data, err = tmp.ToProvisionType()
+		data, reservation.NodeID, err = tmp.ToProvisionType()
 		if err != nil {
 			return nil, err
 		}
@@ -211,7 +210,7 @@ func reservationFromSchema(w types.TfgridReservationWorkload1) (*provision.Reser
 			return nil, err
 		}
 
-		data, err = tmp.ToProvisionType()
+		data, reservation.NodeID, err = tmp.ToProvisionType()
 		if err != nil {
 			return nil, err
 		}
@@ -233,7 +232,7 @@ func reservationFromSchema(w types.TfgridReservationWorkload1) (*provision.Reser
 			return nil, err
 		}
 
-		data, err = tmp.ToProvisionType()
+		data, reservation.NodeID, err = tmp.ToProvisionType()
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/gedis/types/provision/tfgrid_reservation_container_1.go b/pkg/gedis/types/provision/tfgrid_reservation_container_1.go
index 4d9d61b0..1aab7bc5 100644
--- a/pkg/gedis/types/provision/tfgrid_reservation_container_1.go
+++ b/pkg/gedis/types/provision/tfgrid_reservation_container_1.go
@@ -22,7 +22,7 @@ type TfgridReservationContainer1 struct {
 }
 
 // ToProvisionType converts TfgridReservationContainer1 to provision.Container
-func (c TfgridReservationContainer1) ToProvisionType() (provision.Container, error) {
+func (c TfgridReservationContainer1) ToProvisionType() (provision.Container, string, error) {
 	container := provision.Container{
 		FList:        c.Flist,
 		FlistStorage: c.HubURL,
@@ -45,7 +45,7 @@ func (c TfgridReservationContainer1) ToProvisionType() (provision.Container, err
 		}
 	}
 
-	return container, nil
+	return container, c.NodeID, nil
 }
 
 //TfgridReservationContainerMount1 jsx schema
diff --git a/pkg/gedis/types/provision/tfgrid_reservation_volume_1.go b/pkg/gedis/types/provision/tfgrid_reservation_volume_1.go
index 48f20f4c..c26c9951 100644
--- a/pkg/gedis/types/provision/tfgrid_reservation_volume_1.go
+++ b/pkg/gedis/types/provision/tfgrid_reservation_volume_1.go
@@ -16,7 +16,7 @@ type TfgridReservationVolume1 struct {
 }
 
 // ToProvisionType converts TfgridReservationVolume1 to provision.Volume
-func (v TfgridReservationVolume1) ToProvisionType() (provision.Volume, error) {
+func (v TfgridReservationVolume1) ToProvisionType() (provision.Volume, string, error) {
 	volume := provision.Volume{
 		Size: uint64(v.Size),
 	}
@@ -26,9 +26,9 @@ func (v TfgridReservationVolume1) ToProvisionType() (provision.Volume, error) {
 	case "SSD":
 		volume.Type = provision.SSDDiskType
 	default:
-		return volume, fmt.Errorf("disk type %s not supported", v.Type.String())
+		return volume, v.NodeID, fmt.Errorf("disk type %s not supported", v.Type.String())
 	}
-	return volume, nil
+	return volume, v.NodeID, nil
 }
 
 //TfgridReservationVolume1TypeEnum jsx schema
diff --git a/pkg/gedis/types/provision/tfgrid_reservation_zdb_1.go b/pkg/gedis/types/provision/tfgrid_reservation_zdb_1.go
index 0571065d..289bf760 100644
--- a/pkg/gedis/types/provision/tfgrid_reservation_zdb_1.go
+++ b/pkg/gedis/types/provision/tfgrid_reservation_zdb_1.go
@@ -21,7 +21,7 @@ type TfgridReservationZdb1 struct {
 }
 
 //ToProvisionType converts TfgridReservationZdb1 to provision.ZDB
-func (z TfgridReservationZdb1) ToProvisionType() (provision.ZDB, error) {
+func (z TfgridReservationZdb1) ToProvisionType() (provision.ZDB, string, error) {
 	zdb := provision.ZDB{
 		Size:     uint64(z.Size),
 		Password: z.Password,
@@ -33,7 +33,7 @@ func (z TfgridReservationZdb1) ToProvisionType() (provision.ZDB, error) {
 	case "ssd":
 		zdb.DiskType = pkg.SSDDevice
 	default:
-		return zdb, fmt.Errorf("device type %s not supported", z.DiskType.String())
+		return zdb, z.NodeID, fmt.Errorf("device type %s not supported", z.DiskType.String())
 	}
 
 	switch z.Mode.String() {
@@ -42,10 +42,10 @@ func (z TfgridReservationZdb1) ToProvisionType() (provision.ZDB, error) {
 	case "user":
 		zdb.Mode = pkg.ZDBModeUser
 	default:
-		return zdb, fmt.Errorf("0-db mode %s not supported", z.Mode.String())
+		return zdb, z.NodeID, fmt.Errorf("0-db mode %s not supported", z.Mode.String())
 	}
 
-	return zdb, nil
+	return zdb, z.NodeID, nil
 }
 
 //TfgridReservationZdb1ModeEnum jsx schema
diff --git a/pkg/provision/crypto.go b/pkg/provision/crypto.go
index 634efc89..78a56b29 100644
--- a/pkg/provision/crypto.go
+++ b/pkg/provision/crypto.go
@@ -17,11 +17,11 @@ func (r *Reservation) Sign(privateKey ed25519.PrivateKey) error {
 	//FIME: Since the ID is only set when the reservation is sent to bcdb
 	// we cannot use it in the signature. This is a problem
 
-	// _, err := buf.WriteString(r.ID)
-	// if err != nil {
-	// 	return err
-	// }
-	_, err := buf.WriteString(r.User)
+	_, err := buf.WriteString(r.NodeID)
+	if err != nil {
+		return err
+	}
+	_, err = buf.WriteString(r.User)
 	if err != nil {
 		return err
 	}
@@ -48,7 +48,12 @@ func Verify(r *Reservation) error {
 	//FIME: Since the ID is only set when the reservation is sent to bcdb
 	// we cannot use it in the signature. This is a problem
 
-	_, err := buf.WriteString(r.User)
+	_, err := buf.WriteString(r.NodeID)
+	if err != nil {
+		return err
+	}
+
+	_, err = buf.WriteString(r.User)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/provision/remote_store.go b/pkg/provision/remote_store.go
index b78eccc1..c1682c47 100644
--- a/pkg/provision/remote_store.go
+++ b/pkg/provision/remote_store.go
@@ -23,8 +23,11 @@ func NewHTTPStore(url string) *HTTPStore {
 }
 
 // Reserve adds a reservation to the BCDB
-func (s *HTTPStore) Reserve(r *Reservation, nodeID pkg.Identifier) (string, error) {
-	url := fmt.Sprintf("%s/reservations/%s", s.baseURL, nodeID.Identity())
+func (s *HTTPStore) Reserve(r *Reservation) (string, error) {
+	if r.NodeID == "" {
+		return "", fmt.Errorf("nodeID cannot be empty in the reservation")
+	}
+	url := fmt.Sprintf("%s/reservations/%s", s.baseURL, r.NodeID)
 
 	buf := &bytes.Buffer{}
 
diff --git a/pkg/provision/reservation.go b/pkg/provision/reservation.go
index d48245a0..72293805 100644
--- a/pkg/provision/reservation.go
+++ b/pkg/provision/reservation.go
@@ -39,6 +39,8 @@ var (
 type Reservation struct {
 	// ID of the reservation
 	ID string `json:"id"`
+	// NodeID of the node where to deploy this reservation
+	NodeID string `json:"node_id"`
 	// Identification of the user requesting the reservation
 	User string `json:"user_id"`
 	// Type of the reservation (container, zdb, vm, etc...)
diff --git a/tools/tfuser/cmds_provision.go b/tools/tfuser/cmds_provision.go
index bb9915d9..e8765cb1 100644
--- a/tools/tfuser/cmds_provision.go
+++ b/tools/tfuser/cmds_provision.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	"github.com/threefoldtech/zos/pkg"
 	"github.com/threefoldtech/zos/pkg/identity"
 	"github.com/threefoldtech/zos/pkg/provision"
 
@@ -61,28 +60,29 @@ func cmdsProvision(c *cli.Context) error {
 
 	r.Duration = duration
 	r.Created = time.Now()
-
 	// set the user ID into the reservation schema
 	r.User = keypair.Identity()
 
-	if err := r.Sign(keypair.PrivateKey); err != nil {
-		return errors.Wrap(err, "failed to sign the reservation")
-	}
+	for _, nodeID := range nodeIDs {
+		r.NodeID = nodeID
 
-	if err := output(path, r); err != nil {
-		return errors.Wrapf(err, "failed to write provision schema to %s after signature", path)
-	}
+		if err := r.Sign(keypair.PrivateKey); err != nil {
+			return errors.Wrap(err, "failed to sign the reservation")
+		}
 
-	for _, nodeID := range nodeIDs {
-		id, err := client.Reserve(r, pkg.StrIdentifier(nodeID))
+		id, err := client.Reserve(r)
 		if err != nil {
 			return errors.Wrap(err, "failed to send reservation")
 		}
 
-		fmt.Printf("Reservation for %v send to node %s\n", duration, nodeID)
+		fmt.Printf("Reservation for %v send to node %s\n", duration, r.NodeID)
 		fmt.Printf("Resource: %v\n", id)
 	}
 
+	if err := output(path, r); err != nil {
+		return errors.Wrapf(err, "failed to write provision schema to %s after signature", path)
+	}
+
 	return nil
 }
 
diff --git a/tools/tfuser/main.go b/tools/tfuser/main.go
index 17945bdc..3d1492ec 100644
--- a/tools/tfuser/main.go
+++ b/tools/tfuser/main.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/url"
 
-	"github.com/threefoldtech/zos/pkg"
 	"github.com/threefoldtech/zos/pkg/gedis"
 
 	"github.com/threefoldtech/zos/pkg/provision"
@@ -302,7 +301,7 @@ func main() {
 }
 
 type reserver interface {
-	Reserve(r *provision.Reservation, nodeID pkg.Identifier) (string, error)
+	Reserve(r *provision.Reservation) (string, error)
 }
 type clientIface interface {
 	network.TNoDB


Test patch:
diff --git a/pkg/gedis/commands_provision_test.go b/pkg/gedis/commands_provision_test.go
index 3175a00e..12780d47 100644
--- a/pkg/gedis/commands_provision_test.go
+++ b/pkg/gedis/commands_provision_test.go
@@ -164,8 +164,8 @@ func TestProvisionReserve(t *testing.T) {
 		pool: pool,
 	}
 
-	id := pkg.StrIdentifier("101")
 	reservation := provision.Reservation{
+		NodeID:  "101",
 		Type:    provision.ContainerReservation,
 		ID:      "10",
 		Created: time.Now(),
@@ -205,7 +205,7 @@ func TestProvisionReserve(t *testing.T) {
 			"id": 10,
 		}), nil)
 
-	result, err := gedis.Reserve(&reservation, id)
+	result, err := gedis.Reserve(&reservation)
 
 	require.NoError(err)
 	require.Equal("10", result)
diff --git a/pkg/gedis/types/provision/model_test.go b/pkg/gedis/types/provision/model_test.go
index 84746d69..227605e0 100644
--- a/pkg/gedis/types/provision/model_test.go
+++ b/pkg/gedis/types/provision/model_test.go
@@ -140,7 +140,7 @@ func TestTfgridReservationContainer1_ToProvisionType(t *testing.T) {
 				NetworkConnection: tt.fields.NetworkConnection,
 				StatsAggregator:   tt.fields.StatsAggregator,
 			}
-			got, err := c.ToProvisionType()
+			got, _, err := c.ToProvisionType()
 			if !tt.wantErr {
 				require.NoError(t, err)
 				assert.DeepEqual(t, tt.want, got)
@@ -204,7 +204,7 @@ func TestTfgridReservationVolume1_ToProvisionType(t *testing.T) {
 				Type:            tt.fields.Type,
 				StatsAggregator: tt.fields.StatsAggregator,
 			}
-			got, err := v.ToProvisionType()
+			got, _, err := v.ToProvisionType()
 			if !tt.wantErr {
 				require.NoError(t, err)
 				assert.DeepEqual(t, tt.want, got)
@@ -308,7 +308,7 @@ func TestTfgridReservationZdb1_ToProvisionType(t *testing.T) {
 				DiskType:      tt.fields.DiskType,
 				Public:        tt.fields.Public,
 			}
-			got, err := z.ToProvisionType()
+			got, _, err := z.ToProvisionType()
 			if !tt.wantErr {
 				require.NoError(t, err)
 				assert.DeepEqual(t, tt.want, got)
diff --git a/pkg/provision/crypto_test.go b/pkg/provision/crypto_test.go
index a80b4e38..1a2e6d1b 100644
--- a/pkg/provision/crypto_test.go
+++ b/pkg/provision/crypto_test.go
@@ -20,10 +20,11 @@ func TestVerifySignature(t *testing.T) {
 	require.NoError(t, err)
 
 	r := &Reservation{
-		ID:   "reservationID",
-		User: keyPair.Identity(),
-		Type: ContainerReservation,
-		Data: data,
+		ID:     "reservationID",
+		NodeID: "node1",
+		User:   keyPair.Identity(),
+		Type:   ContainerReservation,
+		Data:   data,
 	}
 
 	err = r.Sign(keyPair.PrivateKey)