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

feature: only re-build with pull: true when base images have changed #222

Open
brettwillis opened this issue Aug 20, 2024 · 6 comments
Open
Labels
kind/enhancement Improvements or new features

Comments

@brettwillis
Copy link

What happened?

pulumi up always says the contextHash has changed, and always rebuilds, only to discover that the output has not changed. Immediately re-running will again say context hash is different, event after refresh etc.

Example

pulumi up diff:

    ~ docker-build:index:Image: (update)
        [id=sha256:0a49b414b7272b3b4458cadce0cc62d5c658f5a1c44e5997cf2c12c4cf56f62c]
        [urn=urn:pulumi:dev::myproject::docker-build:index:Image::image-indexer]
        [provider=urn:pulumi:dev::myproject::pulumi:providers:docker-build::default_0_0_6::286a454e-0463-46e7-a196-75e5bd1d6601]
      - contextHash: "0e6f2bec5f86c06b6e95001047445b621ace8b81ac0508bd8770f4f8654a882e"

Output of pulumi about

CLI          
Version      3.129.0
Go Version   go1.22.6
Go Compiler  gc

Plugins
KIND      NAME    VERSION
language  nodejs  unknown

Host     
OS       darwin
Version  14.6.1
Arch     arm64

...


Backend        
Name          <my-machine>
URL            file://<my-home-dir>
User           <my-username>
Organizations  
Token type     personal

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

@brettwillis brettwillis added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 20, 2024
@blampe
Copy link
Contributor

blampe commented Aug 21, 2024

@brettwillis sorry you're running into this. We need some additional information in order to debug the problem:

  • Can you please include the full output of pulumi about, including the version of docker-build you're using?
  • Can you please include the code you're running as part of your program?

In general it's very difficult to diagnose issues like this that are sensitive to the build environment, so the more information you can provide the better!

@blampe blampe added awaiting-feedback and removed needs-triage Needs attention from the triage team labels Aug 21, 2024
@brettwillis
Copy link
Author

Sorry yes here's the provider versions from the stack output, docker-build v0.0.6.

pulumi:providers:cloudflare                       urn:pulumi:dev::hhc::pulumi:providers:cloudflare::default_5_36_0
pulumi:providers:gcp                              urn:pulumi:dev::hhc::pulumi:providers:gcp::default_7_37_0
pulumi:providers:docker-build                     urn:pulumi:dev::hhc::pulumi:providers:docker-build::default_0_0_6
pulumi:providers:pulumi-nodejs                    urn:pulumi:dev::hhc::pulumi:providers:pulumi-nodejs::default

And the relevant part of the program (sorry I should have known to include this in the first place!):

const indexerImage = new docker.Image('image-indexer', {
  tags: [
    pulumi.interpolate`${imageRegistryUrl}/indexer:v${indexerPkg.version}`,
    pulumi.interpolate`${imageRegistryUrl}/indexer:latest`,
  ],
  context: {
    location: '../service-indexer',
  },
  platforms: [
    'linux/amd64',
  ],
  push: true,
  pull: true,
  secrets: {
    'bun': pulumi.secret(pulumi.interpolate`[install.scopes]\nmyorg = { url = "https://npm.pkg.github.com/", token = "${config.requireSecret('github_packages_token')}" }\n`),
  },
}, {
  dependsOn: imageRegistry,
  deletedWith: imageRegistry,
});

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

blampe commented Aug 21, 2024

@brettwillis ahh I see now, thank you!

It's always re-building due to pull: true -- this should definitely be clearer in the docs.

The original thinking behind this behavior was that pull-ing images is typically intended to build against the most current tags, so we should always perform the build to be safe. If we didn't perform the build, and if you didn't touch your Dockerfile for a while, your image could get out of sync with the latest available base images which seems undesirable.

I'm curious about your use case for pulling, though? And does the reasoning above make sense to you or is it unreasonable?

@blampe blampe added awaiting-feedback and removed needs-triage Needs attention from the triage team labels Aug 21, 2024
@brettwillis
Copy link
Author

@blampe ah yes that seems obvious now when you point it out.

I'm curious about your use case for pulling, though? And does the reasoning above make sense to you or is it unreasonable?

Yeah I imagined something a little different in my mind: when I need to do a rebuild due to actual context changes, then you might as well pull the latest images while you're at it... or when it's been a while since the last build... However I can certainly see the reasoning behind the current behaviour.

So maybe what I should be doing instead is specifying the fully-qualified tags of my images in my Dockerfile, then manually updating those when I re-deploy whenever I deem it appropriate.

But yes at least two points of feedback:

  • making this clearer in the docs
  • perhaps (if possible) making it clearer in the update diff somehow (e.g. highlighting pull: true or something instead of missing context)

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

blampe commented Aug 21, 2024

Yep, that's all super reasonable.

One of the aims of the provider has been to stay as close as possible to the native CLI, and this is one area where we're deviating in a surprising way. For example running docker buildx build --pull won't do a full re-build unless the base images have changed. We took a shortcut here because it's a little involved to figure out which images you depend on, but strictly speaking it's possible -- especially considering that we're already parsing your Dockerfile.

In other words, we could:

  • Parse your Dockerfile and pull out all your base images.
  • Store an output like "dependencies" which tracks all your dependent base images and tags. This might be mildly useful just to see when your dependencies change, and it would make the diff a little more obvious.

Then, when pull: true is specified:

  • Issue a HEAD request on your dependent images to find their current digests -- this is fast and can be parallelized.
  • Compare remote digests with existing dependencies and perform a build if they differ.

That way you would get something much closer to Docker's own behavior -- if your app and remotes are both unchanged nothing happens.

@brettwillis
Copy link
Author

That sounds like it would be amazing!

@blampe blampe added kind/enhancement Improvements or new features and removed kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 22, 2024
@blampe blampe changed the title bug: Image always builds (context always out of date) feature: only re-build with pull: true when base images have changed Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

3 participants