diff options
author | Dan Duvall <dduvall@wikimedia.org> | 2017-06-07 12:36:46 -0700 |
---|---|---|
committer | Dan Duvall <dduvall@wikimedia.org> | 2017-06-12 09:22:56 -0700 |
commit | b6c3ad2aac5263ac7eb7c1c34dcd55075c8fbd04 (patch) | |
tree | 6c736b918eb4a098790571909105183a5bb8ab58 | |
parent | a3124c04207a5fb7815a407f1978a91c54d22f7b (diff) | |
download | blubber-b6c3ad2aac5263ac7eb7c1c34dcd55075c8fbd04.tar.gz |
Fix variant expansion for bool config fields
Summary:
Refactored bool config fields to use a new `config.Flag` type that keeps
track of whether it was set by unmarshalled data or merged from another
instance, fixing the behavior of these fields when overwritten by
variants.
Fixes T166353
Test Plan: Run config unit tests (`cd config; go test`)
Reviewers: thcipriani, Joe, hashar, mobrovac, mmodell, #release-engineering-team
Reviewed By: mobrovac
Tags: #release-engineering-team
Maniphest Tasks: T166353
Differential Revision: https://phabricator.wikimedia.org/D680
-rw-r--r-- | config/common.go | 5 | ||||
-rw-r--r-- | config/flag.go | 23 | ||||
-rw-r--r-- | config/flag_test.go | 31 | ||||
-rw-r--r-- | config/npm.go | 23 | ||||
-rw-r--r-- | docker/compiler.go | 2 |
5 files changed, 67 insertions, 17 deletions
diff --git a/config/common.go b/config/common.go index 6f3b97a..31916dd 100644 --- a/config/common.go +++ b/config/common.go @@ -9,7 +9,7 @@ type CommonConfig struct { Apt AptConfig `yaml:"apt"` Npm NpmConfig `yaml:"npm"` Runs RunsConfig `yaml:"runs"` - SharedVolume bool `yaml:"sharedvolume"` + SharedVolume Flag `yaml:"sharedvolume"` EntryPoint []string `yaml:"entrypoint"` } @@ -21,8 +21,7 @@ func (cc1 *CommonConfig) Merge(cc2 CommonConfig) { cc1.Apt.Merge(cc2.Apt) cc1.Npm.Merge(cc2.Npm) cc1.Runs.Merge(cc2.Runs) - - cc1.SharedVolume = cc1.SharedVolume || cc2.SharedVolume + cc1.SharedVolume.Merge(cc2.SharedVolume) if len(cc1.EntryPoint) < 1 { cc1.EntryPoint = cc2.EntryPoint diff --git a/config/flag.go b/config/flag.go new file mode 100644 index 0000000..aa4cc68 --- /dev/null +++ b/config/flag.go @@ -0,0 +1,23 @@ +package config + +type Flag struct { + True bool + set bool +} + +func (flag *Flag) UnmarshalYAML(unmarshal func(interface {}) error) error { + if err := unmarshal(&flag.True); err != nil { + return err + } + + flag.set = true + + return nil +} + +func (flag *Flag) Merge(flag2 Flag) { + if flag2.set { + flag.True = flag2.True + flag.set = true + } +} diff --git a/config/flag_test.go b/config/flag_test.go new file mode 100644 index 0000000..39aa254 --- /dev/null +++ b/config/flag_test.go @@ -0,0 +1,31 @@ +package config_test + +import ( + "testing" + "gopkg.in/stretchr/testify.v1/assert" + + "phabricator.wikimedia.org/source/blubber.git/config" +) + +const yaml = `--- +npm: { install: true } +sharedvolume: false + +variants: + development: + sharedvolume: true + npm: { install: false } +` + +func TestFlagOverwrite(t *testing.T) { + cfg, err := config.ReadConfig([]byte(yaml)) + + assert.Nil(t, err) + + variant, err := config.ExpandVariant(cfg, "development") + + assert.Nil(t, err) + + assert.False(t, variant.Npm.Install.True) + assert.True(t, variant.SharedVolume.True) +} diff --git a/config/npm.go b/config/npm.go index f02df54..91cfc26 100644 --- a/config/npm.go +++ b/config/npm.go @@ -1,20 +1,19 @@ package config import ( - "bytes" "path" "phabricator.wikimedia.org/source/blubber.git/build" ) -const TempNpmInstallDir = "/tmp/node-deps/" +const tempNpmInstallDir = "/tmp/node-deps/" type NpmConfig struct { - Install bool `yaml:"install"` + Install Flag `yaml:"install"` Env string `yaml:"env"` } func (npm *NpmConfig) Merge(npm2 NpmConfig) { - npm.Install = npm.Install || npm2.Install + npm.Install.Merge(npm2.Install) if npm2.Env != "" { npm.Env = npm2.Env @@ -22,25 +21,23 @@ func (npm *NpmConfig) Merge(npm2 NpmConfig) { } func (npm NpmConfig) InstructionsForPhase(phase build.Phase) []build.Instruction{ - if npm.Install { + if npm.Install.True { switch phase { case build.PhasePreInstall: - npmCmd := new(bytes.Buffer) - - npmCmd.WriteString("npm install") + npmCmd := "npm install" if npm.Env == "production" { - npmCmd.WriteString(" --production && npm dedupe") + npmCmd += " --production && npm dedupe" } return []build.Instruction{ - {build.Run, []string{"mkdir -p ", TempNpmInstallDir}}, - {build.Copy, []string{"package.json", TempNpmInstallDir}}, - {build.Run, []string{"cd ", TempNpmInstallDir, " && ", npmCmd.String()}}, + {build.Run, []string{"mkdir -p ", tempNpmInstallDir}}, + {build.Copy, []string{"package.json", tempNpmInstallDir}}, + {build.Run, []string{"cd ", tempNpmInstallDir, " && ", npmCmd}}, } case build.PhasePostInstall: return []build.Instruction{ - {build.Run, []string{"mv ", path.Join(TempNpmInstallDir, "node_modules"), " ./"}}, + {build.Run, []string{"mv ", path.Join(tempNpmInstallDir, "node_modules"), " ./"}}, } } } diff --git a/docker/compiler.go b/docker/compiler.go index 76b52a1..9f1e5d1 100644 --- a/docker/compiler.go +++ b/docker/compiler.go @@ -49,7 +49,7 @@ func CompileStage(buffer *bytes.Buffer, stage string, vcfg *config.VariantConfig CompilePhase(buffer, vcfg, build.PhasePreInstall) - if vcfg.SharedVolume { + if vcfg.SharedVolume.True { Writeln(buffer, "VOLUME [\"", vcfg.Runs.In, "\"]") } else { Writeln(buffer, "COPY . \"", vcfg.Runs.In, "\"") |