Skip to content

Commit

Permalink
use dataDir/config.yaml as the default config path (#4466)
Browse files Browse the repository at this point in the history
A recent change in behaviour has caused some confusion where the default
config path changed from `repo/config.yaml` to
`~/.config/bacalhau/config.yaml`. This PR reverts this change and
continue using a `config.yaml` under the repo/dataDir if present and if
no explicit config file was provided by the user through `--config`
flag.

The assumption that many users have is deleting the data directory (e.g.
`~/.bacalhau`), or pointing to a fresh directory using `$BACALHAU_DIR`
env variable or `--data-dir` flag, is reseting the state and
configuration of the node or client. However, this wasn't the case as
the default config path was outside the data directory.

### Before this PR
This is an example where issues could happen:
```
# user changes the api port 
# here the user didn't provide a --config file, so configurations 
# were written to a user global `~/.config/bacalhau/config.yaml` 
# and not local to the data dir
bacalhau config set api.port 2233

# user starts a node and expects 2233
# this works as the user didn't specify --config and the default path was picked
bacalhau serve --compute --orchestrator

# user decides to start another node, or just stop the previous one and start fresh again
export BACALHAU_DIR=$(mktemp -d -t bacalhau_dir)
bacalhau serve --compute --orchestrator

# here the user expects the node to start with default api.port of 1234
# but the default configuration was read again
```

### After this PR
```
# user changes the api port 
# the configs are now written to the data dir `$BACALHAU_DIR/config.yaml`
bacalhau config set api.port 2233

# user starts a node and expects 2233
# this works as the user didn't specify --config and the data dir path was picked
bacalhau serve --compute --orchestrator

# user decides to start another node, or just stop the previous one and start fresh again
export BACALHAU_DIR=$(mktemp -d -t bacalhau_dir)
bacalhau serve --compute --orchestrator

# here the node starts with fresh config as the data dir has empty configuratoin
```

### Different Combinations
with the upcoming v1.5.0 release, users are able to define explicit
configuration files using `--config` flag, as well as the existing
`--data-dir` flag to define where bacalhau stores its state. These are
also configurable using env variables.

The default `data-dir` if none is specified is `~/.bacalhau`, and the
default config path is `$(dataDir)/config.yaml`. Lets explore what that
means in action

#### Setting config
```
# will use ~/.bacalhau/config.yaml
bacalhau config set api.port 2233

# will use hello.yaml
bacalhau config set --config hello.yaml api.port 2233

# will use customDir/config.yaml
bacalhau config set --data-dir customDir api.port 2233

# will use hello.yaml
bacalhau config set --config hello.yaml --data-dir customDir api.port 2233
```

#### Applying config
`bacalhau serve` has the same flags as `bacalhau config set`, and the
same logic and precedence applies
```
# will use ~/.bacalhau/config.yaml
bacalhau serve --orchestrator --compute

# will use hello.yaml
bacalhau serve --orchestrator --compute --config hello.yaml

# will use customDir/config.yaml
bacalhau serve --orchestrator --compute --data-dir customDir 

# will use hello.yaml instead of customDir/config.yaml
bacalhau serve --orchestrator --compute --config hello.yaml --data-dir customDir 
```
  • Loading branch information
wdbaruni authored Sep 19, 2024
1 parent 07e89ff commit ad6d795
Show file tree
Hide file tree
Showing 11 changed files with 389 additions and 227 deletions.
45 changes: 20 additions & 25 deletions cmd/cli/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (
"github.com/spf13/viper"

"github.com/bacalhau-project/bacalhau/cmd/util"
"github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags"
"github.com/bacalhau-project/bacalhau/cmd/util/hook"
"github.com/bacalhau-project/bacalhau/pkg/config"
"github.com/bacalhau-project/bacalhau/pkg/config/types"
util2 "github.com/bacalhau-project/bacalhau/pkg/storage/util"
)

func newSetCmd() *cobra.Command {
Expand All @@ -26,40 +26,35 @@ func newSetCmd() *cobra.Command {
PostRunE: hook.ClientPostRunHooks,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
// initialize a new or open an existing repo. We need to ensure a repo
// exists before we can create or modify a config file in it.
_, err := util.SetupRepoConfig(cmd)
// load configs to get the config file path
bacalhauConfig, rawConfig, err := util.SetupConfigs(cmd)
if err != nil {
return fmt.Errorf("failed to setup repo: %w", err)
return fmt.Errorf("failed to setup config: %w", err)
}
return setConfig(cmd.PersistentFlags().Lookup("config").Value.String(), args[0], args[1:]...)
},
// provide auto completion for arguments to the `set` command
ValidArgsFunction: setAutoComplete,
}

bacalhauCfgDir := "bacalhau"
bacalhauCfgFile := config.DefaultFileName
configPath := rawConfig.ConfigFileUsed()
if configPath == "" {
// we fall back to the default config file path $BACALHAU_DIR/config.yaml
// this requires initializing a new or opening an existing data-dir
_, err = util.SetupRepo(bacalhauConfig)
if err != nil {
return fmt.Errorf("failed to setup data dir: %w", err)
}
configPath = filepath.Join(bacalhauConfig.DataDir, config.DefaultFileName)
}

usrCfgDir, err := os.UserConfigDir()
if err != nil {
log.Warn().Err(err).Msg("Failed to find user-specific configuration directory. Using current directory to write config.")
} else {
bacalhauCfgDir = filepath.Join(usrCfgDir, bacalhauCfgDir)
if err := os.MkdirAll(bacalhauCfgDir, util2.OS_USER_RWX); err != nil {
// This means we failed to create a directory either in the current directory, or the user config dir
// indicating a some-what serious misconfiguration of the system. We panic here to provide as much
// detail as possible.
log.Panic().Err(err).Msgf("Failed to create bacalhau configuration directory: %s", bacalhauCfgDir)
}
bacalhauCfgFile = filepath.Join(bacalhauCfgDir, bacalhauCfgFile)
return setConfig(configPath, args[0], args[1:]...)
},
// Provide auto completion for arguments to the `set` command
ValidArgsFunction: setAutoComplete,
}

setCmd.PersistentFlags().String("config", bacalhauCfgFile, "Path to the config file")
setCmd.PersistentFlags().VarP(cliflags.NewWriteConfigFlag(), "config", "c", "Path to the config file (default is $BACALHAU_DIR/config.yaml)")
return setCmd
}

func setConfig(cfgFilePath, key string, value ...string) error {
log.Info().Msgf("Writing config to %s", cfgFilePath)
v := viper.New()
v.SetConfigFile(cfgFilePath)
if err := v.ReadInConfig(); err != nil {
Expand Down
119 changes: 83 additions & 36 deletions cmd/cli/config/set_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//go:build unit || !integration

package config
package config_test

import (
"io"
Expand All @@ -11,61 +11,108 @@ import (
"github.com/stretchr/testify/require"
"sigs.k8s.io/yaml"

cmd2 "github.com/bacalhau-project/bacalhau/cmd/cli"
"github.com/bacalhau-project/bacalhau/pkg/config/types"
)

// TestAdditiveSet calls sets on the config system sequentially with different values
// and asserts that after each set only the newly set value was added to the config.
// Essentially we are testing two different things:
func TestAdditiveSet(t *testing.T) {
cfgFilePath := filepath.Join(t.TempDir(), "config.yaml")
func TestSetWithExplicitConfigPath(t *testing.T) {
tempDir := t.TempDir()
explicitConfigPath := filepath.Join(tempDir, "explicit_config.yaml")

err := setConfig(cfgFilePath, "api.host", "127.0.0.1")
require.NoError(t, err)
err = setConfig(cfgFilePath, "api.port", "1234")
// Create an empty config file
_, err := os.Create(explicitConfigPath)
require.NoError(t, err)

expected := types.Bacalhau{API: types.API{
Host: "127.0.0.1",
Port: 1234,
}}
actual := unmarshalConfigFile(t, cfgFilePath)
cmd := cmd2.NewRootCmd()
cmd.SetArgs([]string{"config", "set", "--config", explicitConfigPath, "api.host", "2.2.2.2"})

err = cmd.Execute()
require.NoError(t, err)

actual := unmarshalConfigFile(t, explicitConfigPath)
expected := types.Bacalhau{API: types.API{Host: "2.2.2.2"}}
require.Equal(t, expected, actual)
}

func TestSetWithDefaultConfigPath(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("BACALHAU_DIR", tempDir)

cmd := cmd2.NewRootCmd()
cmd.SetArgs([]string{"config", "set", "api.port", "5678"})

err = setConfig(cfgFilePath, "compute.enabled", "true")
err := cmd.Execute()
require.NoError(t, err)
err = setConfig(cfgFilePath, "compute.orchestrators", "http://127.0.0.1:1234", "http://1.1.1.1:1234")

defaultConfigPath := filepath.Join(tempDir, "config.yaml")
actual := unmarshalConfigFile(t, defaultConfigPath)
expected := types.Bacalhau{API: types.API{Port: 5678}}
require.Equal(t, expected, actual)
}

func TestSetMultipleValues(t *testing.T) {
tempDir := t.TempDir()
os.Setenv("BACALHAU_DIR", tempDir)
defer os.Unsetenv("BACALHAU_DIR")

cmd := cmd2.NewRootCmd()
cmd.SetArgs([]string{"config", "set", "compute.orchestrators", "http://127.0.0.1:1234", "http://1.1.1.1:1234"})

err := cmd.Execute()
require.NoError(t, err)

expected = types.Bacalhau{
API: types.API{
Host: "127.0.0.1",
Port: 1234,
},
Compute: types.Compute{
Enabled: true,
Orchestrators: []string{
"http://127.0.0.1:1234",
"http://1.1.1.1:1234",
},
defaultConfigPath := filepath.Join(tempDir, "config.yaml")
actual := unmarshalConfigFile(t, defaultConfigPath)
expected := types.Bacalhau{Compute: types.Compute{
Orchestrators: []string{
"http://127.0.0.1:1234",
"http://1.1.1.1:1234",
},
}
actual = unmarshalConfigFile(t, cfgFilePath)

}}
require.Equal(t, expected, actual)

}

func TestSetFailure(t *testing.T) {
cfgFilePath := filepath.Join(t.TempDir(), "config.yaml")
// fails as the key isn't a valid config key
err := setConfig(cfgFilePath, "not.a.config.key", "porkchop sandwiches")
func TestSetInvalidKey(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("BACALHAU_DIR", tempDir)

cmd := cmd2.NewRootCmd()
cmd.SetArgs([]string{"config", "set", "invalid.key", "value"})

err := cmd.Execute()
require.Error(t, err)
require.Contains(t, err.Error(), "not a valid config key")
}

func unmarshalConfigFile(t testing.TB, path string) types.Bacalhau {
func TestSetAdditiveChanges(t *testing.T) {
tempDir := t.TempDir()
configPath := filepath.Join(tempDir, "config.yaml")

// Create an empty config file
_, err := os.Create(configPath)
require.NoError(t, err)

// First set
cmd1 := cmd2.NewRootCmd()
cmd1.SetArgs([]string{"config", "set", "--config", configPath, "api.host", "127.0.0.1"})
err = cmd1.Execute()
require.NoError(t, err)

// Second set
cmd2 := cmd2.NewRootCmd()
cmd2.SetArgs([]string{"config", "set", "--config", configPath, "api.port", "1234"})
err = cmd2.Execute()
require.NoError(t, err)

actual := unmarshalConfigFile(t, configPath)
expected := types.Bacalhau{API: types.API{
Host: "127.0.0.1",
Port: 1234,
}}
require.Equal(t, expected, actual)
}

func unmarshalConfigFile(t testing.TB, path string) types.Bacalhau {
configFile, err := os.Open(path)
require.NoError(t, err)
t.Cleanup(func() {
Expand Down
20 changes: 20 additions & 0 deletions cmd/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/bacalhau-project/bacalhau/cmd/util"
"github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags"
"github.com/bacalhau-project/bacalhau/cmd/util/flags/configflags"
"github.com/bacalhau-project/bacalhau/pkg/config/types"
"github.com/bacalhau-project/bacalhau/pkg/logger"
"github.com/bacalhau-project/bacalhau/pkg/system"
)

Expand Down Expand Up @@ -69,6 +71,22 @@ func NewRootCmd() *cobra.Command {
return err
}

// Configure logging
// While we allow users to configure logging via the config file, they are applied
// and will override this configuration at a later stage when the config is loaded.
// This is needed to ensure any logs before the config is loaded are captured.
logMode := viper.GetString(types.LoggingModeKey)
if logMode == "" {
logMode = string(logger.LogModeDefault)
}
logLevel := viper.GetString(types.LoggingLevelKey)
if logLevel == "" {
logLevel = "Info"
}
if err := logger.ConfigureLogging(logMode, logLevel); err != nil {
return fmt.Errorf("failed to configure logging: %w", err)
}

return nil
}

Expand Down Expand Up @@ -110,6 +128,8 @@ func NewRootCmd() *cobra.Command {
func Execute(ctx context.Context) {
rootCmd := NewRootCmd()
rootCmd.SetContext(ctx)
rootCmd.SilenceErrors = true
rootCmd.SilenceUsage = true
if err := rootCmd.Execute(); err != nil {
util.Fatal(rootCmd, err, 1)
}
Expand Down
18 changes: 0 additions & 18 deletions cmd/cli/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"net"
"net/url"
"os"
"strings"

Expand Down Expand Up @@ -390,10 +389,6 @@ func buildEnvVariables(
var envvars strings.Builder
envvars.WriteString(fmt.Sprintf("export %s=%s\n", config.KeyAsEnvVar(types.APIHostKey), getAPIURL(cfg.API)))
envvars.WriteString(fmt.Sprintf("export %s=%d\n", config.KeyAsEnvVar(types.APIPortKey), cfg.API.Port))
if cfg.Orchestrator.Enabled {
envvars.WriteString(fmt.Sprintf("export %s=%s\n",
config.KeyAsEnvVar(types.ComputeOrchestratorsKey), getPublicNATSOrchestratorURL(cfg.Orchestrator)))
}
return envvars.String()
}

Expand All @@ -404,16 +399,3 @@ func getAPIURL(cfg types.API) string {
return cfg.Host
}
}

func getPublicNATSOrchestratorURL(cfg types.Orchestrator) *url.URL {
orchestrator := &url.URL{
Scheme: "nats",
Host: cfg.Advertise,
}

if cfg.Advertise == "" {
orchestrator.Host = fmt.Sprintf("127.0.0.1:%d", cfg.Port)
}

return orchestrator
}
Loading

0 comments on commit ad6d795

Please sign in to comment.