From 729d07a371c0539453f5754303d40b2cf0d2e642 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 14:50:07 -0400 Subject: [PATCH 1/2] Compose: Improve error messages when resource creation/updates fail. Signed-off-by: Daniel Nephin --- cli/command/stack/deploy_composefile.go | 41 ++++++++++--------------- cli/compose/loader/volume.go | 5 +-- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index 642867cba9..145da67daa 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -196,17 +196,18 @@ func createSecrets( for _, secretSpec := range secrets { secret, _, err := client.SecretInspectWithRaw(ctx, secretSpec.Name) - if err == nil { + switch { + case err == nil: // secret already exists, then we update that if err := client.SecretUpdate(ctx, secret.ID, secret.Meta.Version, secretSpec); err != nil { - return err + return errors.Wrapf(err, "failed to update secret %s", secretSpec.Name) } - } else if apiclient.IsErrSecretNotFound(err) { + case apiclient.IsErrSecretNotFound(err): // secret does not exist, then we create a new one. if _, err := client.SecretCreate(ctx, secretSpec); err != nil { - return err + return errors.Wrapf(err, "failed to create secret %s", secretSpec.Name) } - } else { + default: return err } } @@ -222,17 +223,18 @@ func createConfigs( for _, configSpec := range configs { config, _, err := client.ConfigInspectWithRaw(ctx, configSpec.Name) - if err == nil { + switch { + case err == nil: // config already exists, then we update that if err := client.ConfigUpdate(ctx, config.ID, config.Meta.Version, configSpec); err != nil { - return err + errors.Wrapf(err, "failed to update config %s", configSpec.Name) } - } else if apiclient.IsErrConfigNotFound(err) { + case apiclient.IsErrConfigNotFound(err): // config does not exist, then we create a new one. if _, err := client.ConfigCreate(ctx, configSpec); err != nil { - return err + errors.Wrapf(err, "failed to create config %s", configSpec.Name) } - } else { + default: return err } } @@ -269,10 +271,9 @@ func createNetworks( fmt.Fprintf(dockerCli.Out(), "Creating network %s\n", name) if _, err := client.NetworkCreate(ctx, name, createOpts); err != nil { - return err + return errors.Wrapf(err, "failed to create network %s", internalName) } } - return nil } @@ -312,19 +313,15 @@ func deployServices( if service, exists := existingServiceMap[name]; exists { fmt.Fprintf(out, "Updating service %s (id: %s)\n", name, service.ID) - updateOpts := types.ServiceUpdateOptions{} - if sendAuth { - updateOpts.EncodedRegistryAuth = encodedAuth - } response, err := apiClient.ServiceUpdate( ctx, service.ID, service.Version, serviceSpec, - updateOpts, + types.ServiceUpdateOptions{EncodedRegistryAuth: encodedAuth}, ) if err != nil { - return err + return errors.Wrapf(err, "failed to update service %s", name) } for _, warning := range response.Warnings { @@ -333,15 +330,11 @@ func deployServices( } else { fmt.Fprintf(out, "Creating service %s\n", name) - createOpts := types.ServiceCreateOptions{} - if sendAuth { - createOpts.EncodedRegistryAuth = encodedAuth - } + createOpts := types.ServiceCreateOptions{EncodedRegistryAuth: encodedAuth} if _, err := apiClient.ServiceCreate(ctx, serviceSpec, createOpts); err != nil { - return err + return errors.Wrapf(err, "failed to create service %s", name) } } } - return nil } diff --git a/cli/compose/loader/volume.go b/cli/compose/loader/volume.go index 0b8bc1b063..865d78ff7f 100644 --- a/cli/compose/loader/volume.go +++ b/cli/compose/loader/volume.go @@ -114,8 +114,5 @@ func isFilePath(source string) bool { // Windows absolute path first, next := utf8.DecodeRuneInString(source) - if unicode.IsLetter(first) && source[next] == ':' { - return true - } - return false + return unicode.IsLetter(first) && source[next] == ':' } From 97ebc194380a7cc7744df820a08244b1457a93cf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 15:22:09 -0400 Subject: [PATCH 2/2] Use a map instad of a switch/case for Compose transform. Signed-off-by: Daniel Nephin --- cli/compose/loader/loader.go | 75 ++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index f12a7f0eea..4cf06f66df 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -196,7 +196,7 @@ func transform(source map[string]interface{}, target interface{}) error { data := mapstructure.Metadata{} config := &mapstructure.DecoderConfig{ DecodeHook: mapstructure.ComposeDecodeHookFunc( - transformHook, + createTransformHook(), mapstructure.StringToTimeDurationHookFunc()), Result: target, Metadata: &data, @@ -208,46 +208,33 @@ func transform(source map[string]interface{}, target interface{}) error { return decoder.Decode(source) } -func transformHook( - source reflect.Type, - target reflect.Type, - data interface{}, -) (interface{}, error) { - switch target { - case reflect.TypeOf(types.External{}): - return transformExternal(data) - case reflect.TypeOf(types.HealthCheckTest{}): - return transformHealthCheckTest(data) - case reflect.TypeOf(types.ShellCommand{}): - return transformShellCommand(data) - case reflect.TypeOf(types.StringList{}): - return transformStringList(data) - case reflect.TypeOf(map[string]string{}): - return transformMapStringString(data) - case reflect.TypeOf(types.UlimitsConfig{}): - return transformUlimits(data) - case reflect.TypeOf(types.UnitBytes(0)): - return transformSize(data) - case reflect.TypeOf([]types.ServicePortConfig{}): - return transformServicePort(data) - case reflect.TypeOf(types.ServiceSecretConfig{}): - return transformStringSourceMap(data) - case reflect.TypeOf(types.ServiceConfigObjConfig{}): - return transformStringSourceMap(data) - case reflect.TypeOf(types.StringOrNumberList{}): - return transformStringOrNumberList(data) - case reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): - return transformServiceNetworkMap(data) - case reflect.TypeOf(types.MappingWithEquals{}): - return transformMappingOrList(data, "=", true), nil - case reflect.TypeOf(types.Labels{}): - return transformMappingOrList(data, "=", false), nil - case reflect.TypeOf(types.MappingWithColon{}): - return transformMappingOrList(data, ":", false), nil - case reflect.TypeOf(types.ServiceVolumeConfig{}): - return transformServiceVolumeConfig(data) +func createTransformHook() mapstructure.DecodeHookFuncType { + transforms := map[reflect.Type]func(interface{}) (interface{}, error){ + reflect.TypeOf(types.External{}): transformExternal, + reflect.TypeOf(types.HealthCheckTest{}): transformHealthCheckTest, + reflect.TypeOf(types.ShellCommand{}): transformShellCommand, + reflect.TypeOf(types.StringList{}): transformStringList, + reflect.TypeOf(map[string]string{}): transformMapStringString, + reflect.TypeOf(types.UlimitsConfig{}): transformUlimits, + reflect.TypeOf(types.UnitBytes(0)): transformSize, + reflect.TypeOf([]types.ServicePortConfig{}): transformServicePort, + reflect.TypeOf(types.ServiceSecretConfig{}): transformStringSourceMap, + reflect.TypeOf(types.ServiceConfigObjConfig{}): transformStringSourceMap, + reflect.TypeOf(types.StringOrNumberList{}): transformStringOrNumberList, + reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): transformServiceNetworkMap, + reflect.TypeOf(types.MappingWithEquals{}): transformMappingOrListFunc("=", true), + reflect.TypeOf(types.Labels{}): transformMappingOrListFunc("=", false), + reflect.TypeOf(types.MappingWithColon{}): transformMappingOrListFunc(":", false), + reflect.TypeOf(types.ServiceVolumeConfig{}): transformServiceVolumeConfig, + } + + return func(_ reflect.Type, target reflect.Type, data interface{}) (interface{}, error) { + transform, ok := transforms[target] + if !ok { + return data, nil + } + return transform(data) } - return data, nil } // keys needs to be converted to strings for jsonschema @@ -618,6 +605,12 @@ func transformStringList(data interface{}) (interface{}, error) { } } +func transformMappingOrListFunc(sep string, allowNil bool) func(interface{}) (interface{}, error) { + return func(data interface{}) (interface{}, error) { + return transformMappingOrList(data, sep, allowNil), nil + } +} + func transformMappingOrList(mappingOrList interface{}, sep string, allowNil bool) interface{} { switch value := mappingOrList.(type) { case map[string]interface{}: @@ -659,7 +652,7 @@ func transformHealthCheckTest(data interface{}) (interface{}, error) { } } -func transformSize(value interface{}) (int64, error) { +func transformSize(value interface{}) (interface{}, error) { switch value := value.(type) { case int: return int64(value), nil