-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: new exec command to execute a custom action #8696
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
feat: new exec command to execute a custom action #8696
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8696 +/- ##
==========================================
- Coverage 70.48% 64.44% -6.05%
==========================================
Files 515 617 +102
Lines 23150 31168 +8018
==========================================
+ Hits 16317 20085 +3768
- Misses 5776 9577 +3801
- Partials 1057 1506 +449
... and 406 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
438be6d
to
1c28e4c
Compare
e0e5df0
to
3a9e52e
Compare
cmd/skaffold/app/cmd/exec.go
Outdated
return withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error { | ||
buildArtifacts, err := getBuildArtifactsAndSetTagsForAction(configs, r.ApplyDefaultRepo, args[0]) | ||
if err != nil { | ||
tips.PrintUseRunVsDeploy(out) |
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.
this tip message "run vs deploy" is likely not correct for the exec command. Should likely be changed to a more appropriate err msg
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.
Done! I added a new tip message explaining how to use images from previous builds. Thanks!
nit: I believe everytime 2+ actions are run together there will be a warning in stdout that liely shouldn't be there:
I think this is related to the docker purge issue we mentioned in the past, forget the status of that Doesn't have to be addressed here, just wanted to point out this UX issue |
Just tried it out, works great! |
Yes! That's exactly what's happening. I have a branch with the changes to improve this for the custom actions case (renzodavid9/skaffold@ca-feature-8628...renzodavid9:skaffold:ca-issue-prune-err) where I split the docker client logic in another method so we don't run prune. This is to improve the custom actions case, but for a long term solution the idea is to use docker labels to know what to prune. My idea is to create the PR with the changes once this one gets merged. |
…to the last one always
bc96425
to
da83ca4
Compare
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.
LGTM!
Fixes: #8628
Description
This PR adds the logic to include the new
exec
command to execute a Custom Action together with the new v2 events to communicate the state of anexec
run.