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

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

Closed
guineveresaenger opened this issue Dec 6, 2022 · 13 comments
Closed

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

guineveresaenger opened this issue Dec 6, 2022 · 13 comments
Assignees
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed

Comments

@guineveresaenger
Copy link
Contributor

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

  • [localImageName](https://www.pulumi.com/registry/packages/docker/api-docs/image/#localimagename_nodejs) - please use imageName in combination with skipPush: true
  • [digest](https://www.pulumi.com/registry/packages/docker/api-docs/image/#digest_nodejs)
  • [id](https://www.pulumi.com/registry/packages/docker/api-docs/image/#id_nodejs)

Soft deprecated (in schema, but no functionality)

  • [env](https://www.pulumi.com/registry/packages/docker/api-docs/image/#env_nodejs) - takes any field you like, does nothing. Use BuilderVersion instead

Not feature complete

  • [extraOptions](https://www.pulumi.com/registry/packages/docker/api-docs/image/#extraoptions_nodejs) - we need to implement all former CLI flags explicitly. Feedback requested! We will also set these explicitly as buildArg options, not a grab bag.
  • CacheFrom and stages` - coming soon
  • target - coming soon
  • Full documentation - coming soon to the registry

Migrating

  • localImageName is deprecated. You can build any image using imageName. You may set skipPush to true if you do not wish to push your image to a registry.
  • ImageRegistry is now Registry.
  • For dotnet users, you will need to import using Pulumi.Docker.Inputs;and instead of new DockerBuild you will need to migrate to new DockerBuildArgs.
  • For Python users, you will need to import DockerBuildArgs and use DockerBuildArgs instead of DockerBuild to construct a new Docker build object.
  • The image Id output field is currently the same as the Image name. This may be subject to change.

The new stuff!

  • Image is a Custom Resource with Create, Update, and Delete enabled. This means the provider will detect a diff when the underlying code has changed! 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. See thoughts on implementing Read
  • The Docker client handles all logic. No more shelling out to the Docker CLI. This should help resolve race conditions in authenticating against a registry.
  • This provider is YAML compatible.
  • Image is fully schematized: all language SDKs should exhibit the same behavior.
  • We see build logs output to the console
  • We see push logs output to the console

Latest on default branch

  • Buildkit mode is enabled by default
@jkodroff
Copy link
Member

jkodroff commented Dec 9, 2022

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
Copy link

Super excited about this!

@AaronFriel AaronFriel pinned this issue Mar 9, 2023
@AaronFriel AaronFriel unpinned this issue Mar 9, 2023
@AaronFriel
Copy link
Contributor

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

@LouisAmon
Copy link

LouisAmon commented Mar 14, 2023

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:

  • Support for CacheTo argument (GitHub Actions caching)
  • Being able to set a local_image_name AND untag_remote_name=True would be a great combo to keep a tidy local registry
  • digest output (and potential image.id)

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

@gunar
Copy link

gunar commented May 24, 2023

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
Copy link

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
Copy link

gunar commented May 24, 2023

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

@gunar
Copy link

gunar commented May 24, 2023

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
Copy link

jchook commented May 25, 2023

@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
Copy link
Contributor

@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
Copy link
Contributor

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

@lukehoban lukehoban added kind/enhancement Improvements or new features resolution/fixed This issue was fixed labels Jul 15, 2023
@lukehoban
Copy link
Contributor

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
Copy link

gunar commented Sep 12, 2023

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.

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 resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

8 participants