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

Add support for transitive dependencies (needs). #1983

Merged
merged 1 commit into from
Oct 20, 2021
Merged

Add support for transitive dependencies (needs). #1983

merged 1 commit into from
Oct 20, 2021

Conversation

paichinger
Copy link
Contributor

Problem

When using selectors in helmfile one can use the --include-needs parameter to also install the dependencies of the selected release(s). However, this doesn't work for transitive needs, only the direct needs of the specified releases will be included. The problem is described in more detail with an example here:
#1915

Solution

After conferring with @mumoshu we decided to introduce a new parameter called --include-transitive-needs, which will also include transitive needs. We deliberately decided to not change the behavior of --include-needs (even if including transitives might be the expected behavior for most users) to ensure backwards compatibility.

Notes

--include-transitive-needs will override any potential exclusions done by selectors or conditions. So even if you explicitly exclude a release via a selector it will still be part of the deployment in case it is a direct or transitive need of any of the specified releases.

Fixes #1915
Closes #1915

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your contribution! ☺️

I've left a few comments for potential problems- it would be awesome if you could confirm those and submit a follow-up PR! Thanks.

@mumoshu mumoshu merged commit 77e6268 into roboll:master Oct 20, 2021
@paichinger paichinger deleted the transitive-dependencies branch October 21, 2021 08:45
@dudicoco
Copy link
Contributor

dudicoco commented Nov 4, 2021

Hi @pjotre86 and @mumoshu,

As far as I know, needs has been broken for a few months now, see #1881.

Does this PR fix this bug?

@paichinger
Copy link
Contributor Author

paichinger commented Nov 5, 2021

Hey @Hey @dudicoco !
Not sure, I never ran into the problem with needs described in #1881.

@dudicoco
Copy link
Contributor

dudicoco commented Nov 7, 2021

Hi @pjotre86,

Did you try to recreate the issue per the instructions in #1881?

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.

Include transitive dependencies
3 participants