Merge pull request #145 from dnephin/improve-compose-secrets-errors

Compose: Improve error messages when resource create/update fails

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{}
+			createOpts := types.ServiceCreateOptions{EncodedRegistryAuth: encodedAuth}
-			if sendAuth {
-				createOpts.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
 }

cli/compose/loader/loader.go

@@ -195,7 +195,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,
@@ -207,46 +207,33 @@ func transform(source map[string]interface{}, target interface{}) error {
 	return decoder.Decode(source)
 }
 
-func transformHook(
+func createTransformHook() mapstructure.DecodeHookFuncType {
-	source reflect.Type,
+	transforms := map[reflect.Type]func(interface{}) (interface{}, error){
-	target reflect.Type,
+		reflect.TypeOf(types.External{}):                         transformExternal,
-	data interface{},
+		reflect.TypeOf(types.HealthCheckTest{}):                  transformHealthCheckTest,
-) (interface{}, error) {
+		reflect.TypeOf(types.ShellCommand{}):                     transformShellCommand,
-	switch target {
+		reflect.TypeOf(types.StringList{}):                       transformStringList,
-	case reflect.TypeOf(types.External{}):
+		reflect.TypeOf(map[string]string{}):                      transformMapStringString,
-		return transformExternal(data)
+		reflect.TypeOf(types.UlimitsConfig{}):                    transformUlimits,
-	case reflect.TypeOf(types.HealthCheckTest{}):
+		reflect.TypeOf(types.UnitBytes(0)):                       transformSize,
-		return transformHealthCheckTest(data)
+		reflect.TypeOf([]types.ServicePortConfig{}):              transformServicePort,
-	case reflect.TypeOf(types.ShellCommand{}):
+		reflect.TypeOf(types.ServiceSecretConfig{}):              transformStringSourceMap,
-		return transformShellCommand(data)
+		reflect.TypeOf(types.ServiceConfigObjConfig{}):           transformStringSourceMap,
-	case reflect.TypeOf(types.StringList{}):
+		reflect.TypeOf(types.StringOrNumberList{}):               transformStringOrNumberList,
-		return transformStringList(data)
+		reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): transformServiceNetworkMap,
-	case reflect.TypeOf(map[string]string{}):
+		reflect.TypeOf(types.MappingWithEquals{}):                transformMappingOrListFunc("=", true),
-		return transformMapStringString(data)
+		reflect.TypeOf(types.Labels{}):                           transformMappingOrListFunc("=", false),
-	case reflect.TypeOf(types.UlimitsConfig{}):
+		reflect.TypeOf(types.MappingWithColon{}):                 transformMappingOrListFunc(":", false),
-		return transformUlimits(data)
+		reflect.TypeOf(types.ServiceVolumeConfig{}):              transformServiceVolumeConfig,
-	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)
 	}
+
+	return func(_ reflect.Type, target reflect.Type, data interface{}) (interface{}, error) {
+		transform, ok := transforms[target]
+		if !ok {
 			return data, nil
+		}
+		return transform(data)
+	}
 }
 
 // keys needs to be converted to strings for jsonschema
@@ -617,6 +604,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{}:
@@ -658,7 +651,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

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 unicode.IsLetter(first) && source[next] == ':'
-		return true
-	}
-	return false
 }