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

Old builder.name value is used on preview, may lead to crash #279

Open
stazz opened this issue Oct 7, 2024 · 5 comments
Open

Old builder.name value is used on preview, may lead to crash #279

stazz opened this issue Oct 7, 2024 · 5 comments
Labels
kind/bug Some behavior is incorrect or out of spec

Comments

@stazz
Copy link

stazz commented Oct 7, 2024

What happened?

I hae Image resource with builder.name property serialized to stack from previous runs. Running pulumi preview with different builder name gives this error: error: Preview failed: new builder: no builder "<old builder name>" found.

This happens always if one reuses builder set up by docker/setup-buildx-action action, as it produces unique name on every run.

Example

Have Image resource with builder.name set. Run pulumi up once, shut down old builder instance, create new one with different name, and then run pulumi preview.

Output of pulumi about

CLI
Version      3.134.1
Go Version   go1.23.1
Go Compiler  gc

Host
OS       debian
Version  12.7
Arch     aarch64

Backend
Name           pulumi.com
URL            https://app.pulumi.com
User           Unknown
Organizations
Token type     personal

Pulumi locates its logs in /tmp by default
warning: Failed to read project: no Pulumi.yaml project file found (searching upwards from <cwd>). If you have not created a project yet, use `pulumi new` to do so: no project file found
warning: Failed to get information about the current stack: no Pulumi.yaml project file found (searching upwards from <cwd>). If you have not created a project yet, use `pulumi new` to do so: no project file found

The output is a bit wonky since I am utilising Pulumi Automation API to run my pipeline, as it is a bit complex (has some other components than just Pulumi), and I am storing the state and encryption key in AWS (not using Pulumi cloud). Furthermore, I am running Pulumi inside Docker container pulumi/pulumi-nodejs-22:3.134.1.

But the versions in the package.json are like this:

    "@pulumi/aws-native": "0.125.0",
    "@pulumi/aws": "6.53.0",
    "@pulumi/docker-build": "0.0.6",
    "@pulumi/postgresql": "3.12.0",
    "@pulumi/pulumi": "3.134.1",
    "@pulumi/random": "4.16.6",

Additional context

In order to reproduce this, there already needs to be Image resource existing in stack with some builder.name value.
Then, if a different builder.name value is provided on e.g. preview command, the bug will occur.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@stazz stazz added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 7, 2024
@rquitales
Copy link
Member

rquitales commented Oct 11, 2024

@stazz This would be expected behavior since the old builder no longer exists, and you're instructing Pulumi in your Pulumi program to explicitly use the old deleted builder. You will need to update your Pulumi program's Image.builder.name to point to the newer builder's name. Alternatively, you could remove the Image.builder configuration to use the default ambient builder.

@rquitales rquitales added awaiting-feedback and removed needs-triage Needs attention from the triage team labels Oct 11, 2024
@blampe
Copy link
Contributor

blampe commented Oct 14, 2024

@stazz elaborating on the above, pulumi will automatically create a docker-container builder if one isn't already available (otherwise it will try to use the first available docker-container builder). In many cases this makes the setup-buildx-action unnecessary.

@stazz
Copy link
Author

stazz commented Oct 21, 2024

Ah, so I simply just omit the builder name. That works for my usecase with setup-buildx-action, thank you for advice! 👍

However, I still feel like it is a bug, if I specify correct name for new builder on refresh, and the state has name of old, non-existent builder, and then it crashes (especially since it doesn't crash in same situation for up). Since then the only way to resolve this is to delete resource or edit it in the state file, which might not be very obvious.

I think the nature of Docker BuilderX instance is transient, and the provider should be able to handle errors related to them gracefully, especially if end-state is valid (like it is in this case).

@pulumi-bot pulumi-bot added needs-triage Needs attention from the triage team and removed awaiting-feedback labels Oct 21, 2024
@blampe
Copy link
Contributor

blampe commented Oct 22, 2024

However, I still feel like it is a bug, if I specify correct name for new builder on refresh, and the state has name of old, non-existent builder, and then it crashes (especially since it doesn't crash in same situation for up). Since then the only way to resolve this is to delete resource or edit it in the state file, which might not be very obvious.

I totally agree! Unfortunately Pulumi has a limitation here where it doesn't actually invoke your program for delete or refresh operations -- it just checks whether your current state reflects reality. In other words those operations don't see that you've made any modifications to your code. pulumi/esc#199 (comment)

This comes up frequently in the context of Docker (#121) because these "cloud" resources are actually local and transient.

What I think we can do here is treat the builder's 404 as if the resource was deleted, so your subsequent update will re-create it. Edit: although thinking about it more, the image can still exist even if the builder is gone so I'm not sure this makes sense.

@blampe blampe removed the needs-triage Needs attention from the triage team label Oct 22, 2024
@stazz
Copy link
Author

stazz commented Oct 23, 2024

Ahh, okay, that was interesting piece of knowledge that Pulumi doesn't invoke program for those operations! 🤔 Doesn't sound like that root cause will be resolved anytime soon (at least not during 3.x version era). :(

As to what to do with this particular issue, that is tricky indeed. I noticed that my Image resources do keep disappearing on refresh due to credentials being invalidated, and recreated during other operations. Luckily, I use ECR-backed layer caching, so the ill effects are just excessive delta information during preview. That of course is not nice, but not the end of the world either. But if this behaviour would be the same for when the builder instance is not found, then at least it would be consistent from end-user point of view. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

4 participants