Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify configs package #1032

Merged
merged 10 commits into from
Jun 7, 2019
Merged

Simplify configs package #1032

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b12344d
Remove unnecessary configs. Move Core config to root of config struct.
NicolasMahe Jun 6, 2019
4b68ecd
Apply config change across codebase
NicolasMahe Jun 6, 2019
0a8f3f6
Rename docker label mesg.core to mesg.engine
NicolasMahe Jun 6, 2019
d75cafa
Remove useless MESG_ENDPONT_TCP env from mesg services
NicolasMahe Jun 6, 2019
d4619af
Remove non existant config from info api
NicolasMahe Jun 6, 2019
63ab932
fix lint
NicolasMahe Jun 6, 2019
63c07e6
fix test
NicolasMahe Jun 7, 2019
0914cc8
Set back engine path to /mesg
NicolasMahe Jun 7, 2019
7010a4a
Set mesg folder inside the docker to the same place as local (~/.mesg)
NicolasMahe Jun 7, 2019
dc22b05
Merge branch 'dev' into feature/configs
NicolasMahe Jun 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 11 additions & 50 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import (
"fmt"
"os"
"path/filepath"
"strconv"
"strings"
"sync"

"github.com/kelseyhightower/envconfig"
"github.com/mesg-foundation/core/version"
"github.com/mesg-foundation/core/x/xstrings"
homedir "github.com/mitchellh/go-homedir"
"github.com/sirupsen/logrus"
)

Expand All @@ -29,11 +25,10 @@ var (

// Config contains all the configuration needed.
type Config struct {
Server struct {
Address string
}
Name string
Path string

Client struct {
Server struct {
Address string
}

Expand All @@ -43,47 +38,25 @@ type Config struct {
Level string
}

Core struct {
Image string
Name string
Path string

Database struct {
ServiceRelativePath string
ExecutionRelativePath string
}
Database struct {
ServiceRelativePath string
ExecutionRelativePath string
}

Service ServiceConfigGroup

Docker struct {
Socket string
Core struct {
Path string
}
}
}

// New creates a new config with default values.
func New() (*Config, error) {
home, err := homedir.Dir()
if err != nil {
return nil, err
}

var c Config
c.Server.Address = ":50052"
c.Client.Address = "localhost:50052"
c.Log.Format = "text"
c.Log.Level = "info"
c.Log.ForceColors = false
c.Core.Image = "mesg/engine:" + strings.Split(version.Version, " ")[0]
c.Core.Name = "engine"
c.Core.Path = filepath.Join(home, ".mesg")
c.Core.Database.ServiceRelativePath = filepath.Join("database", "services", serviceDBVersion)
c.Core.Database.ExecutionRelativePath = filepath.Join("database", "executions", executionDBVersion)
c.Docker.Core.Path = "/mesg"
c.Docker.Socket = "/var/run/docker.sock"
c.Name = "engine"
c.Path = filepath.Join("mesg")
NicolasMahe marked this conversation as resolved.
Show resolved Hide resolved
c.Database.ServiceRelativePath = filepath.Join("database", "services", serviceDBVersion)
c.Database.ExecutionRelativePath = filepath.Join("database", "executions", executionDBVersion)
c.Service = c.getServiceConfigGroup()
return &c, nil
}
Expand Down Expand Up @@ -120,7 +93,7 @@ func (c *Config) Load() error {

// Prepare setups local directories or any other required thing based on config
func (c *Config) Prepare() error {
return os.MkdirAll(c.Core.Path, os.FileMode(0755))
return os.MkdirAll(c.Path, os.FileMode(0755))
}

// Validate checks values and return an error if any validation failed.
Expand All @@ -133,15 +106,3 @@ func (c *Config) Validate() error {
}
return nil
}

// DaemonEnv returns the needed environmental variable for the Daemon.
func (c *Config) DaemonEnv() map[string]string {
return map[string]string{
"MESG_SERVER_ADDRESS": c.Server.Address,
"MESG_LOG_FORMAT": c.Log.Format,
"MESG_LOG_LEVEL": c.Log.Level,
"MESG_LOG_FORCECOLORS": strconv.FormatBool(c.Log.ForceColors),
"MESG_CORE_NAME": c.Core.Name,
"MESG_CORE_PATH": c.Docker.Core.Path,
}
}
33 changes: 4 additions & 29 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,22 @@ package config
import (
"os"
"path/filepath"
"strconv"
"strings"
"testing"

homedir "github.com/mitchellh/go-homedir"
"github.com/stretchr/testify/require"
)

func TestDefaultValue(t *testing.T) {
home, _ := homedir.Dir()
c, err := New()
require.NoError(t, err)
require.Equal(t, ":50052", c.Server.Address)
require.Equal(t, "localhost:50052", c.Client.Address)
require.Equal(t, "text", c.Log.Format)
require.Equal(t, "info", c.Log.Level)
require.Equal(t, false, c.Log.ForceColors)
require.Equal(t, filepath.Join(home, ".mesg"), c.Core.Path)
require.Equal(t, filepath.Join("database", "services", serviceDBVersion), c.Core.Database.ServiceRelativePath)
require.Equal(t, filepath.Join("database", "executions", executionDBVersion), c.Core.Database.ExecutionRelativePath)
require.Equal(t, "engine", c.Core.Name)
require.Equal(t, "/mesg", c.Docker.Core.Path)
require.Equal(t, "/var/run/docker.sock", c.Docker.Socket)
require.True(t, strings.HasPrefix(c.Core.Image, "mesg/engine:"))
require.Equal(t, filepath.Join("mesg"), c.Path)
require.Equal(t, filepath.Join("database", "services", serviceDBVersion), c.Database.ServiceRelativePath)
require.Equal(t, filepath.Join("database", "executions", executionDBVersion), c.Database.ExecutionRelativePath)
require.Equal(t, "engine", c.Name)
}

func TestGlobal(t *testing.T) {
Expand All @@ -38,11 +30,9 @@ func TestGlobal(t *testing.T) {
func TestLoad(t *testing.T) {
snapsnot := map[string]string{
"MESG_SERVER_ADDRESS": "",
"MESG_CLIENT_ADDRESS": "",
"MESG_LOG_FORMAT": "",
"MESG_LOG_LEVEL": "",
"MESG_LOG_FORCECOLORS": "",
"MESG_CORE_IMAGE": "",
}
for key := range snapsnot {
snapsnot[key] = os.Getenv(key)
Expand All @@ -54,19 +44,15 @@ func TestLoad(t *testing.T) {
}()

os.Setenv("MESG_SERVER_ADDRESS", "test_server_address")
os.Setenv("MESG_CLIENT_ADDRESS", "test_client_address")
os.Setenv("MESG_LOG_FORMAT", "test_log_format")
os.Setenv("MESG_LOG_LEVEL", "test_log_level")
os.Setenv("MESG_LOG_FORCECOLORS", "true")
os.Setenv("MESG_CORE_IMAGE", "test_core_image")
c, _ := New()
c.Load()
require.Equal(t, "test_server_address", c.Server.Address)
require.Equal(t, "test_client_address", c.Client.Address)
require.Equal(t, "test_log_format", c.Log.Format)
require.Equal(t, "test_log_level", c.Log.Level)
require.Equal(t, true, c.Log.ForceColors)
require.Equal(t, "test_core_image", c.Core.Image)
}

func TestValidate(t *testing.T) {
Expand All @@ -81,14 +67,3 @@ func TestValidate(t *testing.T) {
c.Log.Level = "wrongValue"
require.Error(t, c.Validate())
}

func TestDaemonEnv(t *testing.T) {
c, _ := New()
env := c.DaemonEnv()
require.Equal(t, c.Server.Address, env["MESG_SERVER_ADDRESS"])
require.Equal(t, c.Log.Level, env["MESG_LOG_LEVEL"])
require.Equal(t, c.Log.Format, env["MESG_LOG_FORMAT"])
require.Equal(t, strconv.FormatBool(c.Log.ForceColors), env["MESG_LOG_FORCECOLORS"])
require.Equal(t, c.Core.Name, env["MESG_CORE_NAME"])
require.Equal(t, c.Docker.Core.Path, env["MESG_CORE_PATH"])
}
2 changes: 1 addition & 1 deletion container/container_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestNew(t *testing.T) {
}

ln := <-dt.LastNetworkCreate()
require.Equal(t, cfg.Core.Name, ln.Name)
require.Equal(t, cfg.Name, ln.Name)
require.Equal(t, types.NetworkCreate{
CheckDuplicate: true,
Driver: "overlay",
Expand Down
2 changes: 1 addition & 1 deletion container/namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const namespaceSeparator string = "-"

// Namespace creates a namespace from a list of string.
func (c *DockerContainer) Namespace(ss []string) string {
ssWithPrefix := append([]string{c.config.Core.Name}, ss...)
ssWithPrefix := append([]string{c.config.Name}, ss...)
namespace := strings.Join(ssWithPrefix, namespaceSeparator)
namespace = strings.Replace(namespace, " ", namespaceSeparator, -1)
return namespace
Expand Down
4 changes: 2 additions & 2 deletions container/namespace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ func TestNamespace(t *testing.T) {
cfg, _ := config.Global()
c, _ := New()
namespace := c.Namespace([]string{"test"})
require.Equal(t, namespace, strings.Join([]string{cfg.Core.Name, "test"}, namespaceSeparator))
require.Equal(t, namespace, strings.Join([]string{cfg.Name, "test"}, namespaceSeparator))
}

func TestNamespaceReplaceSpace(t *testing.T) {
cfg, _ := config.Global()
c, _ := New()
namespace := c.Namespace([]string{"test foo"})
require.Equal(t, namespace, strings.Join([]string{cfg.Core.Name, "test-foo"}, namespaceSeparator))
require.Equal(t, namespace, strings.Join([]string{cfg.Name, "test-foo"}, namespaceSeparator))
}
4 changes: 2 additions & 2 deletions core/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ func initDependencies() (*dependencies, error) {
}

// init services db.
serviceDB, err := database.NewServiceDB(filepath.Join(config.Core.Path, config.Core.Database.ServiceRelativePath))
serviceDB, err := database.NewServiceDB(filepath.Join(config.Path, config.Database.ServiceRelativePath))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.Database.ServiceRelativePath should already contain config.Path

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the end it makes sense to keep config.Path out of config.Database.XXXXRelativePath so by only modifiying config.Path, it will change all other path that use this folder.
The user doesn't need to know all sub-folder.

Copy link
Contributor

@krhubert krhubert Jun 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I want to avoid is the fact that other package needs to modify configs path. In that case, only config package manages paths etc... What you wrote is true also when config package joins paths.

To be clear I don't want a user to specify the full path for the database. I want to move a join call into config package.

I know we have this in the previous version of engine, but it could be refactored and clear a bit if :)

if err != nil {
return nil, err
}

// init execution db.
executionDB, err := database.NewExecutionDB(filepath.Join(config.Core.Path, config.Core.Database.ExecutionRelativePath))
executionDB, err := database.NewExecutionDB(filepath.Join(config.Path, config.Database.ExecutionRelativePath))
if err != nil {
return nil, err
}
Expand Down
Loading