Cmd fixes and improvements (#903)

* rename ReloadBeacon service and cmd to LoadBeacon
* change the way Home service works to daemon level
* add load beacon process after generating new keys with cmd
* improve some cmd descriptions
* give more functionality to util check cmd
* change the way we check if beacon id is set on cmds

cmd/drand-cli/cli.go

@@ -18,6 +18,8 @@ import (
 	"strings"
 	"sync"
 
+	common2 "github.com/drand/drand/protobuf/common"
+
 	"github.com/drand/drand/core/migration"
 
 	"github.com/drand/drand/common/scheme"
@@ -325,10 +327,10 @@ var appCommands = []*cli.Command{
 		},
 	},
 	{
-		Name:   "reload",
-		Usage:  "Launch a sharing protocol which has been previously stopped",
+		Name:   "load",
+		Usage:  "Launch a sharing protocol from filesystem",
 		Flags:  toArray(controlFlag, beaconIDFlag),
-		Action: reloadCmd,
+		Action: loadCmd,
 	},
 	{
 		Name:  "follow",
@@ -339,13 +341,27 @@ var appCommands = []*cli.Command{
 	},
 	{
 		Name: "generate-keypair",
-		Usage: "Generate the longterm keypair (drand.private, drand.public)" +
-			"for this node.\n",
+		Usage: "Generate the longterm keypair (drand.private, drand.public) " +
+			"for this node, and load it on the drand daemon if it is up and running.\n",
 		ArgsUsage: "<address> is the address other nodes will be able to contact this node on (specified as 'private-listen' to the daemon)",
-		Flags:     toArray(folderFlag, insecureFlag, beaconIDFlag),
+		Flags:     toArray(controlFlag, folderFlag, insecureFlag, beaconIDFlag),
 		Action: func(c *cli.Context) error {
 			banner()
-			return keygenCmd(c)
+			err := keygenCmd(c)
+
+			// If keys were generated successfully, daemon needs to load them
+			// In other to load them, we run LoadBeacon cmd.
+			//
+			// TIP: If an error is found, it may indicate daemon is not running. If that is the case, keys will be loaded
+			// on drand startup.
+			if err == nil {
+				err2 := loadCmd(c)
+				if err2 != nil {
+					fmt.Fprintf(os.Stdout, "Keys couldn't be loaded on drand daemon. If it is not running, "+
+						"these new keys will loaded on startup. Err: %s", err2)
+				}
+			}
+			return err
 		},
 		Before: checkMigration,
 	},
@@ -383,7 +399,7 @@ var appCommands = []*cli.Command{
 			},
 			{
 				Name:      "chain-info",
-				Usage:     "Get the binding chain information that this nodes participates to",
+				Usage:     "Get the binding chain information that this node participates to",
 				ArgsUsage: "`ADDRESS1` `ADDRESS2` ... provides the addresses of the node to try to contact to.",
 				Flags:     toArray(tlsCertFlag, insecureFlag, hashOnly, hashInfoNoReq),
 				Action:    getChainInfo,
@@ -400,7 +416,7 @@ var appCommands = []*cli.Command{
 					" in the group for accessibility over the gRPC communication. If the node " +
 					" is not running behind TLS, you need to pass the tls-disable flag. You can " +
 					"also check a whole group's connectivity with the group flag.",
-				Flags:  toArray(groupFlag, certsDirFlag, insecureFlag, verboseFlag),
+				Flags:  toArray(groupFlag, certsDirFlag, insecureFlag, verboseFlag, beaconIDFlag),
 				Action: checkConnection,
 			},
 			{
@@ -714,17 +730,24 @@ func getThreshold(c *cli.Context) (int, error) {
 
 func checkConnection(c *cli.Context) error {
 	var names []string
+	beaconID := ""
+
 	if c.IsSet(groupFlag.Name) {
+		if c.IsSet(beaconIDFlag.Name) {
+			return fmt.Errorf("id flag is not reqired when using group flag")
+		}
 		if err := testEmptyGroup(c.String(groupFlag.Name)); err != nil {
 			return err
 		}
 		group := new(key.Group)
 		if err := key.Load(c.String(groupFlag.Name), group); err != nil {
 			return fmt.Errorf("loading group failed: %s", err)
 		}
+
 		for _, id := range group.Nodes {
 			names = append(names, id.Address())
 		}
+		beaconID = group.ID
 	} else if c.Args().Present() {
 		for _, serverAddr := range c.Args().Slice() {
 			_, _, err := gonet.SplitHostPort(serverAddr)
@@ -733,16 +756,25 @@ func checkConnection(c *cli.Context) error {
 			}
 			names = append(names, serverAddr)
 		}
+		beaconID = c.String(beaconIDFlag.Name)
 	} else {
 		return fmt.Errorf("drand: check-group expects a list of identities or %s flag", groupFlag.Name)
 	}
+
 	conf := contextToConfig(c)
+	isVerbose := c.IsSet(verboseFlag.Name)
+	allGood := true
+	isIdentityCheck := c.IsSet(groupFlag.Name) || c.IsSet(beaconIDFlag.Name)
+	invalidIds := make([]string, 0)
 
-	var isVerbose = c.IsSet(verboseFlag.Name)
-	var allGood = true
-	var invalidIds []string
 	for _, address := range names {
-		err := checkIdentityAddress(conf, address, !c.Bool(insecureFlag.Name))
+		var err error
+		if isIdentityCheck {
+			err = checkIdentityAddress(conf, address, !c.Bool(insecureFlag.Name), beaconID)
+		} else {
+			err = remotePingToNode(address)
+		}
+
 		if err != nil {
 			if isVerbose {
 				fmt.Fprintf(output, "drand: error checking id %s: %s\n", address, err)
@@ -761,12 +793,15 @@ func checkConnection(c *cli.Context) error {
 	return nil
 }
 
-func checkIdentityAddress(conf *core.Config, addr string, tls bool) error {
+func checkIdentityAddress(conf *core.Config, addr string, tls bool, beaconID string) error {
 	peer := net.CreatePeer(addr, tls)
 	client := net.NewGrpcClientFromCertManager(conf.Certs())
+
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
-	identityResp, err := client.GetIdentity(ctx, peer, &drand.IdentityRequest{})
+
+	metadata := &common2.Metadata{BeaconID: beaconID}
+	identityResp, err := client.GetIdentity(ctx, peer, &drand.IdentityRequest{Metadata: metadata})
 	if err != nil {
 		return err
 	}

cmd/drand-cli/cli_test.go

@@ -282,7 +282,7 @@ func TestUtilCheck(t *testing.T) {
 
 	// run the check tool it should fail because key and address are not
 	// consistent
-	check := []string{"drand", "util", "check", "--tls-disable", listenAddr}
+	check := []string{"drand", "util", "check", "--tls-disable", "--id", beaconID, listenAddr}
 	require.Error(t, CLI().Run(check))
 
 	// cancel the daemon and make it listen on the right address
@@ -709,7 +709,7 @@ func TestDrandReloadBeacon(t *testing.T) {
 	time.Sleep(1 * time.Second)
 
 	// try to reload a beacon which is already loaded
-	err := instances[3].reload(beaconID)
+	err := instances[3].load(beaconID)
 	require.Error(t, err)
 
 	// wait some time to generate some randomness
@@ -723,7 +723,7 @@ func TestDrandReloadBeacon(t *testing.T) {
 	testPing(t, instances[3].ctrlPort)
 
 	// reload a beacon
-	err = instances[3].reload(beaconID)
+	err = instances[3].load(beaconID)
 	require.NoError(t, err)
 
 	// test beacon process status
@@ -843,10 +843,10 @@ func (d *drandInstance) share(t *testing.T, leaderURL, beaconID string) {
 	}()
 }
 
-func (d *drandInstance) reload(beaconID string) error {
+func (d *drandInstance) load(beaconID string) error {
 	reloadArgs := []string{
 		"drand",
-		"reload",
+		"load",
 		"--control", d.ctrlPort,
 		"--id", beaconID,
 	}

cmd/drand-cli/control.go

@@ -1,6 +1,7 @@
 package drand
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -201,19 +202,19 @@ func leadShareCmd(c *cli.Context) error {
 	return groupOut(c, group)
 }
 
-func reloadCmd(c *cli.Context) error {
+func loadCmd(c *cli.Context) error {
 	client, err := controlClient(c)
 	if err != nil {
 		return err
 	}
 
 	beaconID := getBeaconID(c)
-	_, err = client.ReloadBeacon(beaconID)
+	_, err = client.LoadBeacon(beaconID)
 	if err != nil {
 		return fmt.Errorf("could not reload the beacon process [%s]: %s", beaconID, err)
 	}
 
-	fmt.Fprintf(output, "Beacon process [%s] is alive again \n", beaconID)
+	fmt.Fprintf(output, "Beacon process [%s] was loaded on drand.\n", beaconID)
 	return nil
 }
 
@@ -257,7 +258,7 @@ func reshareCmd(c *cli.Context) error {
 
 		oldPath = c.String(oldGroupFlag.Name)
 
-		if beaconID != "" {
+		if c.IsSet(beaconIDFlag.Name) {
 			return fmt.Errorf("beacon id flag is not required when using --%s", oldGroupFlag.Name)
 		}
 
@@ -312,7 +313,7 @@ func leadReshareCmd(c *cli.Context) error {
 		}
 		oldPath = c.String(oldGroupFlag.Name)
 
-		if beaconID != "" {
+		if c.IsSet(beaconIDFlag.Name) {
 			return fmt.Errorf("beacon id flag is not required when using --%s", oldGroupFlag.Name)
 		}
 
@@ -417,6 +418,21 @@ func pingpongCmd(c *cli.Context) error {
 	return nil
 }
 
+func remotePingToNode(addr string) error {
+	peer := net.CreatePeer(addr, false)
+	client := net.NewGrpcClient()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	_, err := client.Home(ctx, peer, &drand.HomeRequest{})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 //nolint:gocyclo
 func statusCmd(c *cli.Context) error {
 	client, err := controlClient(c)
@@ -426,9 +442,9 @@ func statusCmd(c *cli.Context) error {
 
 	listIds := c.IsSet(listIdsFlag.Name)
 	allIds := c.IsSet(allBeaconsFlag.Name)
-	beaconID := c.String(beaconIDFlag.Name)
+	beaconID := c.IsSet(beaconIDFlag.Name)
 
-	if beaconID != "" && (allIds || listIds) {
+	if beaconID && (allIds || listIds) {
 		return fmt.Errorf("drand: can't use --%s with --%s or --%s flags at the same time",
 			beaconIDFlag.Name, allBeaconsFlag.Name, listIdsFlag.Name)
 	}

cmd/drand-cli/daemon.go

@@ -17,7 +17,7 @@ func startCmd(c *cli.Context) error {
 	}
 
 	// Check stores and start BeaconProcess
-	err = drandDaemon.LoadBeacons(c.String(metricsFlag.Name))
+	err = drandDaemon.LoadBeaconsFromDisk(c.String(metricsFlag.Name))
 	if err != nil {
 		return fmt.Errorf("couldn't load existing beacons: %s", err)
 	}
@@ -32,15 +32,18 @@ func stopDaemon(c *cli.Context) error {
 		return err
 	}
 
-	beaconID := c.String(beaconIDFlag.Name)
-	_, err = ctrlClient.Shutdown(beaconID)
+	isBeaconIDSet := c.IsSet(beaconIDFlag.Name)
+	if isBeaconIDSet {
+		beaconID := getBeaconID(c)
+		_, err = ctrlClient.Shutdown(beaconID)
 
-	if beaconID != "" {
 		if err != nil {
 			return fmt.Errorf("error stopping beacon process [%s]: %w", beaconID, err)
 		}
 		fmt.Fprintf(output, "beacon process [%s] stopped correctly. Bye.\n", beaconID)
 	} else {
+		_, err = ctrlClient.Shutdown("")
+
 		if err != nil {
 			return fmt.Errorf("error stopping drand daemon: %w", err)
 		}

core/drand_daemon.go

@@ -224,15 +224,15 @@ func (dd *DrandDaemon) RemoveBeaconHandler(beaconID string, bp *BeaconProcess) {
 
 // LoadBeacons checks for existing stores and creates the corresponding BeaconProcess
 // accordingly to each stored BeaconID
-func (dd *DrandDaemon) LoadBeacons(metricsFlag string) error {
+func (dd *DrandDaemon) LoadBeaconsFromDisk(metricsFlag string) error {
 	// Load possible existing stores
 	stores, err := key.NewFileStores(dd.opts.ConfigFolderMB())
 	if err != nil {
 		return err
 	}
 
 	for beaconID, fs := range stores {
-		bp, err := dd.LoadBeacon(beaconID, fs)
+		bp, err := dd.LoadBeaconFromStore(beaconID, fs)
 		if err != nil {
 			return err
 		}
@@ -246,12 +246,12 @@ func (dd *DrandDaemon) LoadBeacons(metricsFlag string) error {
 	return nil
 }
 
-func (dd *DrandDaemon) ReloadBeaconFromDisk(beaconID string) (*BeaconProcess, error) {
+func (dd *DrandDaemon) LoadBeaconFromDisk(beaconID string) (*BeaconProcess, error) {
 	store := key.NewFileStore(dd.opts.ConfigFolderMB(), beaconID)
-	return dd.LoadBeacon(beaconID, store)
+	return dd.LoadBeaconFromStore(beaconID, store)
 }
 
-func (dd *DrandDaemon) LoadBeacon(beaconID string, store key.Store) (*BeaconProcess, error) {
+func (dd *DrandDaemon) LoadBeaconFromStore(beaconID string, store key.Store) (*BeaconProcess, error) {
 	bp, err := dd.InstantiateBeaconProcess(beaconID, store)
 	if err != nil {
 		fmt.Printf("beacon id [%s]: can't instantiate randomness beacon. err: %s \n", beaconID, err)

core/drand_daemon_control.go

@@ -161,7 +161,7 @@ func (dd *DrandDaemon) Shutdown(ctx context.Context, in *drand.ShutdownRequest)
 }
 
 // ReloadBeacon
-func (dd *DrandDaemon) ReloadBeacon(ctx context.Context, in *drand.ReloadBeaconRequest) (*drand.ReloadBeaconResponse, error) {
+func (dd *DrandDaemon) LoadBeacon(ctx context.Context, in *drand.LoadBeaconRequest) (*drand.LoadBeaconResponse, error) {
 	beaconID, err := dd.readBeaconID(in.GetMetadata())
 	if err != nil {
 		return nil, err
@@ -172,13 +172,13 @@ func (dd *DrandDaemon) ReloadBeacon(ctx context.Context, in *drand.ReloadBeaconR
 		return nil, fmt.Errorf("beacon id [%s] is already running", beaconID)
 	}
 
-	_, err = dd.ReloadBeaconFromDisk(beaconID)
+	_, err = dd.LoadBeaconFromDisk(beaconID)
 	if err != nil {
 		return nil, err
 	}
 
 	metadata := common.NewMetadata(dd.version.ToProto())
-	return &drand.ReloadBeaconResponse{Metadata: metadata}, nil
+	return &drand.LoadBeaconResponse{Metadata: metadata}, nil
 }
 
 // BackupDatabase triggers a backup of the primary database.

core/drand_daemon_public.go

@@ -3,6 +3,8 @@ package core
 import (
 	"context"
 
+	"github.com/drand/drand/protobuf/common"
+
 	"github.com/drand/drand/protobuf/drand"
 )
 
@@ -60,12 +62,9 @@ func (dd *DrandDaemon) PrivateRand(c context.Context, in *drand.PrivateRandReque
 
 // Home provides the address the local node is listening
 func (dd *DrandDaemon) Home(c context.Context, in *drand.HomeRequest) (*drand.HomeResponse, error) {
-	bp, err := dd.getBeaconProcessFromRequest(in.GetMetadata())
-	if err != nil {
-		return nil, err
-	}
+	ctx := common.NewMetadata(dd.version.ToProto())
 
-	return bp.Home(c, in)
+	return &drand.HomeResponse{Metadata: ctx}, nil
 }
 
 // ChainInfo replies with the chain information this node participates to