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

Perform docker builds even during preview so we can get users errors prior to a real update. #135

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## HEAD (Unreleased)

* `docker.buildAndPushImage` will now build images during a `preview`, not just during an `update`.
This allows docker errors to be found earlier and more safely in the development cycle.

## 1.2.0 (2020-01-29)
* Upgrade to pulumi-terraform-bridge v1.6.4

Expand Down
3 changes: 3 additions & 0 deletions examples/aws/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,6 @@ const image3 = new docker.Image("path-example", {
export const image1Name = image1.imageName;
export const image2Name = image2.imageName;
export const image3Name = image3.imageName;

export const image4Name = docker.buildAndPushImage(
"test-name", "./app", /*repositoryUrl:*/ undefined, /*logResource:*/ undefined);
5 changes: 5 additions & 0 deletions examples/broken_dockerfile/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
name: broken
runtime: nodejs
description: A minimal TypeScript Pulumi program
template:
description: A minimal TypeScript Pulumi program
2 changes: 2 additions & 0 deletions examples/broken_dockerfile/app/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FR OM nginx
COPY content /usr/share/nginx/html
1 change: 1 addition & 0 deletions examples/broken_dockerfile/app/content/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1> Hi from Pulumi </h1>
18 changes: 18 additions & 0 deletions examples/broken_dockerfile/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2016-2018, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

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

export const imageName = docker.buildAndPushImage(
"test-name", "./app", /*repositoryUrl:*/ undefined, /*logResource:*/ undefined);
9 changes: 9 additions & 0 deletions examples/broken_dockerfile/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "broken",
"devDependencies": {
"@types/node": "^8.0.0"
},
"dependencies": {
"@pulumi/pulumi": "dev",
}
}
19 changes: 18 additions & 1 deletion examples/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,23 @@ func TestAws(t *testing.T) {
}
}

func TestBrokenDockerfile(t *testing.T) {
cwd, err := os.Getwd()
if !assert.NoError(t, err) {
t.FailNow()
}

opts := base.With(integration.ProgramTestOptions{
Dependencies: []string{
"@pulumi/docker",
},
Dir: path.Join(cwd, "broken_dockerfile"),
ExpectFailure: true,
SkipRefresh: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: i will be able to pass in SkipUpdate here once pulumi/pulumi#3894 goes in. This will allow us to validate that it's the preview step that is causing us to fail.

})
integration.ProgramTest(t, &opts)
}

func TestNginx(t *testing.T) {
cwd, err := os.Getwd()
if !assert.NoError(t, err) {
Expand Down Expand Up @@ -139,4 +156,4 @@ func TestDockerfileWithMultipleTargets(t *testing.T) {
Dir: path.Join(cwd, "dockerfile-with-targets"),
})
integration.ProgramTest(t, &opts)
}
}
147 changes: 90 additions & 57 deletions sdk/nodejs/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export interface DockerBuild {

let dockerPasswordPromise: Promise<boolean> | undefined;

function useDockerPasswordStdin(logResource: pulumi.Resource) {
function useDockerPasswordStdin(logResource: pulumi.Resource| undefined) {
if (!dockerPasswordPromise) {
dockerPasswordPromise = useDockerPasswordStdinWorker();
}
Expand Down Expand Up @@ -157,31 +157,58 @@ export function buildAndPushImageAsync(
export function buildAndPushImage(
imageName: string,
pathOrBuild: pulumi.Input<string | DockerBuild>,
repositoryUrl: pulumi.Input<string>,
logResource: pulumi.Resource,
repositoryUrl: pulumi.Input<string> | undefined,
logResource: pulumi.Resource | undefined,
connectToRegistry?: () => pulumi.Input<Registry>,
skipPush: boolean = false): pulumi.Output<string> {

return pulumi.all([pathOrBuild, repositoryUrl])
.apply(async ([pathOrBuildVal, repositoryUrlVal]) => {
// We do something rather interesting here. We do not want to proceed if we don't actually have
// a value yet for `pathOrBuild`. So we do a normal `ouput(...).apply(...)`. However, we *do*
// want proceed if we don't have a value yet for `repositoryUrl`. In that case, we'll just
// build without actually pushing. To support that, we run `.apply` on the repoUrl, but we pass
// in `runWithUnknowns:true` to actually continue on in that case.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: higher level layers will likely need the same. i.e. we call buildAndPushImage from inside Outputs higher up that will likely not run since they won't have a known repoUrl. Will be making further changes in pulumi/awsx once this is in.

return pulumi.output(pathOrBuild).apply(pathOrBuild => {
const op = pulumi.output(repositoryUrl);

// Give an initial message indicating what we're about to do. That way, if anything
// takes a while, the user has an idea about what's going on.
logEphemeral("Starting docker build and push...", logResource);
// @ts-ignore Allow calling the 'runWithUnknowns' overload.
const res: pulumi.Output<string> = op.apply(u => buildAndPushImageImpl(pathOrBuild, u), /*runWithUnknowns:*/ true);

const result = await buildAndPushImageWorkerAsync(
imageName, pathOrBuildVal, repositoryUrlVal, logResource, connectToRegistry, skipPush);
return res;
});

// If we got here, then building/pushing didn't throw any errors. Update the status bar
// indicating that things worked properly. That way, the info bar isn't stuck showing the very
// last thing printed by some subcommand we launched.
logEphemeral("Successfully pushed to docker", logResource);
async function buildAndPushImageImpl(
pathOrBuild: string | pulumi.Unwrap<DockerBuild>,
repositoryUrl: string | undefined): Promise<string> {

return result;
});
// if we got an unknown repository url, just set to undefined for the remainder of
// processing. The rest of the code can handle that.
if (pulumi.containsUnknowns(repositoryUrl)) {
repositoryUrl = undefined;
}

if (repositoryUrl) {
checkRepositoryUrl(repositoryUrl);
}

logEphemeral("Starting docker build...", logResource);
const buildResult = await buildImage(imageName, pathOrBuild, repositoryUrl, logResource, connectToRegistry);
logEphemeral("Completed docker build", logResource);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we always attempt to do the build.


// If we have no repository url, then we definitely can't push our build result. Same if
// we're in preview.
if (skipPush || !repositoryUrl || pulumi.runtime.isDryRun()) {
return imageName;
}

logEphemeral("Starting docker push...", logResource);
const result = await pushImage(repositoryUrl, buildResult, logResource);
logEphemeral("Completed docker build", logResource);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we only attempt to push if we're in a normal update.


return result;
}
}

function logEphemeral(message: string, logResource: pulumi.Resource) {
function logEphemeral(message: string, logResource: pulumi.Resource | undefined) {
pulumi.log.info(message, logResource, /*streamId:*/ undefined, /*ephemeral:*/ true);
}

Expand Down Expand Up @@ -219,17 +246,12 @@ export function checkRepositoryUrl(repositoryUrl: string) {
}
}

async function buildAndPushImageWorkerAsync(
baseImageName: string,
pathOrBuild: string | pulumi.Unwrap<DockerBuild>,
repositoryUrl: string,
logResource: pulumi.Resource,
connectToRegistry: (() => pulumi.Input<Registry>) | undefined,
skipPush: boolean): Promise<string> {

checkRepositoryUrl(repositoryUrl);

const tag = utils.getImageNameAndTag(baseImageName).tag;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need the tag except for pushing. so this moved into pushImageAsync.

async function buildImage(
baseImageName: string,
pathOrBuild: string | pulumi.Unwrap<DockerBuild>,
repositoryUrl: string | undefined,
logResource: pulumi.Resource | undefined,
connectToRegistry: (() => pulumi.Input<Registry>) | undefined): Promise<BuildResult> {

// login immediately if we're going to have to actually communicate with a remote registry.
//
Expand Down Expand Up @@ -264,15 +286,25 @@ async function buildAndPushImageWorkerAsync(
if (pullFromCache) {
const dockerBuild = <pulumi.UnwrappedObject<DockerBuild>>pathOrBuild;
const cacheFromParam = (typeof dockerBuild.cacheFrom === "boolean" ? {} : dockerBuild.cacheFrom) || {};
cacheFrom = pullCacheAsync(baseImageName, cacheFromParam, repositoryUrl, logResource);

// pullFromCache is only true if repositoryUrl is present.
cacheFrom = pullCacheAsync(baseImageName, cacheFromParam, repositoryUrl!, logResource);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}

// Next, build the image.
const {imageId, stages} = await buildImageAsync(baseImageName, pathOrBuild, logResource, cacheFrom);
if (imageId === undefined) {
const buildResult = await buildImageAsync(baseImageName, pathOrBuild, logResource, cacheFrom);
if (buildResult.imageId === undefined) {
throw new Error("Internal error: docker build did not produce an imageId.");
}

return buildResult;
}

async function pushImage(repositoryUrl: string, buildResult: BuildResult, logResource: pulumi.Resource | undefined): Promise<string> {
const { imageName: baseImageName, imageId, stages } = buildResult;

const tag = utils.getImageNameAndTag(baseImageName).tag;

// Generate a name that uniquely will identify this built image. This is similar in purpose to
// the name@digest form that can be normally be retrieved from a docker repository. However,
// this tag doesn't require actually pushing the image, nor does it require communicating with
Expand All @@ -283,24 +315,24 @@ async function buildAndPushImageWorkerAsync(

// Use those to push the image. Then just return the unique target name. as the final result
// for our caller to use. Only push the image during an update, do not push during a preview.
if (!pulumi.runtime.isDryRun() && !skipPush) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this moved up to the top-level logic.

// Push the final image first, then push the stage images to use for caching.

// First, push with both the optionally-requested-tag *and* imageId (which is guaranteed to
// be defined). By using the imageId we give the image a fully unique location that we can
// successfully pull regardless of whatever else has happened at this repositoryUrl.

// Next, push only with the optionally-requested-tag. Users of this API still want to get a
// nice and simple url that they can reach this image at, without having the explicit imageId
// hash added to it. Note: this location is not guaranteed to be idempotent. For example,
// pushes on other machines might overwrite that location.
await tagAndPushImageAsync(baseImageName, repositoryUrl, tag, imageId, logResource);
await tagAndPushImageAsync(baseImageName, repositoryUrl, tag, /*imageId:*/ undefined, logResource);

for (const stage of stages) {
await tagAndPushImageAsync(
localStageImageName(baseImageName, stage), repositoryUrl, stage, /*imageId:*/ undefined, logResource);
}


// Push the final image first, then push the stage images to use for caching.

// First, push with both the optionally-requested-tag *and* imageId (which is guaranteed to
// be defined). By using the imageId we give the image a fully unique location that we can
// successfully pull regardless of whatever else has happened at this repositoryUrl.

// Next, push only with the optionally-requested-tag. Users of this API still want to get a
// nice and simple url that they can reach this image at, without having the explicit imageId
// hash added to it. Note: this location is not guaranteed to be idempotent. For example,
// pushes on other machines might overwrite that location.
await tagAndPushImageAsync(baseImageName, repositoryUrl, tag, imageId, logResource);
await tagAndPushImageAsync(baseImageName, repositoryUrl, tag, /*imageId:*/ undefined, logResource);

for (const stage of stages) {
await tagAndPushImageAsync(
localStageImageName(baseImageName, stage), repositoryUrl, stage, /*imageId:*/ undefined, logResource);
}

return uniqueTaggedImageName;
Expand Down Expand Up @@ -332,7 +364,7 @@ async function pullCacheAsync(
imageName: string,
cacheFrom: pulumi.Unwrap<CacheFrom>,
repoUrl: string,
logResource: pulumi.Resource): Promise<string[] | undefined> {
logResource: pulumi.Resource | undefined): Promise<string[] | undefined> {

// Ensure that we have a repository URL. If we don't, we won't be able to pull anything.
if (!repoUrl) {
Expand Down Expand Up @@ -365,14 +397,15 @@ async function pullCacheAsync(
}

interface BuildResult {
imageName: string;
imageId: string;
stages: string[];
}

async function buildImageAsync(
imageName: string,
pathOrBuild: string | pulumi.Unwrap<DockerBuild>,
logResource: pulumi.Resource,
logResource: pulumi.Resource | undefined,
cacheFrom: Promise<string[] | undefined>): Promise<BuildResult> {

let build: pulumi.Unwrap<DockerBuild>;
Expand Down Expand Up @@ -430,14 +463,14 @@ async function buildImageAsync(
const colonIndex = imageId.lastIndexOf(":");
imageId = colonIndex < 0 ? imageId : imageId.substr(colonIndex + 1);

return {imageId, stages};
return { imageName, imageId, stages };
}

async function dockerBuild(
imageName: string,
build: pulumi.Unwrap<DockerBuild>,
cacheFrom: Promise<string[] | undefined>,
logResource: pulumi.Resource,
logResource: pulumi.Resource | undefined,
target?: string): Promise<void> {

// Prepare the build arguments.
Expand Down Expand Up @@ -483,7 +516,7 @@ interface LoginResult {
// registry with that user, there's no need to do it again.
const loginResults: LoginResult[] = [];

function loginToRegistry(registry: pulumi.Unwrap<Registry>, logResource: pulumi.Resource): Promise<void> {
function loginToRegistry(registry: pulumi.Unwrap<Registry>, logResource: pulumi.Resource| undefined): Promise<void> {
const {registry: registryName, username, password} = registry;

// See if we've issued an outstanding requests to login into this registry. If so, just
Expand Down Expand Up @@ -524,7 +557,7 @@ function loginToRegistry(registry: pulumi.Unwrap<Registry>, logResource: pulumi.
async function tagAndPushImageAsync(
imageName: string, repositoryUrl: string,
tag: string | undefined, imageId: string | undefined,
logResource: pulumi.Resource): Promise<void> {
logResource: pulumi.Resource | undefined): Promise<void> {

// Ensure we have a unique target name for this image, and tag and push to that unique target.
await doTagAndPushAsync(createTaggedImageName(repositoryUrl, tag, imageId));
Expand Down Expand Up @@ -572,7 +605,7 @@ function getFailureMessage(
async function runCommandThatMustSucceed(
cmd: string,
args: string[],
logResource: pulumi.Resource,
logResource: pulumi.Resource | undefined,
reportFullCommandLine: boolean = true,
stdin?: string,
env?: { [name: string]: string }): Promise<string> {
Expand Down Expand Up @@ -610,7 +643,7 @@ async function runCommandThatMustSucceed(
async function runCommandThatCanFail(
cmd: string,
args: string[],
logResource: pulumi.Resource,
logResource: pulumi.Resource | undefined,
reportFullCommandLine: boolean,
reportErrorAsWarning: boolean,
stdin?: string,
Expand Down