Skip to content

Commit 6b53e85

Browse files
committed
Parallelize import download/read/unmarshal work while preserving ordered merge semantics
Replace global dependency download locking with per-cache locking and in-flight dedupe Add regression coverage for concurrent imports, merge order, disabled imports, and shared git subpaths Make URL dependency downloads context-aware and document download synchronization behavior Signed-off-by: Ryan Swanson <ryan.swanson@loft.sh>
1 parent cd129f6 commit 6b53e85

6 files changed

Lines changed: 579 additions & 122 deletions

File tree

e2e/tests/imports/imports.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,30 @@ var _ = DevSpaceDescribe("imports", func() {
139139
framework.ExpectError(err)
140140
})
141141

142+
ginkgo.It("should import multiple git subpaths from the same repository", func() {
143+
tempDir, err := framework.CopyToTempDir("tests/imports/testdata/git-subpaths")
144+
framework.ExpectNoError(err)
145+
defer framework.CleanupTempDir(initialDir, tempDir)
146+
147+
configBuffer := &bytes.Buffer{}
148+
printCmd := &cmd.PrintCmd{
149+
GlobalFlags: &flags.GlobalFlags{},
150+
Out: configBuffer,
151+
SkipInfo: true,
152+
}
153+
154+
err = printCmd.Run(f)
155+
framework.ExpectNoError(err)
156+
157+
latestConfig := &latest.Config{}
158+
err = yaml.Unmarshal(configBuffer.Bytes(), latestConfig)
159+
framework.ExpectNoError(err)
160+
161+
framework.ExpectEqual(latestConfig.Pipelines["pipeline-a"].Run, "echo \"pipeline-a\"")
162+
framework.ExpectEqual(latestConfig.Pipelines["pipeline-b"].Run, "echo \"pipeline-b\"")
163+
framework.ExpectEqual(latestConfig.Pipelines["pipeline-c"].Run, "echo \"pipeline-c\"")
164+
})
165+
142166
ginkgo.It("should import correctly with localRegistry", func() {
143167
tempDir, err := framework.CopyToTempDir("tests/imports/testdata/localregistry")
144168
framework.ExpectNoError(err)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
version: v2beta1
2+
name: git-subpath-imports
3+
4+
imports:
5+
- git: https://github.com/loft-sh/e2e-test-dependency.git
6+
branch: main
7+
subPath: subA
8+
- git: https://github.com/loft-sh/e2e-test-dependency.git
9+
branch: main
10+
subPath: subB
11+
- git: https://github.com/loft-sh/e2e-test-dependency.git
12+
branch: main
13+
subPath: subC

pkg/devspace/config/loader/imports.go

Lines changed: 80 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88

99
"github.com/loft-sh/devspace/pkg/devspace/config/loader/variable"
1010
"github.com/loft-sh/devspace/pkg/devspace/config/versions"
11+
"github.com/loft-sh/devspace/pkg/devspace/config/versions/latest"
1112
"github.com/loft-sh/devspace/pkg/devspace/config/versions/util"
1213
dependencyutil "github.com/loft-sh/devspace/pkg/devspace/dependency/util"
1314
"github.com/loft-sh/devspace/pkg/util/log"
1415
"github.com/loft-sh/devspace/pkg/util/yamlutil"
1516
"github.com/pkg/errors"
17+
"golang.org/x/sync/errgroup"
1618
)
1719

1820
var ImportSections = []string{
@@ -31,6 +33,14 @@ var ImportSections = []string{
3133
"localRegistry",
3234
}
3335

36+
const maxConcurrentImportDownloads = 8
37+
38+
type resolvedImport struct {
39+
ConfigPath string
40+
Data map[string]interface{}
41+
Disabled bool
42+
}
43+
3444
func ResolveImports(ctx context.Context, resolver variable.Resolver, basePath string, rawData map[string]interface{}, log log.Logger) (map[string]interface{}, error) {
3545
// initially reload variables
3646
err := reloadVariables(resolver, rawData, log)
@@ -59,40 +69,26 @@ func ResolveImports(ctx context.Context, resolver variable.Resolver, basePath st
5969
return nil, err
6070
}
6171

72+
resolvedImports, err := loadImports(ctx, basePath, imports.Imports, version, log)
73+
if err != nil {
74+
return nil, err
75+
}
76+
6277
mergedMap := map[string]interface{}{}
6378
err = util.Convert(rawData, mergedMap)
6479
if err != nil {
6580
return nil, err
6681
}
6782

6883
// load imports
69-
for _, i := range imports.Imports {
70-
if i.Enabled != nil && !*i.Enabled {
84+
for _, resolvedImport := range resolvedImports {
85+
if resolvedImport.Disabled {
7186
continue
87+
} else if resolvedImport.Data == nil {
88+
return nil, errors.Errorf("resolved import is missing data")
7289
}
7390

74-
configPath, err := dependencyutil.DownloadDependency(ctx, basePath, &i.SourceConfig, log)
75-
if err != nil {
76-
return nil, errors.Wrap(err, "resolve import")
77-
}
78-
79-
fileContent, err := os.ReadFile(configPath)
80-
if err != nil {
81-
return nil, errors.Wrap(err, "read import config")
82-
}
83-
84-
importData := map[string]interface{}{}
85-
err = yamlutil.Unmarshal(fileContent, &importData)
86-
if err != nil {
87-
return nil, err
88-
}
89-
90-
configVersion, ok := importData["version"].(string)
91-
if !ok {
92-
return nil, fmt.Errorf("version is missing in import config %s", configPath)
93-
} else if version != configVersion {
94-
return nil, fmt.Errorf("import mismatch %s != %s. Import %s has different version than currently used devspace.yaml, please make sure the versions match between an import and the devspace.yaml using it", version, configVersion, configPath)
95-
}
91+
importData := resolvedImport.Data
9692

9793
// merge sections
9894
for _, section := range ImportSections {
@@ -130,16 +126,71 @@ func ResolveImports(ctx context.Context, resolver variable.Resolver, basePath st
130126
// resolve the import imports
131127
if importData["imports"] != nil {
132128
mergedMap["imports"] = importData["imports"]
129+
130+
// resolve imports
131+
mergedMap, err = ResolveImports(ctx, resolver, filepath.Dir(resolvedImport.ConfigPath), mergedMap, log)
132+
if err != nil {
133+
return nil, err
134+
}
133135
} else {
134136
delete(mergedMap, "imports")
135-
}
136137

137-
// resolve imports
138-
mergedMap, err = ResolveImports(ctx, resolver, filepath.Dir(configPath), mergedMap, log)
139-
if err != nil {
140-
return nil, err
138+
err = reloadVariables(resolver, mergedMap, log)
139+
if err != nil {
140+
return nil, err
141+
}
141142
}
142143
}
143144

144145
return mergedMap, nil
145146
}
147+
148+
func loadImports(ctx context.Context, basePath string, imports []latest.Import, version string, log log.Logger) ([]resolvedImport, error) {
149+
resolvedImports := make([]resolvedImport, len(imports))
150+
151+
eg, ctx := errgroup.WithContext(ctx)
152+
eg.SetLimit(maxConcurrentImportDownloads)
153+
for index := range imports {
154+
importConfig := imports[index]
155+
if importConfig.Enabled != nil && !*importConfig.Enabled {
156+
resolvedImports[index] = resolvedImport{Disabled: true}
157+
continue
158+
}
159+
160+
eg.Go(func() error {
161+
configPath, err := dependencyutil.DownloadDependency(ctx, basePath, &importConfig.SourceConfig, log)
162+
if err != nil {
163+
return errors.Wrap(err, "resolve import")
164+
}
165+
166+
fileContent, err := os.ReadFile(configPath)
167+
if err != nil {
168+
return errors.Wrap(err, "read import config")
169+
}
170+
171+
importData := map[string]interface{}{}
172+
err = yamlutil.Unmarshal(fileContent, &importData)
173+
if err != nil {
174+
return err
175+
}
176+
177+
configVersion, ok := importData["version"].(string)
178+
if !ok {
179+
return fmt.Errorf("version is missing in import config %s", configPath)
180+
} else if version != configVersion {
181+
return fmt.Errorf("import mismatch %s != %s. Import %s has different version than currently used devspace.yaml, please make sure the versions match between an import and the devspace.yaml using it", version, configVersion, configPath)
182+
}
183+
184+
resolvedImports[index] = resolvedImport{
185+
ConfigPath: configPath,
186+
Data: importData,
187+
}
188+
return nil
189+
})
190+
}
191+
if err := eg.Wait(); err != nil {
192+
return nil, err
193+
}
194+
195+
return resolvedImports, nil
196+
}

0 commit comments

Comments
 (0)