From cf86a4d922061386ba2ecae39291535a9b34786e Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 21 Feb 2018 18:31:52 +0100 Subject: [PATCH] Simplify the marshaling of compose types.Config - Add `Version` to `types.Config` - Add a new `Services` types (that is just `[]ServiceConfig`) and add `MarshalYAML` method on it. - Clean other top-level custom marshaling as `Services` is the only one required. Signed-off-by: Vincent Demeester --- cli/command/stack/kubernetes/deploy.go | 4 +- cli/command/stack/kubernetes/loader.go | 27 +----------- cli/command/stack/kubernetes/loader_test.go | 11 ++--- cli/command/stack/loader/loader.go | 10 ++--- cli/command/stack/swarm/deploy_composefile.go | 2 +- cli/compose/loader/full-struct_test.go | 1 + cli/compose/loader/loader.go | 4 +- cli/compose/loader/loader_test.go | 3 ++ cli/compose/loader/merge_test.go | 7 ++++ cli/compose/loader/types_test.go | 41 ++++++++++--------- cli/compose/types/types.go | 20 ++++----- 11 files changed, 60 insertions(+), 70 deletions(-) diff --git a/cli/command/stack/kubernetes/deploy.go b/cli/command/stack/kubernetes/deploy.go index b319eba8b0..5149c8805d 100644 --- a/cli/command/stack/kubernetes/deploy.go +++ b/cli/command/stack/kubernetes/deploy.go @@ -22,11 +22,11 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { } // Parse the compose file - cfg, version, err := loader.LoadComposefile(dockerCli, opts) + cfg, err := loader.LoadComposefile(dockerCli, opts) if err != nil { return err } - stack, err := LoadStack(opts.Namespace, version, *cfg) + stack, err := LoadStack(opts.Namespace, *cfg) if err != nil { return err } diff --git a/cli/command/stack/kubernetes/loader.go b/cli/command/stack/kubernetes/loader.go index 0eca43b283..b4bcf96f5e 100644 --- a/cli/command/stack/kubernetes/loader.go +++ b/cli/command/stack/kubernetes/loader.go @@ -7,32 +7,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -type versionedConfig struct { - composetypes.Config - version string -} - -func (c versionedConfig) MarshalYAML() (interface{}, error) { - services := map[string]composetypes.ServiceConfig{} - for _, service := range c.Services { - services[service.Name] = service - } - return map[string]interface{}{ - "services": services, - "networks": c.Networks, - "volumes": c.Volumes, - "secrets": c.Secrets, - "configs": c.Configs, - "version": c.version, - }, nil -} - // LoadStack loads a stack from a Compose config, with a given name. -func LoadStack(name, version string, cfg composetypes.Config) (*apiv1beta1.Stack, error) { - res, err := yaml.Marshal(versionedConfig{ - version: version, - Config: cfg, - }) +func LoadStack(name string, cfg composetypes.Config) (*apiv1beta1.Stack, error) { + res, err := yaml.Marshal(cfg) if err != nil { return nil, err } diff --git a/cli/command/stack/kubernetes/loader_test.go b/cli/command/stack/kubernetes/loader_test.go index e79202cf2f..a96e66d47b 100644 --- a/cli/command/stack/kubernetes/loader_test.go +++ b/cli/command/stack/kubernetes/loader_test.go @@ -10,7 +10,8 @@ import ( ) func TestLoadStack(t *testing.T) { - s, err := LoadStack("foo", "3.1", composetypes.Config{ + s, err := LoadStack("foo", composetypes.Config{ + Version: "3.1", Filename: "banana", Services: []composetypes.ServiceConfig{ { @@ -29,16 +30,16 @@ func TestLoadStack(t *testing.T) { Name: "foo", }, Spec: apiv1beta1.StackSpec{ - ComposeFile: string(`configs: {} -networks: {} -secrets: {} + ComposeFile: string(`version: "3.1" services: bar: image: bar foo: image: foo -version: "3.1" +networks: {} volumes: {} +secrets: {} +configs: {} `), }, }, s) diff --git a/cli/command/stack/loader/loader.go b/cli/command/stack/loader/loader.go index 6283faa77b..b479094512 100644 --- a/cli/command/stack/loader/loader.go +++ b/cli/command/stack/loader/loader.go @@ -18,21 +18,21 @@ import ( ) // LoadComposefile parse the composefile specified in the cli and returns its Config and version. -func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, string, error) { +func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, error) { configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In()) if err != nil { - return nil, "", err + return nil, err } dicts := getDictsFrom(configDetails.ConfigFiles) config, err := loader.Load(configDetails) if err != nil { if fpe, ok := err.(*loader.ForbiddenPropertiesError); ok { - return nil, "", errors.Errorf("Compose file contains unsupported options:\n\n%s\n", + return nil, errors.Errorf("Compose file contains unsupported options:\n\n%s\n", propertyWarnings(fpe.Properties)) } - return nil, "", err + return nil, err } unsupportedProperties := loader.GetUnsupportedProperties(dicts...) @@ -46,7 +46,7 @@ func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes. fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n", propertyWarnings(deprecatedProperties)) } - return config, configDetails.Version, nil + return config, nil } func getDictsFrom(configFiles []composetypes.ConfigFile) []map[string]interface{} { diff --git a/cli/command/stack/swarm/deploy_composefile.go b/cli/command/stack/swarm/deploy_composefile.go index c45f463f8d..0a3f2ac720 100644 --- a/cli/command/stack/swarm/deploy_composefile.go +++ b/cli/command/stack/swarm/deploy_composefile.go @@ -18,7 +18,7 @@ import ( ) func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error { - config, _, err := loader.LoadComposefile(dockerCli, opts) + config, err := loader.LoadComposefile(dockerCli, opts) if err != nil { return err } diff --git a/cli/compose/loader/full-struct_test.go b/cli/compose/loader/full-struct_test.go index 3e73e7553c..3f2a587bce 100644 --- a/cli/compose/loader/full-struct_test.go +++ b/cli/compose/loader/full-struct_test.go @@ -8,6 +8,7 @@ import ( func fullExampleConfig(workingDir, homeDir string) *types.Config { return &types.Config{ + Version: "3.6", Services: services(workingDir, homeDir), Networks: networks(), Volumes: volumes(), diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 12bdf4b128..1065f2bfd8 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -98,7 +98,9 @@ func validateForbidden(configDict map[string]interface{}) error { func loadSections(config map[string]interface{}, configDetails types.ConfigDetails) (*types.Config, error) { var err error - cfg := types.Config{} + cfg := types.Config{ + Version: schema.Version(config), + } var loaders = []struct { key string diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index 8275627d0a..b93abb0eb7 100644 --- a/cli/compose/loader/loader_test.go +++ b/cli/compose/loader/loader_test.go @@ -119,6 +119,7 @@ func strPtr(val string) *string { } var sampleConfig = types.Config{ + Version: "3.0", Services: []types.ServiceConfig{ { Name: "foo", @@ -174,6 +175,7 @@ func TestParseYAML(t *testing.T) { func TestLoad(t *testing.T) { actual, err := Load(buildConfigDetails(sampleDict, nil)) require.NoError(t, err) + assert.Equal(t, sampleConfig.Version, actual.Version) assert.Equal(t, serviceSort(sampleConfig.Services), serviceSort(actual.Services)) assert.Equal(t, sampleConfig.Networks, actual.Networks) assert.Equal(t, sampleConfig.Volumes, actual.Volumes) @@ -573,6 +575,7 @@ networks: require.NoError(t, err) expected := &types.Config{ Filename: "filename.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "web", diff --git a/cli/compose/loader/merge_test.go b/cli/compose/loader/merge_test.go index 97a68043a8..d9e8599359 100644 --- a/cli/compose/loader/merge_test.go +++ b/cli/compose/loader/merge_test.go @@ -207,6 +207,7 @@ func TestLoadLogging(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -325,6 +326,7 @@ func TestLoadMultipleServicePorts(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -450,6 +452,7 @@ func TestLoadMultipleSecretsConfig(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -575,6 +578,7 @@ func TestLoadMultipleConfigobjsConfig(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -690,6 +694,7 @@ func TestLoadMultipleUlimits(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -808,6 +813,7 @@ func TestLoadMultipleNetworks(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "foo", @@ -895,6 +901,7 @@ func TestLoadMultipleConfigs(t *testing.T) { require.NoError(t, err) require.Equal(t, &types.Config{ Filename: "base.yml", + Version: "3.4", Services: []types.ServiceConfig{ { Name: "bar", diff --git a/cli/compose/loader/types_test.go b/cli/compose/loader/types_test.go index 2dee0b65b1..f27539f88f 100644 --- a/cli/compose/loader/types_test.go +++ b/cli/compose/loader/types_test.go @@ -9,26 +9,7 @@ import ( func TestMarshallConfig(t *testing.T) { cfg := fullExampleConfig("/foo", "/bar") - expected := `configs: {} -networks: - external-network: - name: external-network - external: true - other-external-network: - name: my-cool-network - external: true - other-network: - driver: overlay - driver_opts: - baz: "1" - foo: bar - ipam: - driver: overlay - config: - - subnet: 172.16.238.0/24 - - subnet: 2001:3984:3989::/64 - some-network: {} -secrets: {} + expected := `version: "3.6" services: foo: build: @@ -292,6 +273,24 @@ services: tmpfs: size: 10000 working_dir: /code +networks: + external-network: + name: external-network + external: true + other-external-network: + name: my-cool-network + external: true + other-network: + driver: overlay + driver_opts: + baz: "1" + foo: bar + ipam: + driver: overlay + config: + - subnet: 172.16.238.0/24 + - subnet: 2001:3984:3989::/64 + some-network: {} volumes: another-volume: name: user_specified_name @@ -314,6 +313,8 @@ volumes: baz: "1" foo: bar some-volume: {} +secrets: {} +configs: {} ` actual, err := yaml.Marshal(cfg) diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go index 26f7139a5a..107dc85784 100644 --- a/cli/compose/types/types.go +++ b/cli/compose/types/types.go @@ -71,26 +71,24 @@ func (cd ConfigDetails) LookupEnv(key string) (string, bool) { // Config is a full compose file configuration type Config struct { Filename string `yaml:"-"` - Services []ServiceConfig + Version string + Services Services Networks map[string]NetworkConfig Volumes map[string]VolumeConfig Secrets map[string]SecretConfig Configs map[string]ConfigObjConfig } -// MarshalYAML makes Config implement yaml.Marshaller -func (c *Config) MarshalYAML() (interface{}, error) { +// Services is a list of ServiceConfig +type Services []ServiceConfig + +// MarshalYAML makes Services implement yaml.Marshaller +func (s Services) MarshalYAML() (interface{}, error) { services := map[string]ServiceConfig{} - for _, service := range c.Services { + for _, service := range s { services[service.Name] = service } - return map[string]interface{}{ - "services": services, - "networks": c.Networks, - "volumes": c.Volumes, - "secrets": c.Secrets, - "configs": c.Configs, - }, nil + return services, nil } // ServiceConfig is the configuration of one service