Skip to content

Commit

Permalink
nodeID generation not based on libp2p (#3450)
Browse files Browse the repository at this point in the history
Enable users to set their own nodeID. Users can either set their node
name manually, or chose any of the following providers to automatically
set the name:
- `puuid`: (default) generate a node name using `n-{uuid}` pattern, such
as `n-f1bab231-68ad-4c72-bab6-580cd49bf521`
- `uuid`: generate uuid as a node name
- `hostname`: use the hostname as the node id, but replacing any `.`
with `-` to be compatible with nats
- `aws`: use the EC2 instance name, if the node is deployed on aws
- `gcp`: use the VM's id, if the node is deployed on gcp

### Persisted Node Name
These providers will only be called if no existing node name is found in
`config.yaml`, cli `--name` flag or env variables. Once a node name is
generated, it will be persisted in `config.yaml`

### Examples
```
# set the node name manually
bacalhau serve --name my-custom-name

# use a puuid as the node name (default)
bacalhau serve

# use hostname as the node name
bacalhau serve --name-provider hostname
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added node name configuration options, supporting various providers
like hostname, AWS, GCP, UUID, and PUUID.
- Introduced a new naming convention for nodes and job executions to
enhance readability and uniqueness.
- **Refactor**
- Updated ID generation across the application to use `ShortUUID` and
`ShortNodeID` for job IDs, node IDs, and execution IDs, improving
consistency and clarity in displays and logs.
- **Tests**
- Added comprehensive testing for new node naming strategies and ID
generation methods.
- **Bug Fixes**
- Fixed potential subscription issues in NATS subjects by restricting
characters in node IDs.
- **Documentation**
- Updated internal documentation to reflect changes in ID generation and
node naming conventions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
wdbaruni committed Feb 20, 2024
1 parent 2045425 commit 3c59c46
Show file tree
Hide file tree
Showing 34 changed files with 563 additions and 58 deletions.
2 changes: 1 addition & 1 deletion cmd/cli/describe/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (s *DescribeSuite) TestDescribeJob() {
// Short job id
_, out, err = s.ExecuteTestCobraCommand("describe",
"--api-host", s.Host,
idgen.ShortID(submittedJob.Metadata.ID),
idgen.ShortUUID(submittedJob.Metadata.ID),
"--api-port", fmt.Sprint(s.Port),
)

Expand Down
2 changes: 1 addition & 1 deletion cmd/cli/job/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (o *DescribeOptions) printOutputs(cmd *cobra.Command, executions []*models.
if len(outputs) == 1 {
cmd.Print(out)
} else {
cmd.Printf("%sExecution %s:\n%s", separator, idgen.ShortID(id), out)
cmd.Printf("%sExecution %s:\n%s", separator, idgen.ShortUUID(id), out)
}
separator = "\n"
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/cli/job/executions.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ var (
}
executionColumnID = output.TableColumn[*models.Execution]{
ColumnConfig: table.ColumnConfig{Name: "ID", WidthMax: 10, WidthMaxEnforcer: text.WrapText},
Value: func(e *models.Execution) string { return idgen.ShortID(e.ID) },
Value: func(e *models.Execution) string { return idgen.ShortUUID(e.ID) },
}
executionColumnNodeID = output.TableColumn[*models.Execution]{
ColumnConfig: table.ColumnConfig{Name: "Node ID", WidthMax: 10, WidthMaxEnforcer: text.WrapText},
Value: func(e *models.Execution) string { return idgen.ShortID(e.NodeID) },
Value: func(e *models.Execution) string { return idgen.ShortNodeID(e.NodeID) },
}
executionColumnRev = output.TableColumn[*models.Execution]{
ColumnConfig: table.ColumnConfig{Name: "Rev.", WidthMax: 4, WidthMaxEnforcer: text.WrapText},
Expand Down
4 changes: 2 additions & 2 deletions cmd/cli/job/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ var historyColumns = []output.TableColumn[*models.JobHistory]{
},
{
ColumnConfig: table.ColumnConfig{Name: "Exec. ID", WidthMax: 10, WidthMaxEnforcer: text.WrapText},
Value: func(j *models.JobHistory) string { return idgen.ShortID(j.ExecutionID) },
Value: func(j *models.JobHistory) string { return idgen.ShortUUID(j.ExecutionID) },
},
{
ColumnConfig: table.ColumnConfig{Name: "Node ID", WidthMax: 10, WidthMaxEnforcer: text.WrapText},
Value: func(j *models.JobHistory) string { return idgen.ShortID(j.NodeID) },
Value: func(j *models.JobHistory) string { return idgen.ShortNodeID(j.NodeID) },
},
{
ColumnConfig: table.ColumnConfig{Name: "Rev.", WidthMax: 4, WidthMaxEnforcer: text.WrapText},
Expand Down
2 changes: 1 addition & 1 deletion cmd/cli/job/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ var listColumns = []output.TableColumn[*models.Job]{
ColumnConfig: table.ColumnConfig{
Name: "id",
WidthMax: idgen.ShortIDLengthWithPrefix,
WidthMaxEnforcer: func(col string, maxLen int) string { return idgen.ShortID(col) }},
WidthMaxEnforcer: func(col string, maxLen int) string { return idgen.ShortUUID(col) }},
Value: func(jwi *models.Job) string { return jwi.ID },
},
{
Expand Down
2 changes: 1 addition & 1 deletion cmd/cli/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ var listColumns = []output.TableColumn[*model.JobWithInfo]{
ColumnConfig: table.ColumnConfig{
Name: "id",
WidthMax: idgen.ShortIDLengthWithPrefix,
WidthMaxEnforcer: func(col string, maxLen int) string { return idgen.ShortID(col) }},
WidthMaxEnforcer: func(col string, maxLen int) string { return idgen.ShortUUID(col) }},
Value: func(jwi *model.JobWithInfo) string { return jwi.Job.ID() },
},
{
Expand Down
4 changes: 2 additions & 2 deletions cmd/cli/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (suite *ListSuite) TestList_IdFilter() {
var err error
j := testutils.MakeNoopJob(suite.T())
j, err = suite.Client.Submit(ctx, j)
jobIds = append(jobIds, idgen.ShortID(j.Metadata.ID))
jobIds = append(jobIds, idgen.ShortUUID(j.Metadata.ID))
jobLongIds = append(jobLongIds, j.Metadata.ID)
require.NoError(suite.T(), err)
}
Expand Down Expand Up @@ -262,7 +262,7 @@ func (suite *ListSuite) TestList_SortFlags() {
j := testutils.MakeNoopJob(suite.T())
j, err = suite.Client.Submit(ctx, j)
require.NoError(suite.T(), err)
jobIDs = append(jobIDs, idgen.ShortID(j.Metadata.ID))
jobIDs = append(jobIDs, idgen.ShortUUID(j.Metadata.ID))

// all the middle jobs can have the same timestamp
// but we need the first and last to differ
Expand Down
2 changes: 1 addition & 1 deletion cmd/cli/node/columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
var alwaysColumns = []output.TableColumn[*models.NodeInfo]{
{
ColumnConfig: table.ColumnConfig{Name: "id"},
Value: func(node *models.NodeInfo) string { return idgen.ShortID(node.ID()) },
Value: func(node *models.NodeInfo) string { return idgen.ShortNodeID(node.ID()) },
},
{
ColumnConfig: table.ColumnConfig{Name: "type"},
Expand Down
1 change: 1 addition & 0 deletions cmd/cli/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ func NewCmd() *cobra.Command {
"requester-store": configflags.RequesterJobStorageFlags,
"web-ui": configflags.WebUIFlags,
"node-info-store": configflags.NodeInfoStoreFlags,
"node-name": configflags.NodeNameFlags,
"translations": configflags.JobTranslationFlags,
"docker-cache-manifest": configflags.DockerManifestCacheFlags,
}
Expand Down
25 changes: 18 additions & 7 deletions cmd/cli/serve/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import (
"github.com/bacalhau-project/bacalhau/pkg/compute/store/boltdb"
"github.com/bacalhau-project/bacalhau/pkg/jobstore"
boltjobstore "github.com/bacalhau-project/bacalhau/pkg/jobstore/boltdb"
"github.com/libp2p/go-libp2p/core/peer"
"github.com/bacalhau-project/bacalhau/pkg/util/idgen"
"github.com/rs/zerolog/log"
"github.com/samber/lo"
"github.com/spf13/viper"
"go.uber.org/multierr"

Expand Down Expand Up @@ -263,18 +264,28 @@ func getNodeID(ctx context.Context) (string, error) {
if nodeName != "" {
return nodeName, nil
}

// If no nodeName is defined, then use libp2p peer ID
privKey, err := config.GetLibp2pPrivKey()
nodeNameProviderType, err := config.Get[string](types.NodeNameProvider)
if err != nil {
return "", fmt.Errorf("getNodeID: error getting libp2p private key: %w", err)
return "", err
}

nodeNameProviders := map[string]idgen.NodeNameProvider{
"hostname": idgen.HostnameProvider{},
"aws": idgen.NewAWSNodeNameProvider(),
"gcp": idgen.NewGCPNodeNameProvider(),
"uuid": idgen.UUIDNodeNameProvider{},
"puuid": idgen.PUUIDNodeNameProvider{},
}
nodeNameProvider, ok := nodeNameProviders[nodeNameProviderType]
if !ok {
return "", fmt.Errorf(
"unknown node name provider: %s. Supported providers are: %s", nodeNameProviderType, lo.Keys(nodeNameProviders))
}

peerID, err := peer.IDFromPrivateKey(privKey)
nodeName, err = nodeNameProvider.GenerateNodeName(ctx)
if err != nil {
return "", err
}
nodeName = peerID.String()

// set the new name in the config, so it can be used and persisted later.
config.SetValue(types.NodeName, nodeName)
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,5 @@ func ensureDefaultDownloadLocation(jobID string) (string, error) {
}

func GetDefaultJobFolder(jobID string) string {
return fmt.Sprintf("job-%s", idgen.ShortID(jobID))
return fmt.Sprintf("job-%s", idgen.ShortUUID(jobID))
}
18 changes: 18 additions & 0 deletions cmd/util/flags/configflags/node_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package configflags

import "github.com/bacalhau-project/bacalhau/pkg/config/types"

var NodeNameFlags = []Definition{
{
FlagName: "name",
ConfigPath: types.NodeName,
DefaultValue: "",
Description: `The name of the node. If not set, the node name will be generated automatically based on the chosen name provider.`,
},
{
FlagName: "name-provider",
ConfigPath: types.NodeNameProvider,
DefaultValue: Default.Node.NameProvider,
Description: `The name provider to use to generate the node name, if no name is set.`,
},
}
2 changes: 1 addition & 1 deletion cmd/util/printer/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func summariseExecutions(executions []*models.Execution) map[string][]string {
}

if message != "" {
results[message] = append(results[message], idgen.ShortID(execution.NodeID))
results[message] = append(results[message], idgen.ShortNodeID(execution.NodeID))
}
}
return results
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/printer/print_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func summariseExecutionsLegacy(state model.JobState) map[string][]string {
}

if message != "" {
results[message] = append(results[message], idgen.ShortID(execution.NodeID))
results[message] = append(results[message], idgen.ShortNodeID(execution.NodeID))
}
}
return results
Expand Down
1 change: 1 addition & 0 deletions go.work.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2690,6 +2690,7 @@ google.golang.org/genproto v0.0.0-20230803162519-f966b187b2e5/go.mod h1:oH/ZOT02
google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d/go.mod h1:yZTlhN0tQnXo3h00fuXNCxJdLdIdnVFVBaRJ5LWBbw4=
google.golang.org/genproto v0.0.0-20230920204549-e6e6cdab5c13/go.mod h1:CCviP9RmpZ1mxVr8MUjCnSiY09IbAXZxhLE6EhHIdPU=
google.golang.org/genproto v0.0.0-20231002182017-d307bd883b97/go.mod h1:t1VqOqqvce95G3hIDCT5FeO3YUc6Q4Oe24L/+rNMxRk=
google.golang.org/genproto v0.0.0-20231212172506-995d672761c0/go.mod h1:l/k7rMz0vFTBPy+tFSGvXEd3z+BcoG1k7EHbqm+YBsY=
google.golang.org/genproto/googleapis/api v0.0.0-20230525234035-dd9d682886f9/go.mod h1:vHYtlOoi6TsQ3Uk2yxR7NI5z8uoV+3pZtR4jmHIkRig=
google.golang.org/genproto/googleapis/api v0.0.0-20230526203410-71b5a4ffd15e/go.mod h1:vHYtlOoi6TsQ3Uk2yxR7NI5z8uoV+3pZtR4jmHIkRig=
google.golang.org/genproto/googleapis/api v0.0.0-20230530153820-e85fd2cbaebc/go.mod h1:vHYtlOoi6TsQ3Uk2yxR7NI5z8uoV+3pZtR4jmHIkRig=
Expand Down
1 change: 1 addition & 0 deletions pkg/config/configenv/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var Development = types.BacalhauConfig{
},
},
Node: types.NodeConfig{
NameProvider: "puuid",
ClientAPI: types.APIConfig{
Host: "bootstrap.development.bacalhau.org",
Port: 1234,
Expand Down
1 change: 1 addition & 0 deletions pkg/config/configenv/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var Local = types.BacalhauConfig{
},
},
Node: types.NodeConfig{
NameProvider: "puuid",
ClientAPI: types.APIConfig{
Host: "0.0.0.0",
Port: 1234,
Expand Down
1 change: 1 addition & 0 deletions pkg/config/configenv/production.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var Production = types.BacalhauConfig{
},
},
Node: types.NodeConfig{
NameProvider: "puuid",
ClientAPI: types.APIConfig{
Host: "bootstrap.production.bacalhau.org",
Port: 1234,
Expand Down
1 change: 1 addition & 0 deletions pkg/config/configenv/staging.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var Staging = types.BacalhauConfig{
},
},
Node: types.NodeConfig{
NameProvider: "puuid",
ClientAPI: types.APIConfig{
Host: "bootstrap.staging.bacalhau.org",
Port: 1234,
Expand Down
1 change: 1 addition & 0 deletions pkg/config/configenv/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var Testing = types.BacalhauConfig{
},
},
Node: types.NodeConfig{
NameProvider: "puuid",
ClientAPI: types.APIConfig{
Host: "test",
Port: 9999,
Expand Down
1 change: 1 addition & 0 deletions pkg/config/types/generated_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package types

const Node = "Node"
const NodeName = "Node.Name"
const NodeNameProvider = "Node.NameProvider"
const NodeClientAPI = "Node.ClientAPI"
const NodeClientAPIHost = "Node.ClientAPI.Host"
const NodeClientAPIPort = "Node.ClientAPI.Port"
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/types/generated_viper_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func SetDefaults(cfg BacalhauConfig, opts ...SetOption) {

p.Viper.SetDefault(Node, cfg.Node)
p.Viper.SetDefault(NodeName, cfg.Node.Name)
p.Viper.SetDefault(NodeNameProvider, cfg.Node.NameProvider)
p.Viper.SetDefault(NodeClientAPI, cfg.Node.ClientAPI)
p.Viper.SetDefault(NodeClientAPIHost, cfg.Node.ClientAPI.Host)
p.Viper.SetDefault(NodeClientAPIPort, cfg.Node.ClientAPI.Port)
Expand Down Expand Up @@ -210,6 +211,7 @@ func Set(cfg BacalhauConfig, opts ...SetOption) {

p.Viper.Set(Node, cfg.Node)
p.Viper.Set(NodeName, cfg.Node.Name)
p.Viper.Set(NodeNameProvider, cfg.Node.NameProvider)
p.Viper.Set(NodeClientAPI, cfg.Node.ClientAPI)
p.Viper.Set(NodeClientAPIHost, cfg.Node.ClientAPI.Host)
p.Viper.Set(NodeClientAPIPort, cfg.Node.ClientAPI.Port)
Expand Down
11 changes: 6 additions & 5 deletions pkg/config/types/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import (
)

type NodeConfig struct {
Name string `yaml:"Name"`
ClientAPI APIConfig `yaml:"ClientAPI"`
ServerAPI APIConfig `yaml:"ServerAPI"`
Libp2p Libp2pConfig `yaml:"Libp2P"`
IPFS IpfsConfig `yaml:"IPFS"`
Name string `yaml:"Name"`
NameProvider string `yaml:"NameProvider"`
ClientAPI APIConfig `yaml:"ClientAPI"`
ServerAPI APIConfig `yaml:"ServerAPI"`
Libp2p Libp2pConfig `yaml:"Libp2P"`
IPFS IpfsConfig `yaml:"IPFS"`

Compute ComputeConfig `yaml:"Compute"`
Requester RequesterConfig `yaml:"Requester"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/jobstore/boltdb/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (b *BoltJobStore) getJob(tx *bolt.Tx, jobID string) (models.Job, error) {
// reifyJobID ensures the provided job ID is a full-length ID. This is either through
// returning the ID, or resolving the short ID to a single job id.
func (b *BoltJobStore) reifyJobID(tx *bolt.Tx, jobID string) (string, error) {
if idgen.ShortID(jobID) == jobID {
if idgen.ShortUUID(jobID) == jobID {
bktJobs, err := NewBucketPath(BucketJobs).Get(tx, false)
if err != nil {
return "", err
Expand Down
2 changes: 1 addition & 1 deletion pkg/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func defaultStationLogging() io.Writer {
}

func loggerWithNodeID(nodeID string) zerolog.Logger {
return log.With().Str(nodeIDFieldName, idgen.ShortID(nodeID)).Logger()
return log.With().Str(nodeIDFieldName, idgen.ShortNodeID(nodeID)).Logger()
}

// ContextWithNodeIDLogger will return a context with nodeID is added to the logging context.
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/execution_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ type ExecutionID struct {

// String returns a string representation of the execution id
func (e ExecutionID) String() string {
return fmt.Sprintf("%s:%s:%s", e.JobID, idgen.ShortID(e.NodeID), e.ExecutionID)
return fmt.Sprintf("%s:%s:%s", e.JobID, idgen.ShortNodeID(e.NodeID), e.ExecutionID)
}

type ExecutionState struct {
Expand Down
10 changes: 9 additions & 1 deletion pkg/nats/transport/nats.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import (

const NodeInfoSubjectPrefix = "node.info."

// reservedChars are the characters that are not allowed in node IDs as nodes
// subscribe to subjects with their node IDs, and these are wildcards
// in NATS subjects that could cause a node to subscribe to unintended subjects.
const reservedChars = ".*>"

type NATSTransportConfig struct {
NodeID string
Port int
Expand Down Expand Up @@ -51,14 +56,17 @@ func (c *NATSTransportConfig) Validate() error {
mErr = multierror.Append(mErr, errors.New("node ID contains a space"))
} else if validate.ContainsNull(c.NodeID) {
mErr = multierror.Append(mErr, errors.New("node ID contains a null character"))
} else if strings.ContainsAny(c.NodeID, reservedChars) {
mErr = multierror.Append(mErr, fmt.Errorf("node ID '%s' contains one or more reserved characters: %s", c.NodeID, reservedChars))
}

if c.IsRequesterNode {
mErr = multierror.Append(mErr, validate.IsGreaterThanZero(c.Port, "port %d must be greater than zero", c.Port))

// if cluster config is set, validate it
if c.ClusterName != "" || c.ClusterPort != 0 || c.ClusterAdvertisedAddress != "" || len(c.ClusterPeers) > 0 {
mErr = multierror.Append(mErr, validate.IsGreaterThanZero(c.ClusterPort, "cluster port %d must be greater than zero", c.Port))
mErr = multierror.Append(mErr,
validate.IsGreaterThanZero(c.ClusterPort, "cluster port %d must be greater than zero", c.ClusterPort))
}
} else {
if validate.IsEmpty(c.Orchestrators) {
Expand Down
Loading

0 comments on commit 3c59c46

Please sign in to comment.