From 1ac5314d5bb39b5778950301e53e89e32e86667b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Mon, 23 Jan 2017 23:39:04 -0500 Subject: [PATCH 1/8] Add RenderConfig option to ContainerPilot. This is useful in AutoPilot applications like MySQL that need to rewrite the containerpilot.conf file and force a SIGHUP. --- config/render.go | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ core/app.go | 18 ++++++++++-- 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 config/render.go diff --git a/config/render.go b/config/render.go new file mode 100644 index 00000000..1b59bc0a --- /dev/null +++ b/config/render.go @@ -0,0 +1,75 @@ +package config + +import ( + "errors" + "fmt" + "io/ioutil" + "os" + "strings" +) + +// A Render Application +type RenderApp struct { + ConfigFlag string + RenderFlag string + RenderedConfig []byte +} + +// EmptyApp creates an empty application +func EmptyApp() *RenderApp { + app := &RenderApp{} + return app +} + +// NewRenderApp creates a new Config Rendering application +func NewRenderApp(configFlag string, renderFlag string) (*RenderApp, error) { + + a := EmptyApp() + + if configFlag == "" { + return nil, errors.New("-config flag is required") + } + if renderFlag != "-" && !strings.HasPrefix(renderFlag, "file://") { + return nil, errors.New("-render flag is invalid") + } + + a.ConfigFlag = configFlag + a.RenderFlag = renderFlag + + var data []byte + if strings.HasPrefix(configFlag, "file://") { + var err error + fName := strings.SplitAfter(configFlag, "file://")[1] + if data, err = ioutil.ReadFile(fName); err != nil { + return nil, fmt.Errorf("Could not read config file: %s", err) + } + } else { + data = []byte(configFlag) + } + + template, err := ApplyTemplate(data) + if err != nil { + return nil, fmt.Errorf( + "Could not apply template to config: %v", err) + } + + a.RenderedConfig = template + + return a, nil +} + +func (a *RenderApp) Run() { + if a.RenderFlag == "-" { + fmt.Printf("%s", a.RenderedConfig) + os.Exit(0) + } + if strings.HasPrefix(a.RenderFlag, "file://") { + var err error + fName := strings.SplitAfter(a.RenderFlag, "file://")[1] + if err = ioutil.WriteFile(fName, a.RenderedConfig, 0644); err != nil { + panic(fmt.Errorf("Could not write config file: %s", err)) + } + os.Exit(0) + } + os.Exit(1) +} diff --git a/core/app.go b/core/app.go index 9363680e..8cdb2847 100644 --- a/core/app.go +++ b/core/app.go @@ -29,6 +29,10 @@ var ( GitHash string ) +type Runnable interface { + Run() +} + // App encapsulates the state of ContainerPilot after the initial setup. // after it is run, it can be reloaded and paused with signals. type App struct { @@ -59,14 +63,17 @@ func EmptyApp() *App { } // LoadApp parses the commandline arguments and loads the config -func LoadApp() (*App, error) { +func LoadApp() (Runnable, error) { var configFlag string + var renderFlag string var versionFlag bool if !flag.Parsed() { flag.StringVar(&configFlag, "config", "", "JSON config or file:// path to JSON config file.") + flag.StringVar(&renderFlag, "render", "", + "- for stdout or file:// path where to save redenred JSON config file.") flag.BoolVar(&versionFlag, "version", false, "Show version identifier and quit.") flag.Parse() } @@ -79,7 +86,14 @@ func LoadApp() (*App, error) { } os.Setenv("CONTAINERPILOT_PID", fmt.Sprintf("%v", os.Getpid())) - app, err := NewApp(configFlag) + var app Runnable + var err error + if renderFlag == "" { + app, err = NewApp(configFlag) + } else { + app, err = config.NewRenderApp(configFlag, renderFlag) + } + if err != nil { return nil, err } From 5985ec60c22588f2bbf2e48d1aee40c47896cb47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Tue, 24 Jan 2017 00:25:15 -0500 Subject: [PATCH 2/8] LoadApp returns a Runnable interface. The unit tests now cast the runnable to core.App in order to pass. --- core/app_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/app_test.go b/core/app_test.go index 09aca13c..4bd406c0 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -58,10 +58,11 @@ func TestValidConfigParse(t *testing.T) { os.Setenv("TEST", "HELLO") os.Args = []string{"this", "-config", testJSON, "/testdata/test.sh", "valid1", "--debug"} - app, err := LoadApp() + runnable, err := LoadApp() if err != nil { t.Fatalf("Unexpected error in LoadApp: %v", err) } + var app = runnable.(*App) if len(app.Backends) != 2 || len(app.Services) != 2 { t.Fatalf("Expected 2 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) From 6444caa86195d302cc294737bcabae242a7c1a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Tue, 24 Jan 2017 00:37:11 -0500 Subject: [PATCH 3/8] Make lint build happy. --- config/render.go | 24 ++++++++++++------------ core/app.go | 1 + 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/config/render.go b/config/render.go index 1b59bc0a..c7539bf8 100644 --- a/config/render.go +++ b/config/render.go @@ -8,11 +8,11 @@ import ( "strings" ) -// A Render Application +// RenderApp encapsulates the rendered configuration file and the input and +// output paths. type RenderApp struct { - ConfigFlag string - RenderFlag string - RenderedConfig []byte + renderFlag string + renderedConfig []byte } // EmptyApp creates an empty application @@ -33,8 +33,7 @@ func NewRenderApp(configFlag string, renderFlag string) (*RenderApp, error) { return nil, errors.New("-render flag is invalid") } - a.ConfigFlag = configFlag - a.RenderFlag = renderFlag + a.renderFlag = renderFlag var data []byte if strings.HasPrefix(configFlag, "file://") { @@ -53,20 +52,21 @@ func NewRenderApp(configFlag string, renderFlag string) (*RenderApp, error) { "Could not apply template to config: %v", err) } - a.RenderedConfig = template + a.renderedConfig = template return a, nil } +// Run outputs the rendered configuration file to RenderApp.RenderFlag func (a *RenderApp) Run() { - if a.RenderFlag == "-" { - fmt.Printf("%s", a.RenderedConfig) + if a.renderFlag == "-" { + fmt.Printf("%s", a.renderedConfig) os.Exit(0) } - if strings.HasPrefix(a.RenderFlag, "file://") { + if strings.HasPrefix(a.renderFlag, "file://") { var err error - fName := strings.SplitAfter(a.RenderFlag, "file://")[1] - if err = ioutil.WriteFile(fName, a.RenderedConfig, 0644); err != nil { + fName := strings.SplitAfter(a.renderFlag, "file://")[1] + if err = ioutil.WriteFile(fName, a.renderedConfig, 0644); err != nil { panic(fmt.Errorf("Could not write config file: %s", err)) } os.Exit(0) diff --git a/core/app.go b/core/app.go index 8cdb2847..790ad144 100644 --- a/core/app.go +++ b/core/app.go @@ -29,6 +29,7 @@ var ( GitHash string ) +// Runnable is the returned interface for LoadApp. type Runnable interface { Run() } From 06e0dc63843f9e20ca99e19b6f16aae8880c7d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Wed, 25 Jan 2017 22:11:45 -0500 Subject: [PATCH 4/8] Moved the render code into core/app.go --- config/render.go | 75 ------------------------------------------------ core/app.go | 67 ++++++++++++++++++++++++++++++++---------- core/app_test.go | 3 +- 3 files changed, 53 insertions(+), 92 deletions(-) delete mode 100644 config/render.go diff --git a/config/render.go b/config/render.go deleted file mode 100644 index c7539bf8..00000000 --- a/config/render.go +++ /dev/null @@ -1,75 +0,0 @@ -package config - -import ( - "errors" - "fmt" - "io/ioutil" - "os" - "strings" -) - -// RenderApp encapsulates the rendered configuration file and the input and -// output paths. -type RenderApp struct { - renderFlag string - renderedConfig []byte -} - -// EmptyApp creates an empty application -func EmptyApp() *RenderApp { - app := &RenderApp{} - return app -} - -// NewRenderApp creates a new Config Rendering application -func NewRenderApp(configFlag string, renderFlag string) (*RenderApp, error) { - - a := EmptyApp() - - if configFlag == "" { - return nil, errors.New("-config flag is required") - } - if renderFlag != "-" && !strings.HasPrefix(renderFlag, "file://") { - return nil, errors.New("-render flag is invalid") - } - - a.renderFlag = renderFlag - - var data []byte - if strings.HasPrefix(configFlag, "file://") { - var err error - fName := strings.SplitAfter(configFlag, "file://")[1] - if data, err = ioutil.ReadFile(fName); err != nil { - return nil, fmt.Errorf("Could not read config file: %s", err) - } - } else { - data = []byte(configFlag) - } - - template, err := ApplyTemplate(data) - if err != nil { - return nil, fmt.Errorf( - "Could not apply template to config: %v", err) - } - - a.renderedConfig = template - - return a, nil -} - -// Run outputs the rendered configuration file to RenderApp.RenderFlag -func (a *RenderApp) Run() { - if a.renderFlag == "-" { - fmt.Printf("%s", a.renderedConfig) - os.Exit(0) - } - if strings.HasPrefix(a.renderFlag, "file://") { - var err error - fName := strings.SplitAfter(a.renderFlag, "file://")[1] - if err = ioutil.WriteFile(fName, a.renderedConfig, 0644); err != nil { - panic(fmt.Errorf("Could not write config file: %s", err)) - } - os.Exit(0) - } - os.Exit(1) -} diff --git a/core/app.go b/core/app.go index 790ad144..d6516d5e 100644 --- a/core/app.go +++ b/core/app.go @@ -4,6 +4,7 @@ import ( "encoding/json" "flag" "fmt" + "io/ioutil" "os" "strings" "sync" @@ -29,11 +30,6 @@ var ( GitHash string ) -// Runnable is the returned interface for LoadApp. -type Runnable interface { - Run() -} - // App encapsulates the state of ContainerPilot after the initial setup. // after it is run, it can be reloaded and paused with signals. type App struct { @@ -64,7 +60,7 @@ func EmptyApp() *App { } // LoadApp parses the commandline arguments and loads the config -func LoadApp() (Runnable, error) { +func LoadApp() (*App, error) { var configFlag string var renderFlag string @@ -74,7 +70,7 @@ func LoadApp() (Runnable, error) { flag.StringVar(&configFlag, "config", "", "JSON config or file:// path to JSON config file.") flag.StringVar(&renderFlag, "render", "", - "- for stdout or file:// path where to save redenred JSON config file.") + "- for stdout or file:// path where to save rendered JSON config file.") flag.BoolVar(&versionFlag, "version", false, "Show version identifier and quit.") flag.Parse() } @@ -85,16 +81,16 @@ func LoadApp() (Runnable, error) { if configFlag == "" { configFlag = os.Getenv("CONTAINERPILOT") } - - os.Setenv("CONTAINERPILOT_PID", fmt.Sprintf("%v", os.Getpid())) - var app Runnable - var err error - if renderFlag == "" { - app, err = NewApp(configFlag) - } else { - app, err = config.NewRenderApp(configFlag, renderFlag) + if renderFlag != "" { + err := renderConfig(configFlag, renderFlag) + if err != nil { + return nil, err + } + os.Exit(0) } + os.Setenv("CONTAINERPILOT_PID", fmt.Sprintf("%v", os.Getpid())) + app, err := NewApp(configFlag) if err != nil { return nil, err } @@ -140,6 +136,47 @@ func NewApp(configFlag string) (*App, error) { return a, nil } +// renderConfig renders the templated config in configFlag to renderFlag. +func renderConfig(configFlag string, renderFlag string) error { + if configFlag == "" { + return fmt.Errorf( + "-config flag is required: '%s'", configFlag) + } + + // This is not DRY, the same code path occurs in ParseConfig + var data []byte + if strings.HasPrefix(configFlag, "file://") { + var err error + fName := strings.SplitAfter(configFlag, "file://")[1] + if data, err = ioutil.ReadFile(fName); err != nil { + return fmt.Errorf("Could not read config file: %s", err) + } + } else { + data = []byte(configFlag) + } + + template, err := config.ApplyTemplate(data) + if err != nil { + return fmt.Errorf( + "Could not apply template to config: %v", err) + } + + // Save the template, either to stdout or to file://... + if renderFlag == "-" { + fmt.Printf("%s", template) + } else if strings.HasPrefix(renderFlag, "file://") { + var err error + fName := strings.SplitAfter(renderFlag, "file://")[1] + if err = ioutil.WriteFile(fName, template, 0644); err != nil { + return fmt.Errorf("Could not write config file: %s", err) + } + } else { + return fmt.Errorf("-render flag is invalid: '%s'", renderFlag) + } + + return nil +} + // Normalize the validated service name as an environment variable func getEnvVarNameFromService(service string) string { envKey := strings.ToUpper(service) diff --git a/core/app_test.go b/core/app_test.go index 4bd406c0..09aca13c 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -58,11 +58,10 @@ func TestValidConfigParse(t *testing.T) { os.Setenv("TEST", "HELLO") os.Args = []string{"this", "-config", testJSON, "/testdata/test.sh", "valid1", "--debug"} - runnable, err := LoadApp() + app, err := LoadApp() if err != nil { t.Fatalf("Unexpected error in LoadApp: %v", err) } - var app = runnable.(*App) if len(app.Backends) != 2 || len(app.Services) != 2 { t.Fatalf("Expected 2 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) From d1fdabe1ebf32f29a5a53e90c5779c81a3278ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Thu, 26 Jan 2017 01:44:57 -0500 Subject: [PATCH 5/8] Add unit tests for new -render function. --- core/app_test.go | 109 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/core/app_test.go b/core/app_test.go index 09aca13c..48484829 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -3,6 +3,7 @@ package core import ( "flag" "os" + "os/exec" "reflect" "strings" "testing" @@ -48,7 +49,13 @@ var testJSON = `{ "name": "upstreamB", "poll": 79, "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh {{.ENV_NOT_FOUND}}" - } + }, + { + "name": "upstreamC{{.TEST}}" + "poll": 11, + "onChange": "/bin/to/onChangeEvent/for/upstream.C.sh", + "tag": "dev" + } ] } ` @@ -255,6 +262,88 @@ func TestPidEnvVar(t *testing.T) { } } +func TestInvalidRenderConfigFile(t *testing.T) { + defer argTestCleanup(argTestSetup()) + testRenderExpectError(t, "file:///xxxx", "-", + "Could not read config file: open /xxxx: no such file or directory") +} + +func TestInvalidRenderFileConfig(t *testing.T) { + defer argTestCleanup(argTestSetup()) + testRenderExpectError(t, testJSON, "file:///a/b/c/d/e/f.json", + "Could not write config file: open /a/b/c/d/e/f.json: no such file or directory") +} + +func TestRenderConfig(t *testing.T) { + // Because of the "exit(0)" in LoadApp, we need to use this testing pattern + // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go + if render_file := os.Getenv("__RENDER_FILE"); render_file != "" { + defer argTestCleanup(argTestSetup()) + os.Args = []string{"this", "-config", testJSON, "-render", render_file} + _, err := LoadApp() + t.Fatalf("LoadApp failed with err %v", err) + return + } +} + +func TestRenderConfigFileStdout(t *testing.T) { + // Because of the "exit(0)" in LoadApp, we need to use this testing pattern + // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go + defer os.Remove("testJSON.json") + defer os.Remove("testJSON-stdout.json") + + // Render to file + cmd := exec.Command(os.Args[0], "-test.run=TestRenderConfig") + cmd.Env = append(os.Environ(), "__RENDER_FILE=file://testJSON.json") + if err := cmd.Run(); err != nil { + t.Fatalf("process ran with err %v, want exit status 0", err) + } + if exists, err := fileExists("testJSON.json"); !exists || err != nil { + t.Errorf("Expected file testJSON.json to exist.") + } + + // Render to stdout + cmd = exec.Command(os.Args[0], "-test.run=TestRenderConfig") + cmd.Env = append(os.Environ(), "__RENDER_FILE=-") + cmd.Stdout, _ = os.Create("testJSON-stdout.json") + if err := cmd.Run(); err != nil { + t.Fatalf("process ran with err %v, want exit status 0", err) + } + + // Assert they are the same size, should suffice to accept that + // the files are the same. + a_file, _ := os.Open("testJSON.json") + a_file_stat, _ := a_file.Stat() + b_file, _ := os.Open("testJSON-stdout.json") + b_file_stat, _ := b_file.Stat() + if a_file_stat.Size() != b_file_stat.Size() { + t.Fatalf("Expected the rendered files to be of the same size") + } + return +} + +func TestRenderConfigIsValid(t *testing.T) { + defer os.Remove("testJSON.json") + + // Render to file + cmd := exec.Command(os.Args[0], "-test.run=TestRenderConfig") + cmd.Env = append(os.Environ(), "TEST=SUCCESS", "__RENDER_FILE=file://testJSON.json") + cmd.Run() + + defer argTestCleanup(argTestSetup()) + + os.Setenv("TEST", "FAILED") + os.Args = []string{"this", "-config", "file://testJSON.json", "/testdata/test.sh", "valid1", "--debug"} + app, err := LoadApp() + if err != nil { + t.Fatalf("Unexpected error in LoadApp: %v", err) + } + if strings.HasSuffix(app.Backends[2].Name, "SUCCESS") { + t.Errorf("Expected Backend[2] name to end in %s, but got: %s", "SUCCESS", app.Backends[2].Name) + } + +} + // ---------------------------------------------------- // test helpers @@ -268,6 +357,13 @@ func argTestCleanup(oldArgs []string) { os.Args = oldArgs } +func testRenderExpectError(t *testing.T, testJSON string, render string, expected string) { + os.Args = []string{"this", "-config", testJSON, "-render", render} + if _, err := LoadApp(); err != nil && !strings.Contains(err.Error(), expected) { + t.Errorf("Excepted %s but got %s", expected, err) + } +} + func testParseExpectError(t *testing.T, testJSON string, expected string) { os.Args = []string{"this", "-config", testJSON, "/testdata/test.sh", "test", "--debug"} if _, err := LoadApp(); err != nil && !strings.Contains(err.Error(), expected) { @@ -299,3 +395,14 @@ func validateCommandParsed(t *testing.T, name string, parsed *commands.Command, t.Errorf("%s arguments not configured: %s != %s", name, parsed.Args, expectedArgs) } } + +func fileExists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return true, err +} From cdf7353ae8c374617f29e659513055bb4b2ffd25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Thu, 26 Jan 2017 02:07:46 -0500 Subject: [PATCH 6/8] Fixed lint issues and Regression test. --- core/app_test.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/core/app_test.go b/core/app_test.go index 48484829..3af64687 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -50,12 +50,11 @@ var testJSON = `{ "poll": 79, "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh {{.ENV_NOT_FOUND}}" }, - { - "name": "upstreamC{{.TEST}}" - "poll": 11, - "onChange": "/bin/to/onChangeEvent/for/upstream.C.sh", - "tag": "dev" - } + { + "name": "upstreamC{{.TEST}}", + "poll": 79, + "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh" + } ] } ` @@ -70,7 +69,7 @@ func TestValidConfigParse(t *testing.T) { t.Fatalf("Unexpected error in LoadApp: %v", err) } - if len(app.Backends) != 2 || len(app.Services) != 2 { + if len(app.Backends) != 3 || len(app.Services) != 2 { t.Fatalf("Expected 2 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) } args := flag.Args() @@ -277,9 +276,9 @@ func TestInvalidRenderFileConfig(t *testing.T) { func TestRenderConfig(t *testing.T) { // Because of the "exit(0)" in LoadApp, we need to use this testing pattern // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go - if render_file := os.Getenv("__RENDER_FILE"); render_file != "" { + if renderFile := os.Getenv("__RENDER_FILE"); renderFile != "" { defer argTestCleanup(argTestSetup()) - os.Args = []string{"this", "-config", testJSON, "-render", render_file} + os.Args = []string{"this", "-config", testJSON, "-render", renderFile} _, err := LoadApp() t.Fatalf("LoadApp failed with err %v", err) return @@ -312,11 +311,11 @@ func TestRenderConfigFileStdout(t *testing.T) { // Assert they are the same size, should suffice to accept that // the files are the same. - a_file, _ := os.Open("testJSON.json") - a_file_stat, _ := a_file.Stat() - b_file, _ := os.Open("testJSON-stdout.json") - b_file_stat, _ := b_file.Stat() - if a_file_stat.Size() != b_file_stat.Size() { + aFile, _ := os.Open("testJSON.json") + aFileStat, _ := aFile.Stat() + bFile, _ := os.Open("testJSON-stdout.json") + bFileStat, _ := bFile.Stat() + if aFileStat.Size() != bFileStat.Size() { t.Fatalf("Expected the rendered files to be of the same size") } return From 3bab056e507c1c8795740d7a28042832b8670049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Sodre=CC=81?= Date: Thu, 26 Jan 2017 21:20:05 -0500 Subject: [PATCH 7/8] Changed template rendering call syntax. - containerpilot -template [-config file://] [-out file://] --- core/app.go | 11 +++++++---- core/app_test.go | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/core/app.go b/core/app.go index d6516d5e..d7bb1cd8 100644 --- a/core/app.go +++ b/core/app.go @@ -63,14 +63,17 @@ func EmptyApp() *App { func LoadApp() (*App, error) { var configFlag string - var renderFlag string var versionFlag bool + var renderFlag string + var templateFlag bool if !flag.Parsed() { flag.StringVar(&configFlag, "config", "", "JSON config or file:// path to JSON config file.") - flag.StringVar(&renderFlag, "render", "", - "- for stdout or file:// path where to save rendered JSON config file.") + flag.BoolVar(&templateFlag, "template", false, + "Render template and quit. (default: false)") + flag.StringVar(&renderFlag, "out", "-", + "-(default) for stdout or file:// path where to save rendered JSON config file.") flag.BoolVar(&versionFlag, "version", false, "Show version identifier and quit.") flag.Parse() } @@ -81,7 +84,7 @@ func LoadApp() (*App, error) { if configFlag == "" { configFlag = os.Getenv("CONTAINERPILOT") } - if renderFlag != "" { + if templateFlag { err := renderConfig(configFlag, renderFlag) if err != nil { return nil, err diff --git a/core/app_test.go b/core/app_test.go index 3af64687..821b6b8a 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -70,7 +70,7 @@ func TestValidConfigParse(t *testing.T) { } if len(app.Backends) != 3 || len(app.Services) != 2 { - t.Fatalf("Expected 2 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) + t.Fatalf("Expected 3 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) } args := flag.Args() if len(args) != 3 || args[0] != "/testdata/test.sh" { @@ -278,7 +278,7 @@ func TestRenderConfig(t *testing.T) { // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go if renderFile := os.Getenv("__RENDER_FILE"); renderFile != "" { defer argTestCleanup(argTestSetup()) - os.Args = []string{"this", "-config", testJSON, "-render", renderFile} + os.Args = []string{"this", "-template", "-config", testJSON, "-out", renderFile} _, err := LoadApp() t.Fatalf("LoadApp failed with err %v", err) return @@ -357,7 +357,7 @@ func argTestCleanup(oldArgs []string) { } func testRenderExpectError(t *testing.T, testJSON string, render string, expected string) { - os.Args = []string{"this", "-config", testJSON, "-render", render} + os.Args = []string{"this", "-template", "-config", testJSON, "-out", render} if _, err := LoadApp(); err != nil && !strings.Contains(err.Error(), expected) { t.Errorf("Excepted %s but got %s", expected, err) } From 1c63b1f7c386129ba3d8a02da5c98603ed4c5c70 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 31 Jan 2017 09:26:39 -0500 Subject: [PATCH 8/8] Factor out config rendering code and move to config package --- config/config.go | 55 +++++++++++++++----- config/template_test.go | 93 +++++++++++++++++++++++++++++++++ core/app.go | 45 +--------------- core/app_test.go | 110 +--------------------------------------- 4 files changed, 139 insertions(+), 164 deletions(-) diff --git a/config/config.go b/config/config.go index c062c7e3..835a89d6 100644 --- a/config/config.go +++ b/config/config.go @@ -163,27 +163,35 @@ func (cfg *rawConfig) parseTasks() ([]*tasks.Task, error) { return tasks, nil } -// ParseConfig parses a raw config flag -func ParseConfig(configFlag string) (*Config, error) { - if configFlag == "" { - return nil, errors.New("-config flag is required") +// RenderConfig renders the templated config in configFlag to renderFlag. +func RenderConfig(configFlag, renderFlag string) error { + template, err := renderConfigTemplate(configFlag) + if err != nil { + return err } - var data []byte - if strings.HasPrefix(configFlag, "file://") { + // Save the template, either to stdout or to file://... + if renderFlag == "-" { + fmt.Printf("%s", template) + } else if strings.HasPrefix(renderFlag, "file://") { var err error - fName := strings.SplitAfter(configFlag, "file://")[1] - if data, err = ioutil.ReadFile(fName); err != nil { - return nil, fmt.Errorf("Could not read config file: %s", err) + fName := strings.SplitAfter(renderFlag, "file://")[1] + if err = ioutil.WriteFile(fName, template, 0644); err != nil { + return fmt.Errorf("Could not write config file: %s", err) } } else { - data = []byte(configFlag) + return fmt.Errorf("-render flag is invalid: '%s'", renderFlag) } - template, err := ApplyTemplate(data) + return nil +} + +// ParseConfig parses a raw config flag +func ParseConfig(configFlag string) (*Config, error) { + + template, err := renderConfigTemplate(configFlag) if err != nil { - return nil, fmt.Errorf( - "Could not apply template to config: %v", err) + return nil, err } configMap, err := unmarshalConfig(template) if err != nil { @@ -270,6 +278,27 @@ func ParseConfig(configFlag string) (*Config, error) { return cfg, nil } +func renderConfigTemplate(configFlag string) ([]byte, error) { + if configFlag == "" { + return nil, errors.New("-config flag is required") + } + var data []byte + if strings.HasPrefix(configFlag, "file://") { + var err error + fName := strings.SplitAfter(configFlag, "file://")[1] + if data, err = ioutil.ReadFile(fName); err != nil { + return nil, fmt.Errorf("Could not read config file: %s", err) + } + } else { + data = []byte(configFlag) + } + template, err := ApplyTemplate(data) + if err != nil { + err = fmt.Errorf("Could not apply template to config: %v", err) + } + return template, err +} + func unmarshalConfig(data []byte) (map[string]interface{}, error) { var config map[string]interface{} if err := json.Unmarshal(data, &config); err != nil { diff --git a/config/template_test.go b/config/template_test.go index c6cb4dff..f57632bd 100644 --- a/config/template_test.go +++ b/config/template_test.go @@ -1,8 +1,15 @@ package config import ( + "io/ioutil" + "os" + "path/filepath" "reflect" + "strings" "testing" + + // need to import this so that we have a registered backend + _ "github.com/joyent/containerpilot/discovery/consul" ) func TestParseEnvironment(t *testing.T) { @@ -28,6 +35,74 @@ func TestTemplate(t *testing.T) { validateTemplate(t, "Regex Replace All", `Hello, {{.NAME | regexReplaceAll "[epa]+" "_" }}!`, env, "Hello, T_m_l_t_!") } +func TestInvalidRenderConfigFile(t *testing.T) { + testRenderExpectError(t, "file:///xxxx", "-", + "Could not read config file: open /xxxx: no such file or directory") +} + +func TestInvalidRenderFileConfig(t *testing.T) { + var testJSON = `{"consul": "consul:8500"}` + testRenderExpectError(t, testJSON, "file:///a/b/c/d/e/f.json", + "Could not write config file: open /a/b/c/d/e/f.json: no such file or directory") +} + +func TestRenderConfigFileStdout(t *testing.T) { + + var testJSON = `{ + "consul": "consul:8500", + "backends": [{ + "name": "upstreamA", + "poll": 11, + "onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"}]}` + + // Render to file + defer os.Remove("testJSON.json") + if err := RenderConfig(testJSON, "file://testJSON.json"); err != nil { + t.Fatalf("Expected no error from renderConfigTemplate but got: %v", err) + } + if exists, err := fileExists("testJSON.json"); !exists || err != nil { + t.Errorf("Expected file testJSON.json to exist.") + } + + // Render to stdout + fname := filepath.Join(os.TempDir(), "stdout") + temp, _ := os.Create(fname) + old := os.Stdout + os.Stdout = temp + if err := RenderConfig(testJSON, "-"); err != nil { + t.Fatalf("Expected no error from renderConfigTemplate but got: %v", err) + } + temp.Close() + os.Stdout = old + + renderedOut, _ := ioutil.ReadFile(fname) + renderedFile, _ := ioutil.ReadFile("testJSON.json") + if string(renderedOut) != string(renderedFile) { + t.Fatalf("Expected the rendered file and stdout to be identical") + } +} + +func TestRenderedConfigIsParseable(t *testing.T) { + + var testJSON = `{ + "consul": "consul:8500", + "backends": [{ + "name": "upstreamA{{.TESTRENDERCONFIGISPARSEABLE}}", + "poll": 11, + "onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"}]}` + + os.Setenv("TESTRENDERCONFIGISPARSEABLE", "-ok") + template, _ := renderConfigTemplate(testJSON) + config, err := ParseConfig(string(template)) + if err != nil { + t.Fatalf("Unexpected error in ParseConfig: %v", err) + } + name := config.Backends[0].Name + if name != "upstreamA-ok" { + t.Fatalf("Expected Backend[0] name to be upstreamA-ok but got %s", name) + } +} + // Helper Functions func validateParseEnvironment(t *testing.T, message string, environ []string, expected Environment) { @@ -51,3 +126,21 @@ func validateTemplate(t *testing.T, name string, template string, env Environmen t.Fatalf("%s - Expected %s but got: %s", name, expected, strRes) } } + +func testRenderExpectError(t *testing.T, testJSON, render, expected string) { + err := RenderConfig(testJSON, render) + if err == nil || !strings.Contains(err.Error(), expected) { + t.Errorf("Expected %s but got %s", expected, err) + } +} + +func fileExists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return true, err +} diff --git a/core/app.go b/core/app.go index d7bb1cd8..93958acf 100644 --- a/core/app.go +++ b/core/app.go @@ -4,7 +4,6 @@ import ( "encoding/json" "flag" "fmt" - "io/ioutil" "os" "strings" "sync" @@ -85,7 +84,8 @@ func LoadApp() (*App, error) { configFlag = os.Getenv("CONTAINERPILOT") } if templateFlag { - err := renderConfig(configFlag, renderFlag) + + err := config.RenderConfig(configFlag, renderFlag) if err != nil { return nil, err } @@ -139,47 +139,6 @@ func NewApp(configFlag string) (*App, error) { return a, nil } -// renderConfig renders the templated config in configFlag to renderFlag. -func renderConfig(configFlag string, renderFlag string) error { - if configFlag == "" { - return fmt.Errorf( - "-config flag is required: '%s'", configFlag) - } - - // This is not DRY, the same code path occurs in ParseConfig - var data []byte - if strings.HasPrefix(configFlag, "file://") { - var err error - fName := strings.SplitAfter(configFlag, "file://")[1] - if data, err = ioutil.ReadFile(fName); err != nil { - return fmt.Errorf("Could not read config file: %s", err) - } - } else { - data = []byte(configFlag) - } - - template, err := config.ApplyTemplate(data) - if err != nil { - return fmt.Errorf( - "Could not apply template to config: %v", err) - } - - // Save the template, either to stdout or to file://... - if renderFlag == "-" { - fmt.Printf("%s", template) - } else if strings.HasPrefix(renderFlag, "file://") { - var err error - fName := strings.SplitAfter(renderFlag, "file://")[1] - if err = ioutil.WriteFile(fName, template, 0644); err != nil { - return fmt.Errorf("Could not write config file: %s", err) - } - } else { - return fmt.Errorf("-render flag is invalid: '%s'", renderFlag) - } - - return nil -} - // Normalize the validated service name as an environment variable func getEnvVarNameFromService(service string) string { envKey := strings.ToUpper(service) diff --git a/core/app_test.go b/core/app_test.go index 821b6b8a..09aca13c 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -3,7 +3,6 @@ package core import ( "flag" "os" - "os/exec" "reflect" "strings" "testing" @@ -49,11 +48,6 @@ var testJSON = `{ "name": "upstreamB", "poll": 79, "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh {{.ENV_NOT_FOUND}}" - }, - { - "name": "upstreamC{{.TEST}}", - "poll": 79, - "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh" } ] } @@ -69,8 +63,8 @@ func TestValidConfigParse(t *testing.T) { t.Fatalf("Unexpected error in LoadApp: %v", err) } - if len(app.Backends) != 3 || len(app.Services) != 2 { - t.Fatalf("Expected 3 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) + if len(app.Backends) != 2 || len(app.Services) != 2 { + t.Fatalf("Expected 2 backends and 2 services but got: len(backends)=%d, len(services)=%d", len(app.Backends), len(app.Services)) } args := flag.Args() if len(args) != 3 || args[0] != "/testdata/test.sh" { @@ -261,88 +255,6 @@ func TestPidEnvVar(t *testing.T) { } } -func TestInvalidRenderConfigFile(t *testing.T) { - defer argTestCleanup(argTestSetup()) - testRenderExpectError(t, "file:///xxxx", "-", - "Could not read config file: open /xxxx: no such file or directory") -} - -func TestInvalidRenderFileConfig(t *testing.T) { - defer argTestCleanup(argTestSetup()) - testRenderExpectError(t, testJSON, "file:///a/b/c/d/e/f.json", - "Could not write config file: open /a/b/c/d/e/f.json: no such file or directory") -} - -func TestRenderConfig(t *testing.T) { - // Because of the "exit(0)" in LoadApp, we need to use this testing pattern - // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go - if renderFile := os.Getenv("__RENDER_FILE"); renderFile != "" { - defer argTestCleanup(argTestSetup()) - os.Args = []string{"this", "-template", "-config", testJSON, "-out", renderFile} - _, err := LoadApp() - t.Fatalf("LoadApp failed with err %v", err) - return - } -} - -func TestRenderConfigFileStdout(t *testing.T) { - // Because of the "exit(0)" in LoadApp, we need to use this testing pattern - // http://stackoverflow.com/questions/26225513/how-to-test-os-exit-scenarios-in-go - defer os.Remove("testJSON.json") - defer os.Remove("testJSON-stdout.json") - - // Render to file - cmd := exec.Command(os.Args[0], "-test.run=TestRenderConfig") - cmd.Env = append(os.Environ(), "__RENDER_FILE=file://testJSON.json") - if err := cmd.Run(); err != nil { - t.Fatalf("process ran with err %v, want exit status 0", err) - } - if exists, err := fileExists("testJSON.json"); !exists || err != nil { - t.Errorf("Expected file testJSON.json to exist.") - } - - // Render to stdout - cmd = exec.Command(os.Args[0], "-test.run=TestRenderConfig") - cmd.Env = append(os.Environ(), "__RENDER_FILE=-") - cmd.Stdout, _ = os.Create("testJSON-stdout.json") - if err := cmd.Run(); err != nil { - t.Fatalf("process ran with err %v, want exit status 0", err) - } - - // Assert they are the same size, should suffice to accept that - // the files are the same. - aFile, _ := os.Open("testJSON.json") - aFileStat, _ := aFile.Stat() - bFile, _ := os.Open("testJSON-stdout.json") - bFileStat, _ := bFile.Stat() - if aFileStat.Size() != bFileStat.Size() { - t.Fatalf("Expected the rendered files to be of the same size") - } - return -} - -func TestRenderConfigIsValid(t *testing.T) { - defer os.Remove("testJSON.json") - - // Render to file - cmd := exec.Command(os.Args[0], "-test.run=TestRenderConfig") - cmd.Env = append(os.Environ(), "TEST=SUCCESS", "__RENDER_FILE=file://testJSON.json") - cmd.Run() - - defer argTestCleanup(argTestSetup()) - - os.Setenv("TEST", "FAILED") - os.Args = []string{"this", "-config", "file://testJSON.json", "/testdata/test.sh", "valid1", "--debug"} - app, err := LoadApp() - if err != nil { - t.Fatalf("Unexpected error in LoadApp: %v", err) - } - if strings.HasSuffix(app.Backends[2].Name, "SUCCESS") { - t.Errorf("Expected Backend[2] name to end in %s, but got: %s", "SUCCESS", app.Backends[2].Name) - } - -} - // ---------------------------------------------------- // test helpers @@ -356,13 +268,6 @@ func argTestCleanup(oldArgs []string) { os.Args = oldArgs } -func testRenderExpectError(t *testing.T, testJSON string, render string, expected string) { - os.Args = []string{"this", "-template", "-config", testJSON, "-out", render} - if _, err := LoadApp(); err != nil && !strings.Contains(err.Error(), expected) { - t.Errorf("Excepted %s but got %s", expected, err) - } -} - func testParseExpectError(t *testing.T, testJSON string, expected string) { os.Args = []string{"this", "-config", testJSON, "/testdata/test.sh", "test", "--debug"} if _, err := LoadApp(); err != nil && !strings.Contains(err.Error(), expected) { @@ -394,14 +299,3 @@ func validateCommandParsed(t *testing.T, name string, parsed *commands.Command, t.Errorf("%s arguments not configured: %s != %s", name, parsed.Args, expectedArgs) } } - -func fileExists(path string) (bool, error) { - _, err := os.Stat(path) - if err == nil { - return true, nil - } - if os.IsNotExist(err) { - return false, nil - } - return true, err -}