Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func withFallbackConfig(ctx context.Context, out io.Writer, opts config.Skaffold
if errors.As(err, &e) && e.StatusCode() == proto.StatusCode_CONFIG_FILE_NOT_FOUND_ERR {
if (opts.AutoCreateConfig || opts.AutoInit) && initializer.ValidCmd(opts) {
output.Default.Fprintf(out, "Skaffold config file %s not found - Trying to create one for you...\n", opts.ConfigurationFile)
config, err := initializer.Transparent(context.Background(), out, initConfig.Config{Opts: opts, EnableManifestGeneration: opts.AutoInit})
config, err := initializer.Transparent(ctx, out, initConfig.Config{Opts: opts, EnableManifestGeneration: opts.AutoInit})
if err != nil {
return nil, fmt.Errorf("unable to generate skaffold config file automatically - try running `skaffold init`: %w", err)
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/skaffold/initializer/analyze/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ func NewAnalyzer(c config.Config) *ProjectAnalysis {
// Analyze recursively walks a directory and notifies the analyzers of files and enterDir and exitDir events
// at the end of the analyze function the analysis struct's analyzers should contain the state that we can
// use to do further computation.
func (a *ProjectAnalysis) Analyze(dir string) error {
func (a *ProjectAnalysis) Analyze(ctx context.Context, dir string) error {
if err := ctx.Err(); err != nil {
return err
}

for _, analyzer := range a.analyzers() {
analyzer.enterDir(dir)
}
Expand Down Expand Up @@ -180,15 +184,18 @@ func (a *ProjectAnalysis) Analyze(dir string) error {
// to make skaffold.yaml more portable across OS-es we should always generate /-delimited filePaths
filePath = strings.ReplaceAll(filePath, string(os.PathSeparator), "/")
for _, analyzer := range a.analyzers() {
if err := analyzer.analyzeFile(context.Background(), filePath); err != nil {
if err := ctx.Err(); err != nil {
return err
}
if err := analyzer.analyzeFile(ctx, filePath); err != nil {
return err
}
}
}

// Recurse into subdirectories
for _, subdir := range subdirectories {
if err := a.Analyze(subdir); err != nil {
if err := a.Analyze(ctx, subdir); err != nil {
return err
}
}
Comment on lines 197 to 201
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve responsiveness to cancellation, it's good practice to check if the context is done inside loops that might have many iterations. While the recursive call to Analyze checks the context at the beginning, adding a check here before the call will make cancellation more immediate if there are many subdirectories to process.

for _, subdir := range subdirectories {
	if err := ctx.Err(); err != nil {
		return err
	}
	if err := a.Analyze(ctx, subdir); err != nil {
		return err
	}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/analyze/analyze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ deploy:
t.Override(&jib.Validate, fakeValidateJibConfig)

a := NewAnalyzer(test.config)
err := a.Analyze(".")
err := a.Analyze(t.Context(), ".")

t.CheckError(test.shouldErr, err)
if test.shouldErr {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ spec:
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().WriteFiles(test.filesWithContents).Chdir()
a := analyze.NewAnalyzer(config)
err := a.Analyze(".")
err := a.Analyze(t.Context(), ".")
t.CheckError(test.shouldErr, err)
d := deploy.NewInitializer(a.HelmChartInfo(), config)
dc := d.DeployConfig()
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/initializer/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func DoInit(ctx context.Context, out io.Writer, c config.Config) error {
}
}

a, err := AnalyzeProject(c)
a, err := AnalyzeProject(ctx, c)
if err != nil {
return err
}
Expand All @@ -57,9 +57,9 @@ func DoInit(ctx context.Context, out io.Writer, c config.Config) error {
}

// AnalyzeProject scans the project directory for files and keeps track of what types of files it finds (builders, k8s manifests, etc.).
func AnalyzeProject(c config.Config) (*analyze.ProjectAnalysis, error) {
func AnalyzeProject(ctx context.Context, c config.Config) (*analyze.ProjectAnalysis, error) {
a := analyze.NewAnalyzer(c)
if err := a.Analyze("."); err != nil {
if err := a.Analyze(ctx, "."); err != nil {
return nil, err
}
return a, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/transparent.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func Transparent(ctx context.Context, out io.Writer, c initConfig.Config) (*late
}
}

a, err := AnalyzeProject(c)
a, err := AnalyzeProject(ctx, c)
if err != nil {
return nil, err
}
Expand Down