Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Failed to build an image if dockerfile contains "future" output values #96

Open
xunleii opened this issue Jun 9, 2024 · 7 comments · May be fixed by #103
Open

Failed to build an image if dockerfile contains "future" output values #96

xunleii opened this issue Jun 9, 2024 · 7 comments · May be fixed by #103
Labels
awaiting-upstream Awaiting upstream dependency impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec

Comments

@xunleii
Copy link

xunleii commented Jun 9, 2024

What happened?

To give some context, I'm trying to “modify” images on the fly to inject files directly into a previously created image. To do this, I need to reference a previous image into the dockerfile field (see example).

Unfortunately, pulumi doesn't seem to appreciate this behavior and tries to search for a Dockerfile, as if the dockerfile were empty :

Update preview (dev) :
     Type Name Plan Info
 + pulumi:pulumi:Stack pulumi-docker-build-issues-dev create
 + ├─ docker-build:index:Image a create 1 warning
     └─ docker-build:index:Image b 1 error

Diagnostics :
  docker-build:index:Image (a):
    warning : Skipping preview build because some inputs are unknown. docker-build:index:Image (b):
    error: docker-build:index:Image resource 'b': property dockerfile.location value {<nil>} has a problem: open <redacted>/Dockerfile: no such file or directory  

Example

import * as pulumi from "@pulumi/pulumi";
import * as docker from "@pulumi/docker-build";

const a = new docker.Image("a", { 
    push: true,
    context: { location: "." },
    dockerfile: {inline: "FROM busybox\n"},
    tags: ["localhost/pulumi/a:latest"],
});
const b = new docker.Image("b", {
    push: true,
    context: { location: "." },
    dockerfile: {inline: pulumi.interpolate`FROM ${a.ref}\n`},
    tags: ["localhost/pulumi/b:latest"],
});

export const images = {
    a: a.ref,
    b: b.ref,
};

Output of pulumi about

CLI
Version      3.119.0
Go Version   go1.22.3
Go Compiler  gc                                                                                                                                                                                                                                                                                                                                                                                                                     Plugins
KIND      NAME          VERSION
resource  docker-build  0.0.2
language  nodejs        unknown                                                                                                                                                                                                                                                                                                                                                                                                     Host
OS       ubuntu
Version  22.04
Arch     x86_64                                                                                                                                                                                                                                                                                                                                                                                                                     This project is written in nodejs: executable='${HOME}/.asdf/shims/node' version='v20.10.0'                                                                                                                                                                                                                                                                                                                                 Backend
Name           <redacted>
URL            file://~
User           <redacted>
Organizations
Token type     personal                                                                                                                                                                                                                                                                                                                                                                                                             Dependencies:
NAME                  VERSION
@pulumi/docker-build  0.0.2
@pulumi/pulumi        3.119.0
@types/node           18.19.34
typescript            5.4.5

Pulumi locates its logs in /tmp by default
warning: Failed to get information about the current stack: No current snapshot

Additional context

No response

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).

@xunleii xunleii added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Jun 9, 2024
@mjeffryes
Copy link
Member

Thanks for filing this issue and the PR @xunleii! I'm going to defer to @blampe to review the PR (he should be able to take a look next week), but we should be able to make this scenario work. (We may want to be a little more explicit about threading through the unknowns since when we do support build on preview.)

@mjeffryes mjeffryes added impact/usability Something that impacts users' ability to use the product easily and intuitively and removed needs-triage Needs attention from the triage team labels Jun 12, 2024
@xunleii
Copy link
Author

xunleii commented Jun 13, 2024

Thanks for your response @mjeffryes. That's right, I forgot that we have the flag buildOnPreview. I will update my PR to include the following behaviors :

  • If there is at least dockerfile.inline or docker.location, we can continue de process without issue
  • If none of these values are provided but we are in preview stage and buildOnPreview is enabled :
    • If context.location + /Dockerfile exists, it's good to go
    • Otherwise, we should stop the processing ahead and advise the user that we can't continue due to the lack of information about the Dockerfile
  • If none of these values are provided but we are in preview stage and buildOnPreview is not enabled, we can try to accept this lack of information and continue to the build stage

I will also try to add some test to validate these possible behavior.

@xunleii
Copy link
Author

xunleii commented Jun 14, 2024

After testing on my side, it seems that there is a guard to prevent any build during preview if the entries contains unknown values (

if preview && !input.buildable() {
provider.GetLogger(ctx).Warning("Skipping preview build because some inputs are unknown.")
return id, state, nil
}
).

Thanks to this check and because this issue is related to unknown data inside a Dockerfile, I think my changes should not impact the buildOnPreview behavior.

@blampe
Copy link
Contributor

blampe commented Jun 17, 2024

@xunleii do you still see the issue with v0.0.3? That release included some tweaks around inline dockerfile handling that might address this.

@xunleii
Copy link
Author

xunleii commented Jun 17, 2024

Unfortunately, I have the same issue with this version (v0.0.3)

Diagnostics:
  docker-build:index:Image:
    error: docker-build:index:Image resource 'embedded': property dockerfile.location value {<nil>} has a problem: open /tmp/pulumi-eXoftt2i-4m3noi0H/Dockerfile: no such file or directory

EDIT: I still have this issue trying to create the image with pulumi.Output value inside the dockerfile and a context path

@blampe
Copy link
Contributor

blampe commented Jun 18, 2024

@xunleii thanks for confirming, and thank you for the PR! I will take a look.

The proposal in #103 is reasonable, although I worry about the situation where the user has omitted Dockerfile information, but a default Dockerfile (<context>/Dockerfile) doesn't exist. We'd like to detect problems like this as early as possible, but unfortunately due to pulumi/pulumi-go-provider#155 we're not currently able to distinguish this case (a value is computed/unknown) versus the case where a value has been omitted (and a default should be applied).

@xunleii
Copy link
Author

xunleii commented Jun 18, 2024

@blampe thanks for your review.

I created this repo (A Codespace is possible if you want to test it on your browser) to test all possible cases I found. From what I've seen, #103 hasn't added any regressions but has two new unexpected behaviors when the Docker file contains pulumi.outputs.

As you pointed out, we are currently unable to distinguish unknwon from empty values until pulumi/pulumi-go-provider#155 is resolved and I haven't found a better alternative for this.

For my personal use, I will keep a fork with my patch because I know what the limitations are, but otherwise I propose to wait until the issue in question is resolved.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
awaiting-upstream Awaiting upstream dependency impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec
Projects
None yet
4 participants