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(core): inherit Build action mode from dependant Deploy action #5589

Merged
merged 6 commits into from
Dec 22, 2023

Conversation

stefreak
Copy link
Member

@stefreak stefreak commented Dec 21, 2023

What this PR does / why we need it:
Override the Build action mode with the Deploy action mode, if it depends on the build.

This enables using ${this.mode} in the build action as a proxy to the
Deploy action mode, simplifying configuration for many users.

In previous versions of Garden all actions went into sync Mode when
users ran the command garden deploy --sync without explicitly listing
Deploy actions due to the fact that we defaulted to the * Minimatch
pattern that also matched build actions.

This commit ensures that if you depend on a Build from a Deploy action,
and that Deploy action is in sync mode, the Build action inherits the
mode.

If multiple Deploy actions depend on the same build, and have different
modes, the non-default mode wins. If multiple Deploy actions use
different non-default modes, we print a warning.

In the future, we can make this even more reliable by injecting
different invariants of build actions into the stack graph for each
Deploy action, if the build action is used by multiple Deploy actions
and they are in different modes.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

thsig and others added 3 commits December 21, 2023 16:45
Override the Build action mode with the Deploy action mode, if it depends on the build.

This enables using ${this.mode} in the build action as a proxy to the
Deploy action mode, simplifying configuration for many users.

In previous versions of Garden all actions went into `sync` Mode when
users ran the command `garden deploy --sync` without explicitly listing
build actions due to the fact that we defaulted to the `*` Minimatch
pattern that also matched build actions.

This commit ensures that if you depend on a Build from a Deploy action,
and that Deploy action is in `sync` mode, the Build action inherits the
mode.

If multiple Deploy actions depend on the same build, and have different
modes, the non-default mode wins. If multiple Deploy actions use
different non-default modes, we print a warning.

In the future, we can make this even more reliable by injecting
different invariants of build actions into the stack graph for each
Deploy action, if the build action is used by multiple Deploy actions
and they are in different modes.
When running `garden deploy --sync` without specifying certain actions
to be synced, previously all actions, even builds and runs went into
sync mode due to the minimatch pattern `*` matching all actions
regardless of Kind.
@stefreak stefreak requested a review from thsig December 21, 2023 16:25
@stefreak stefreak marked this pull request as ready for review December 21, 2023 16:25
@stefreak stefreak changed the title Build mode override fix(core): inherit Build action mode from dependant Deploy action Dec 21, 2023
thsig
thsig previously approved these changes Dec 21, 2023
Copy link
Collaborator

@thsig thsig left a comment

Choose a reason for hiding this comment

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

Looks good to me! Added a couple of text-related comments there.

We'll take a more proper look at all this in the new year.

core/src/graph/actions.ts Outdated Show resolved Hide resolved
core/test/unit/src/actions/action-configs-to-graph.ts Outdated Show resolved Hide resolved
@stefreak stefreak force-pushed the build-mode-override branch from 2b2885d to 0bf1b08 Compare December 21, 2023 16:34
Co-authored-by: Thorarinn Sigurdsson <[email protected]>
@stefreak stefreak force-pushed the build-mode-override branch from ce26f40 to bb273e8 Compare December 21, 2023 16:38
@stefreak stefreak mentioned this pull request Dec 21, 2023
@stefreak stefreak added this pull request to the merge queue Dec 22, 2023
Merged via the queue into main with commit e050564 Dec 22, 2023
46 checks passed
@stefreak stefreak deleted the build-mode-override branch December 22, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants