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 18 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
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>
19 changes: 19 additions & 0 deletions examples/broken_dockerfile/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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";

// This should fail during preview as ./app points at a broken docker file.
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)
}
}
146 changes: 91 additions & 55 deletions sdk/nodejs/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,28 +157,28 @@ export function buildAndPushImageAsync(
export function buildAndPushImage(
imageName: string,
pathOrBuild: pulumi.Input<string | DockerBuild>,
repositoryUrl: pulumi.Input<string>,
repositoryUrl: pulumi.Input<string> | undefined,
logResource: pulumi.Resource,
connectToRegistry?: () => pulumi.Input<Registry>,
skipPush: boolean = false): pulumi.Output<string> {

return pulumi.all([pathOrBuild, repositoryUrl])
.apply(async ([pathOrBuildVal, repositoryUrlVal]) => {

// 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);
// 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);

const result = await buildAndPushImageWorkerAsync(
imageName, pathOrBuildVal, repositoryUrlVal, logResource, connectToRegistry, skipPush);
// @ts-ignore Allow calling the 'runWithUnknowns' overload.
const res: pulumi.Output<string> = op.apply(u => helper(pathOrBuild, u), /*runWithUnknowns:*/ true);

// 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);
return res;
});

return result;
});
function helper(pathOrBuild: string | pulumi.Unwrap<DockerBuild>, repositoryUrl: string | undefined) {
return buildAndPushImageWorkerAsync(imageName, pathOrBuild, repositoryUrl, logResource, connectToRegistry, skipPush);
}
}

function logEphemeral(message: string, logResource: pulumi.Resource) {
Expand Down Expand Up @@ -220,16 +220,51 @@ export function checkRepositoryUrl(repositoryUrl: string) {
}

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

checkRepositoryUrl(repositoryUrl);
// 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;
}

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.

if (repositoryUrl) {
checkRepositoryUrl(repositoryUrl);
}

// First, login and pulling from docker if we can.
const cacheFrom = await loginAndPullFromCacheAsync(imageName, pathOrBuild, repositoryUrl, logResource, connectToRegistry);

// Then actually kick off the build.
logEphemeral("Starting docker build...", logResource);
const buildResult = await buildImageAsync(imageName, pathOrBuild, cacheFrom, logResource);
logEphemeral("Completed docker build", logResource);

// 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;
}

// Finally, if this a real update, push the built images to docker.
logEphemeral("Starting docker push...", logResource);
const result = await pushImageAsync(repositoryUrl, buildResult, logResource);
logEphemeral("Completed docker build", logResource);

return result;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried to make it very clear in teh top level function what the overall big-steps are that we're performing. The nuts-and-bolts logic is now moved into the helpers.


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

// login immediately if we're going to have to actually communicate with a remote registry.
//
Expand Down Expand Up @@ -260,19 +295,22 @@ async function buildAndPushImageWorkerAsync(
}

// If the container specified a cacheFrom parameter, first set up the cached stages.
let cacheFrom = Promise.resolve<string[] | undefined>(undefined);
if (pullFromCache) {
const dockerBuild = <pulumi.UnwrappedObject<DockerBuild>>pathOrBuild;
const cacheFromParam = (typeof dockerBuild.cacheFrom === "boolean" ? {} : dockerBuild.cacheFrom) || {};
cacheFrom = pullCacheAsync(baseImageName, cacheFromParam, repositoryUrl, logResource);
}

// Next, build the image.
const {imageId, stages} = await buildImageAsync(baseImageName, pathOrBuild, logResource, cacheFrom);
if (imageId === undefined) {
throw new Error("Internal error: docker build did not produce an imageId.");
// pullFromCache is only true if repositoryUrl is present.
return await pullCacheAsync(baseImageName, cacheFromParam, repositoryUrl!, logResource);
}

return [];
}

async function pushImageAsync(repositoryUrl: string, buildResult: BuildResult, logResource: pulumi.Resource): 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 +321,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,11 +370,11 @@ async function pullCacheAsync(
imageName: string,
cacheFrom: pulumi.Unwrap<CacheFrom>,
repoUrl: string,
logResource: pulumi.Resource): Promise<string[] | undefined> {
logResource: pulumi.Resource): Promise<string[]> {

// Ensure that we have a repository URL. If we don't, we won't be able to pull anything.
if (!repoUrl) {
return undefined;
return [];
}

pulumi.log.debug(`pulling cache for ${imageName} from ${repoUrl}`, logResource);
Expand Down Expand Up @@ -365,15 +403,16 @@ async function pullCacheAsync(
}

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

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

let build: pulumi.Unwrap<DockerBuild>;
if (typeof pathOrBuild === "string") {
Expand Down Expand Up @@ -430,13 +469,13 @@ 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>,
cacheFrom: string[],
logResource: pulumi.Resource,
target?: string): Promise<void> {

Expand All @@ -453,11 +492,8 @@ async function dockerBuild(
if (build.target) {
buildArgs.push(...["--target", build.target]);
}
if (build.cacheFrom) {
const cacheFromImages = await cacheFrom;
if (cacheFromImages && cacheFromImages.length) {
buildArgs.push(...["--cache-from", cacheFromImages.join()]);
}
if (cacheFrom.length) {
buildArgs.push(...["--cache-from", cacheFrom.join()]);
}
if (build.extraOptions) {
buildArgs.push(...build.extraOptions);
Expand Down