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

(Architectural) docker.Image builds image during construction #402

Closed
tenwit opened this issue Oct 20, 2022 · 4 comments
Closed

(Architectural) docker.Image builds image during construction #402

tenwit opened this issue Oct 20, 2022 · 4 comments
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@tenwit
Copy link

tenwit commented Oct 20, 2022

What happened?

The docker.Image constructor does actual work, including building images. This does not follow the Pulumi standard / convention / idiom, and breaks unit tests.

In cloud-centric packages, all work is done in providers, once the Pulumi engine is called. But for Pulumi docker, work is being done in our Pulumi code, before the engine gets involved.

This work fails in any environment that doesn't meet all the requirements (including, has Docker installed), including unit test environments, where it is reasonable to expect that it would work.. since in unit test environments, the only thing that should happen is that resources get created (via the newResource parameter to runtime.setMocks()).

As it currently stands, it's not possible to write unit tests around docker.Image.

Steps to reproduce

MCVE. Run in jest (or tweak to run in mocha):

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

it("should pass without logging errors afterwards", function mcve() {
  new docker.Image("mcve", {
    imageName: "mcve",
    build: "."
  }, {});
  expect(true).toBe(true);
})

beforeAll(function setUpState() {
  pulumi.runtime.setMocks({
    newResource: (
      args: pulumi.runtime.MockResourceArgs,
    ): { id: string; state: any } => {
      let state = {
        ...args.inputs,
        id: `${args.inputs.name ?? args.name}_id`,
        name: args.inputs.name ?? args.name,
        arn: `aws:arn:us-west-2:${args.type}:${args.name}:${args.name}_id`, // The ARN value doesn't matter. It just needs a value to prove that it has been created.
      };

      return {
        id: state.id,
        state: state,
      };
    },
    call: (args: pulumi.runtime.MockCallArgs) => {
      return args.inputs;
    },
  });
});

Expected Behavior

Resource objects should be created, but no actual working (including attempted to create an image) should happen.

Actual Behavior

After the test passes, a slew of errors (rejected promises notifications) are dumped to console. Snippets:

  ●  Cannot log after tests are done. Did you forget to wait for something async in your test?
    Attempted to log "info: [runtime] ".

      at console.log (../node_modules/@jest/console/build/BufferedConsole.js:199:10)
      at Object.info (../node_modules/@pulumi/log/index.ts:60:17)
      at logEphemeral (../node_modules/@pulumi/docker.ts:186:16)
      at Interface.<anonymous> (../node_modules/@pulumi/docker.ts:655:31)
The command 'docker' could not be found in this WSL 2 distro.
We recommend to activate the WSL integration in Docker Desktop settings.

For details about using Docker Desktop with WSL 2, visit:

https://docs.docker.com/go/wsl2/


(node:1387) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:1387) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:1387) UnhandledPromiseRejectionWarning: Error: ' docker build . -t mcve' failed with exit code 1

Output of pulumi about

CLI
Version 3.34.0
Go Version go1.17.11
Go Compiler gc

Plugins
NAME VERSION
aws 4.38.1
command 0.0.3
datadog 4.9.0
docker 3.2.0
nodejs unknown
random 4.8.0

Host
OS debian
Version 11.3
Arch x86_64

This project is written in nodejs: executable='/usr/local/bin/node' version='v16.15.1'

Backend
Name pulumi.com
URL https://app.pulumi.com/paulhicks_sentify
User paulhicks_sentify
Organizations paulhicks_sentify, mahanatx, iqa-managed-services

Dependencies:
NAME VERSION
@mahana-therapeutics/pulumi-aws-accounts 1.0.5
@pulumi/aws 4.38.1
@pulumi/random 4.8.2
@pulumi/pulumi 3.42.0
@pulumi/docker 3.4.1
@types/jest 27.5.2
@types/node 17.0.45
@types/rimraf 3.0.2
@typescript-eslint/eslint-plugin 5.40.0
@typescript-eslint/parser 5.40.0
eslint 8.25.0
eslint-config-airbnb-base 15.0.0
eslint-config-prettier 8.5.0
eslint-plugin-import 2.26.0
eslint-plugin-prettier 4.2.1
husky 8.0.1
jest 28.1.3
prettier 2.7.1
release-it 14.14.3
rimraf 3.0.2
ts-jest 28.0.8
typescript 4.8.4

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

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

@tenwit tenwit added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 20, 2022
@tenwit
Copy link
Author

tenwit commented Oct 20, 2022

This is the same sort of problem as pulumi/pulumi#11113. In that case, the zip file for a lambda is being created during resource construction time. As in this case, it would work if that stuff happened after the Pulumi code has done its work, and during the engine's execution.

@tenwit
Copy link
Author

tenwit commented Oct 20, 2022

Our suites of unit tests for installing applications from source to ECS is unrunnable right now. The whole lot has to be skip()ped. There's quite a few component resources that can't be tested because they construct a docker.Image, or depend on an image.

@guineveresaenger
Copy link
Contributor

Hi @tenwit - I believe this is because Image is currently not a full CRUD resource and we are working on making it one very soon.

Thank you for filing this issue.

@guineveresaenger guineveresaenger removed the needs-triage Needs attention from the triage team label Oct 21, 2022
@AaronFriel AaronFriel added the resolution/fixed This issue was fixed label Mar 9, 2023
@AaronFriel
Copy link
Contributor

This issue should be resolved with the new implementation of the Docker Image resource in v4! See our blog post for more info: https://www.pulumi.com/blog/build-images-50x-faster-docker-v4/

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

No branches or pull requests

4 participants