feat(hooks): invoke customActions from deploy.*.hooks.before/after#10067
Draft
bogdannazarenko wants to merge 3 commits intoGoogleContainerTools:mainfrom
Draft
feat(hooks): invoke customActions from deploy.*.hooks.before/after#10067bogdannazarenko wants to merge 3 commits intoGoogleContainerTools:mainfrom
bogdannazarenko wants to merge 3 commits intoGoogleContainerTools:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces 'Action hooks,' allowing users to reference custom actions as deployment lifecycle hooks. The implementation includes the ActionInvoker interface to prevent package cycles, updated validation to ensure action names exist, and new documentation and examples. A review comment points out that adding fields to the v4beta14 schema might violate versioning policies if that version is already frozen, suggesting a move to a new schema version.
6453e17 to
c8d0251
Compare
Contributor
Author
|
Waiting on maintainers to cut new schema version on main branch |
Adds a new ActionHook to the deploy lifecycle-hook union so users can
reference an existing customActions entry from
deploy.*.hooks.before / .after instead of re-implementing the
containerized task as a host hook with ad-hoc docker run arguments:
deploy:
kubectl: { ... }
hooks:
before:
- action: { name: pre-deploy-check }
after:
- action: { name: migrate-db }
customActions:
- name: migrate-db
containers: [{ image: myorg/migrator:latest, ... }]
Implementation
* schema: ActionHook{Name} joins HostHook and ContainerHook as a
third oneOf=deploy_hook union member on DeployHookItem.
* hooks: new package-level ActionInvoker interface plus a
SetDefaultActionInvoker setter so deploy hook runners can dispatch
without an import cycle back into pkg/skaffold/actions. The
invoker is registered once in runner.New immediately after
GetActionsRunner succeeds, mirroring how
SetupStaticEnvOptions is wired.
* runner: small actionsRunnerInvoker adapter satisfies the new
interface by delegating to ActionsRunner.Exec with nil artifact
slices (hooks do not themselves build).
* deploy.go: the run() dispatch loop recognises ActionHook and
calls runActionHook, which returns a helpful error if no invoker
is wired (e.g. from tests that stub hooks out).
CloudRunDeployHooks still accepts only []HostHook and is therefore
unaffected; wiring ActionHook into CloudRun is a follow-up once the
CloudRun deployer grows its own union type.
Unit coverage for the new ActionHook dispatch path:
- happy path fires pre then post hooks in declaration order
- first failure aborts the remaining hooks and surfaces both the
phase and the action name in the wrapped error
- a config with ActionHook but no registered invoker returns the
'no custom-actions runner available' sentinel
Also wires a new validateActionHookRefs pass into
ProcessWithRunContext so configs that reference an unknown custom
action fail at load time with a targeted message instead of deep
inside the deploy. Covers the three kubernetes-family deployers
(LegacyHelmDeploy, KubectlDeploy, KptDeploy); CloudRunDeployHooks
is intentionally still HostHook-only in this PR.
Adds a new 'Action hooks' section to docs-v2/docs/lifecycle-hooks.md describing the action: union member, how it relates to customActions, which deployers support it (kubectl, helm, kpt), and that it shares a runtime with skaffold exec (so a single action definition is runnable standalone or wrapped as a deploy hook). Ships a runnable examples/hooks-action/ fixture (mirrored under integration/examples/) with pre-deploy-check + post-deploy-smoke actions and a minimal busybox Pod manifest so maintainers can smoke test the new schema end-to-end.
c8d0251 to
d7e5025
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3: Cloud Deploy custom-target parity — action hooks
Final phase of the three-part series started in #10065 and continued in #10066. Neither earlier PR is required to land first; this branch is cut from
main.What's in
New
action:hook union member onDeployHookItem:Reuses the existing
customActionsruntime — a single action definition is runnable standalone viaskaffold execor wrapped as a deploy hook, so users aren't forced to choose between the two shapes.ActionInvoker interface in
pkg/skaffold/hooks. A tinySetDefaultActionInvokersetter is wired once inrunner.Newright afterGetActionsRunnersucceeds. The hooks package stays free of anypkg/skaffold/actionsimport, preserving the existing dependency tree.actionsRunnerInvokeradapter inpkg/skaffold/runnersatisfieshooks.ActionInvokerby delegating to the runner'sExecwith nil artifact slices — hooks are not themselves build producers.Load-time validation (
validateActionHookRefs): unknown action references fail at config parse rather than deep inside a deploy. Missingnamefields are flagged too. Covers kubectl, helm, and kpt deployers.Unit tests (
pkg/skaffold/hooks/action_test.go): ordered dispatch, first-failure aborts remaining hooks, missing-invoker sentinel.Docs + runnable example: new 'Action hooks' section in
docs-v2/docs/lifecycle-hooks.md;examples/hooks-action/(mirrored underintegration/examples/) exercises the schema end-to-end with pre-deploy + post-deploy busybox actions.What's deliberately out of scope
[]HostHookonly. Wiringaction:support into the CloudRun deployer would require promoting its hook list to a union type; saving that for a dedicated follow-up to keep this diff focused.ActionHooksupport — not requested.latestatskaffold/v4beta14per the maintainer-drivenhack/new-version.shworkflow. If you'd prefer the freeze done here, happy to add a commit.Test results
go build ./...— cleango test -short -race ./pkg/skaffold/hooks/... ./pkg/skaffold/schema/validation/... ./pkg/skaffold/runner/... ./pkg/skaffold/deploy/... ./cmd/...— green(the pre-existing
TestNewEnvClient/DOCKER_HOST_not_set,_output_invalid_hostandTestNewRunnerfailures reproduce onmainand are unrelated)./hack/check-samples.sh— passes