pulumi / pulumi-docker

A Docker Pulumi resource package, providing multi-language access to Docker resources and building images.
82 stars 14 forks source link

Docker 4.0.0 - What Changed? (Request for feedback) #436

Closed guineveresaenger closed 1 year ago

guineveresaenger commented 1 year ago

Hello and welcome to pulumi-docker 4.x.x preview! 🎉

Here is a summary of the changes that are being made for you to try out and report back on. Be aware that some of the features from 3..x.x may not have made it into this version yet; it is helpful to us if you let us know which ones you cannot live without. Feature requests welcome! Please use the v4.x.x tag when doing so. 🙏

v4.0.0-alpha.0

Deprecated (not in schema):

Soft deprecated (in schema, but no functionality)

Not feature complete

Migrating

The new stuff!

Latest on default branch

jkodroff commented 1 year ago

My most important use case is a smooth (and working) interface to explicitly specify the platform of the build image, e.g. linux/amd64. I'd like to see an intuitive, explicit parameter for this, something like:

new docker.Image("my-container", {
  build: {
    platform: "linux/amd64",
  },
})

(Whether the setting goes underbuild is debatable.)

bombillazo commented 1 year ago

Super excited about this!

AaronFriel commented 1 year ago

I believe we can close this issue and/or refer users to the release notes?

LouisAmon commented 1 year ago

My agency and myself have been using pulumi-docker since 1.x, so we're welcoming this new major version with open arms 🤗

I'll make a few comments for the sake of progress:

The image Id output field is currently the same as the Image name. This may be subject to change.

I do believe that putting the digest here makes more sense, and it identifies the image uniquely whereas the image name doesn't necessarily

At the very least, it'd be nice to have image.digest as Resource output

localImageName is deprecated

Well, that's kind of an issue for us.

In my opinion, you don't want to keep automatically-generated docker images in your local docker registry as it pollutes a lot.

In 3.x, there were a lot of 123abc-container images on top of the remote ones (e.g. 123456789012.dkr.ecr.eu-west-1.amazonaws.com/foo), so if you used local_image_name you ended up with 3 of each (at least)

Before working with Pulumi, I used to do CLI scripts like this:

# Build the image with a humanly readable name
docker build . --tag my-org/foo

# Tag the image before pushing it to the registry
docker tag my-org/foo 123456789012.dkr.ecr.eu-west-1.amazonaws.com/foo

# Push to AWS ECR
aws ecr push 123456789012.dkr.ecr.eu-west-1.amazonaws.com/foo

# Untag the ECR image to clean up the local namespace
docker rmi my-org/foo 123456789012.dkr.ecr.eu-west-1.amazonaws.com/foo

The idea behind it was that the build artifacts should be cleaned up to improve the Developer Experience, making sure you don't have to regularly browse through images manually.

So, in that regard, 4.0 is a step forward (no more xxxxxx-container images that we need to cleanup), but it's still not doing what we would like because it only keeps the ugly, difficult to read, registry-ready image name in the local registry... which could be improved !

CacheFrom

It would be great if you could also add the CacheTo argument, which is required in order to leverage the GitHub Actions Caching

https://docs.docker.com/build/cache/backends/gha/

🙏

In short, our xmas list is:

That being said, I think you did an outstanding job already releasing 4.0 🚀 Keep up the good work 💪

gunar commented 1 year ago

Your Docker image will not build every single time you run pulumi up; instead, it will trigger an update only on changes to your Pulumi program

It isn't immediately clear to me how I can inform Pulumi that the underlying files changed and that the image should be re-built and pushed. Can you elaborate?

Here's a sample code:

new docker.Image("image", {
  imageName: pulumi.interpolate`gcr.io/${gcp.config.project}/service:${env}`,
  build: {
    context: "../service",
    target: "production",
    platform: "linux/amd64",
  },
}).imageName
LouisAmon commented 1 year ago

My understanding is that pulumi-docker reads the context (so "../service" in your case) and applies your ".dockerignore" file (if any) to find out about the files present in the context

It then forms a digest of all the hashes in more or less the same way that docker's own engine would, and that digest serves as a diff : if no file has changed then the digest is the same and no diff appears

gunar commented 1 year ago

If that is the case, then it isn't working for me on v4.2.1 and with GCR.

gunar commented 1 year ago

In case anyone else if feeling lost, here's the helper code that I ended up writing. It generates a hash of the directory as a unique identifier.

const hash = await calculateDirectoryHash("../service");
new docker.Image("nextjs-image", {
  imageName: pulumi.interpolate`gcr.io/${gcp.config.project}/service:${env}-${hash}`,
  build: {
    context: "../service",
    target: "production",
    platform: "linux/amd64",
  },
});

// …

import * as hasha from "hasha";
import * as ignore from "ignore";
async function calculateDirectoryHash(dir) {
  const ig = ignore.default();
  const gitignorePath = path.join(dir, ".gitignore");

  try {
    await fs.access(gitignorePath);
    const gitignoreContents = await fs.readFile(gitignorePath);
    ig.add(gitignoreContents.toString());
  } catch (error) {
    // If file does not exist, do nothing
  }

  const dirContent = await fs.readdir(dir);
  const validContent = dirContent.filter((file) => !ig.ignores(file));

  const hashes = await Promise.all(
    validContent.map(async (item) => {
      const itemPath = path.join(dir, item);
      const stat = await fs.stat(itemPath);

      if (stat.isDirectory()) {
        // If the item is a directory, recurse into it
        return calculateDirectoryHash(itemPath);
      } else {
        // If the item is a file, hash its content
        return hasha.fromFile(itemPath, { algorithm: "sha256" });
      }
    })
  );

  // Hash the concatenated hashes of all items in the directory
  return hasha(hashes.join(""), { algorithm: "sha256" });
}

I do feel like I'm going in circles here and that there should be a native way to do this.

jchook commented 1 year ago

@gunar have you tried a minimal reproduction of your issue?

The built-in system usually works for me -- does not try to rebuild images unless files have changed.

Perhaps there is a configuration fix.

AaronFriel commented 1 year ago

@gunar can you create an issue with a repro, ideally a GitHub repository containing a Pulumi program & steps to reproduce by modifying a file?

We do know there are some issues with dockerignore behavior, and we're looking into that.

AaronFriel commented 1 year ago

@gunar I've opened a PR #646 which should address any gap between the context hashing in our provider & what you're seeing.

lukehoban commented 1 year ago

Closing this issue out, as the Docker 4.0.0 release (and several additional minor version updates) have been shipped now, and most of the topics raised on this thread have been addressed. For any additional feedback, please do open issues to track!

gunar commented 1 year ago

Apparently I should be using .repoDigest instead of .imageName. It's weird that the latter used to work for me and I'm not sure when that changed. Anyway, it works now and I've been able to remove my workaround. Thank you.