-
Notifications
You must be signed in to change notification settings - Fork 2.1k
cli/compose: assorted fixes and cleanups #6803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
db28780
09cf89e
78458e1
6e13930
42a2111
b35a2d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,10 @@ | |
| package loader | ||
|
|
||
| import ( | ||
| "cmp" | ||
| "fmt" | ||
| "reflect" | ||
| "slices" | ||
| "sort" | ||
|
|
||
| "dario.cat/mergo" | ||
|
|
@@ -52,10 +54,10 @@ func merge(configs []*types.Config) (*types.Config, error) { | |
| } | ||
|
|
||
| func mergeServices(base, override []types.ServiceConfig) ([]types.ServiceConfig, error) { | ||
| baseServices := mapByName(base) | ||
| overrideServices := mapByName(override) | ||
| specials := &specials{ | ||
| m: map[reflect.Type]func(dst, src reflect.Value) error{ | ||
| mergeOpts := []func(*mergo.Config){ | ||
| mergo.WithAppendSlice, | ||
| mergo.WithOverride, | ||
| mergo.WithTransformers(&specials{m: map[reflect.Type]func(dst, src reflect.Value) error{ | ||
| reflect.PointerTo(reflect.TypeFor[types.LoggingConfig]()): safelyMerge(mergeLoggingConfig), | ||
| reflect.TypeFor[[]types.ServicePortConfig](): mergeSlice(toServicePortConfigsMap, toServicePortConfigsSlice), | ||
| reflect.TypeFor[[]types.ServiceSecretConfig](): mergeSlice(toServiceSecretConfigsMap, toServiceSecretConfigsSlice), | ||
|
|
@@ -65,23 +67,34 @@ func mergeServices(base, override []types.ServiceConfig) ([]types.ServiceConfig, | |
| reflect.TypeFor[types.ShellCommand](): mergeShellCommand, | ||
| reflect.PointerTo(reflect.TypeFor[types.ServiceNetworkConfig]()): mergeServiceNetworkConfig, | ||
| reflect.PointerTo(reflect.TypeFor[uint64]()): mergeUint64, | ||
| }, | ||
| }}), | ||
| } | ||
| for name, overrideService := range overrideServices { | ||
| if baseService, ok := baseServices[name]; ok { | ||
| if err := mergo.Merge(&baseService, &overrideService, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(specials)); err != nil { | ||
| return base, fmt.Errorf("cannot merge service %s: %w", name, err) | ||
|
|
||
| baseServices := make(map[string]types.ServiceConfig, len(base)) | ||
| for _, s := range base { | ||
| baseServices[s.Name] = s | ||
| } | ||
|
|
||
| for _, overrideService := range override { | ||
| if baseService, ok := baseServices[overrideService.Name]; ok { | ||
| if err := mergo.Merge(&baseService, &overrideService, mergeOpts...); err != nil { | ||
| return base, fmt.Errorf("cannot merge service %s: %w", overrideService.Name, err) | ||
| } | ||
| baseServices[name] = baseService | ||
| baseServices[overrideService.Name] = baseService | ||
| continue | ||
| } | ||
| baseServices[name] = overrideService | ||
| baseServices[overrideService.Name] = overrideService | ||
| } | ||
|
Comment on lines
+78
to
87
|
||
|
|
||
| services := make([]types.ServiceConfig, 0, len(baseServices)) | ||
| for _, baseService := range baseServices { | ||
| services = append(services, baseService) | ||
| } | ||
| sort.Slice(services, func(i, j int) bool { return services[i].Name < services[j].Name }) | ||
|
|
||
| slices.SortFunc(services, func(a, b types.ServiceConfig) int { | ||
| return cmp.Compare(a.Name, b.Name) | ||
| }) | ||
|
|
||
| return services, nil | ||
| } | ||
|
|
||
|
|
@@ -217,11 +230,13 @@ func sliceToMap(tomap tomapFn, v reflect.Value) (map[any]any, error) { | |
| } | ||
|
|
||
| func mergeLoggingConfig(dst, src reflect.Value) error { | ||
| dstDriver := dst.Elem().FieldByName("Driver").String() | ||
| srcDriver := src.Elem().FieldByName("Driver").String() | ||
|
|
||
| // Same driver, merging options | ||
| if getLoggingDriver(dst.Elem()) == getLoggingDriver(src.Elem()) || | ||
| getLoggingDriver(dst.Elem()) == "" || getLoggingDriver(src.Elem()) == "" { | ||
| if getLoggingDriver(dst.Elem()) == "" { | ||
| dst.Elem().FieldByName("Driver").SetString(getLoggingDriver(src.Elem())) | ||
| if dstDriver == srcDriver || dstDriver == "" || srcDriver == "" { | ||
| if dstDriver == "" { | ||
| dst.Elem().FieldByName("Driver").SetString(srcDriver) | ||
| } | ||
| dstOptions := dst.Elem().FieldByName("Options").Interface().(map[string]string) | ||
| srcOptions := src.Elem().FieldByName("Options").Interface().(map[string]string) | ||
|
|
@@ -270,18 +285,6 @@ func mergeUint64(dst, src reflect.Value) error { | |
| return nil | ||
| } | ||
|
|
||
| func getLoggingDriver(v reflect.Value) string { | ||
| return v.FieldByName("Driver").String() | ||
| } | ||
|
|
||
| func mapByName(services []types.ServiceConfig) map[string]types.ServiceConfig { | ||
| m := map[string]types.ServiceConfig{} | ||
| for _, service := range services { | ||
| m[service.Name] = service | ||
| } | ||
| return m | ||
| } | ||
|
|
||
| func mergeVolumes(base, override map[string]types.VolumeConfig) (map[string]types.VolumeConfig, error) { | ||
| err := mergo.Map(&base, &override, mergo.WithOverride) | ||
| return base, err | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convertEndpointSpecnow sorts ports usingcompareSwarmPortConfigto handle ties beyondPublishedPort, but the existingTestConvertEndpointSpeconly covers distinctPublishedPortvalues. Add a test case with multiple ports sharing the samePublishedPortbut different protocol/target/mode to prevent regressions in the new ordering logic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that will come with the linked PR; it's how I stumbled on it