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 note clearly describing current limitations of dependsOn #4531

Closed
wants to merge 1 commit into from

Conversation

tricky42
Copy link

@tricky42 tricky42 commented Nov 6, 2020

Proposed changes

Add note clearly describing current limitations of dependsOn
More details can be found in the following discussion thread on Slack: https://pulumi-community.slack.com/archives/C84L4E3N1/p1604604723483000

Related issues (optional)

pulumi/pulumi-kubernetes#861
pulumi/pulumi-kubernetes#1315
pulumi/pulumi-kubernetes#1364
pulumi/pulumi#3282

@@ -848,6 +848,8 @@ The `dependsOn` option provides a list of explicit resource dependency resources

Pulumi automatically tracks dependencies between resources when you supply an input argument that came from another resource's [output properties](#outputs). In some cases, however, you may need to explicitly specify additional dependencies that Pulumi doesn't know about, but must respect. This might happen if a dependency is external to the infrastructure itself---such as an application dependency---or is implied due to an ordering or eventual consistency requirement. These dependencies ensure that resource creation, update, and deletion is done in the correct order.

> **Note:** Currently `dependsOn` doesn't work if you depend on [Helm Charts](https://www.pulumi.com/docs/reference/pkg/kubernetes/helm/) or [ConfigFiles](https://www.pulumi.com/docs/reference/pkg/kubernetes/yaml/configfile). The issue is tracked in the following issues: [Helm](https://github.com/pulumi/pulumi-kubernetes/issues/861), [ConfigFile](https://github.com/pulumi/pulumi-kubernetes/issues/1315).
Copy link
Member

Choose a reason for hiding this comment

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

@lukehoban Curious to hear your thoughts on this change. The note is accurate, but I wasn't sure about the following:

  1. We could expand it to clarify that the reason this doesn't work right is that resources are created inside an apply.
  2. It might be better to document more locally on the API references for YAML/Helm rather than on this general-purpose page.

Copy link
Author

Choose a reason for hiding this comment

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

Just from a user perspective I think it is important it is mentioned in the dependsOn section (at least that there are currently restrictions... Otherwise a user will find the dependsOn documentation when he or she looks for a solution to manually define dependencies and think this will work for all resources. Otherwise it would be necessary that the user looks up the general documentation on dependsOn and the specific resource he wants to depend on.

In my case I somehow didn't thought it could be a bug as it seemed to me my usecase was pretty common and the documentation didn't mention any exception.

Copy link
Member

Choose a reason for hiding this comment

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

Yes - my suggestion would be to document this in the Helm docs, and then here phrase this in terms of what the real limitation is in the dependsOn implementation itself, with a note that Helm charts are an example that might be affected by that and a link to the Helm docs.

I'll take care of the second part in the programming-model update. @tricky42 or @lblackstone would you like to make the update in the Helm docs?

Copy link
Member

Choose a reason for hiding this comment

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

Opened pulumi/pulumi-kubernetes#1773 to track the Helm docs.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we ever updated the programming model docs. I skimmed through https://www.pulumi.com/docs/intro/concepts/resources/#dependson and didn't see anything about it.

@infin8x infin8x added this to the 0.56 milestone Apr 26, 2021
@leezen leezen modified the milestones: 0.56, 0.58 Jun 7, 2021
@lukehoban lukehoban modified the milestones: 0.58, 0.59 Jul 7, 2021
@leezen
Copy link
Contributor

leezen commented Nov 24, 2021

A lot has changed with dependsOn since this PR, especially in the Helm Chart area. Sorry we couldn't get this into a merged state and thank you for the suggestions.

@leezen leezen closed this Nov 24, 2021
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.

None yet

7 participants