Skip to content
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

fix: verify hangs if event-logs-file does not exist (#7613) #8961

Merged

Conversation

Darien-Lin
Copy link
Contributor

@Darien-Lin Darien-Lin commented Jul 19, 2023

Fixes: #7613

Description

When running skaffold verify with rpc-port and --event-log-file, if the event-log-file references a path to a file where the directory doesn't exist, the command hangs indefinitely. The changes I am proposing ensure that the directories to event-log-file exists before attempting to create the file and dump the logs. The relevant unit tests are also adjusted with the assumption that the directory does not exist.

In addition, SaveLastLog seems to follow the same issue, which is also fixed in this PR.

0700 is used since 'execute' permission is needed on the directories. Execute permissions on directory are so that files can be accessed in those nested directories.

@Darien-Lin
Copy link
Contributor Author

hm, looking into the permissions error. Weird that the file cannot be created in the directory that we have created.

@Darien-Lin Darien-Lin force-pushed the verify-hangs-with-event-logs branch from 2148144 to 506953d Compare July 19, 2023 18:52
@Darien-Lin
Copy link
Contributor Author

@ericzzzzzzz ericzzzzzzz added the kokoro:run runs the kokoro jobs on a PR label Jul 19, 2023
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jul 19, 2023
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #8961 (506953d) into main (290280e) will decrease coverage by 6.86%.
The diff coverage is 48.91%.

@@            Coverage Diff             @@
##             main    #8961      +/-   ##
==========================================
- Coverage   70.48%   63.62%   -6.86%     
==========================================
  Files         515      624     +109     
  Lines       23150    31932    +8782     
==========================================
+ Hits        16317    20317    +4000     
- Misses       5776    10086    +4310     
- Partials     1057     1529     +472     
Files Changed Coverage Δ
cmd/skaffold/app/cmd/completion.go 13.04% <0.00%> (-1.25%) ⬇️
cmd/skaffold/app/cmd/config/list.go 65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go 88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go 54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go 100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/generate_pipeline.go 60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go 65.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go 66.66% <ø> (ø)
... and 40 more

... and 416 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ericzzzzzzz ericzzzzzzz merged commit db79008 into GoogleContainerTools:main Jul 19, 2023
15 checks passed
@Darien-Lin Darien-Lin deleted the verify-hangs-with-event-logs branch July 20, 2023 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skaffold verify hangs when using --event-log-file w/ dir that doesn't exist
3 participants