[BUG] needs dependency graph is broken #1881
Comments
|
I noticed the very same behaviour, it seemed that the |
|
Running into this as well. |
|
yep, I've got a pretty simple test set up on my end, and |
|
@mumoshu can you please address this? |
|
Sorry everyone I've been occupied by other matters these days so I won't be able to work on this shortly. It would be great if anyone could stand up and submit a PR for this. I'll gladly review any PR related to this ASAP. |
|
Maybe it's caused by this https://github.com/roboll/helmfile/releases/tag/v0.139.2? it makes default behaviour to ignore needs. If it's so it's possible to enable needs back with |
|
can be enabled as a default with |
|
@mumoshu can the whole |
|
@dudicoco Does it have something to do with you're missing |
|
@mumoshu we have one helmfile.yaml file which loads all of the releases and templates them, during the templating the namespace is added, so this is not the issue here.
|
|
@dm3ch Well would you mind clarifying what are you expecting from |
|
@dudicoco Gotcha! Thanks. Do you have any blocker that prevents you from fixing Helmfile in the main branch(as I currently lack spare time to work on Helmfile) OR sticking with |
|
@mumoshu can you please elaborate on what you mean by fixing Helmfile in the main branch? On a side note, perhaps additional maintainers should be added to the repository? There is a high traffic on this repository and I understand the difficulty in maintaining it all by yourself :) Thanks |
@dudicoco I mean "what is preventing you from submitting a pull request yourself" 😃
Yes, I and @renehernandez have already done our best at #1824 and a few Slack channels but had no luck so far. I don't have admin privilege on this repo so I can't add additional maintainers as of today. |
|
FYI, we already have a lot of unit tests like https://github.com/roboll/helmfile/blob/master/pkg/app/app_test.go#L2749 that are for avoiding regressions like this. At least unit tests are passing. Especially the |
|
@mumoshu if it's a PR for fixing the bug, I don't think i'll be able to find and fix the problem within the code :) Regarding the smoke test, the use case here is very simple and straightforward use of |
|
I recently verified Everyone, would you mind sharing a single helmfile.yaml that The first example provided by @dudicoco in this issue lacked |
|
Hi @mumoshu, here's a more complete - name: amazing-app-test-1
chart: my-repo/amazing-app
kubeContext: my-context
namespace: devops
version: 2.0.14
installed: true
timeout: 300
needs:
- my-context/devops/amazing-app-test-3
values: []
- name: amazing-app-test-2
chart: my-repo/amazing-app
kubeContext: my-context
namespace: devops
version: 2.0.14
installed: true
timeout: 300
needs:
- my-context/devops/amazing-app-test-1
values: []
- name: amazing-app-test-3
chart: my-repo/amazing-app
kubeContext: my-context
namespace: devops
version: 2.0.14
installed: true
timeout: 300
needs: []
values: []Let me know if anything else is needed. |
|
@dudicoco Thanks! FWIW, I couldn't reproduce the problem with helmfile 0.139.9 |
|
@mumoshu what command did you use to install the releases? I've used |
|
@dudicoco I just ran plain |
|
@mumoshu i've just tried running it many times with different configurations in my helmfile.yaml and the issue did not go away. Could there be a difference between our helmfile versions? Is there any other information I can provide you with? perhaps debug logs? |
|
@dudicoco Would you mind sharing me a github repo with a complete example of a helmfile project that can reproduce the issue? Also which OS and CPU arch are you using? I use helmfile 0.139.9 on Ubuntu 20.04. I don't actually have access to |
|
@mumoshu please try it with chart I'm using helmfile version v0.139.9 on darwin x86_64. |
|
Hi @mumoshu, how can we track this issue in the new repo? Should I just reopen it there? |
With helmfile v0.139.9 it seems that the
needsdependencies behaviour is broken.With the following example:
Previous behaviour - v0.138.4 (correct):
New behaviour - v0.139.9 (incorrect - releases 1 & 2 are installed in parallel):
The text was updated successfully, but these errors were encountered: