-
Notifications
You must be signed in to change notification settings - Fork 104
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
Use new docker build provider #1278
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
// limitations under the License. | ||
|
||
import * as aws from "@pulumi/aws"; | ||
import * as docker from "@pulumi/docker"; | ||
import * as docker from "@pulumi/docker-build"; | ||
import * as pulumi from "@pulumi/pulumi"; | ||
import * as schema from "../schema-types"; | ||
import * as utils from "../utils"; | ||
|
@@ -62,37 +62,52 @@ export function computeImageFromAsset( | |
throw new Error("Invalid credentials"); | ||
} | ||
return { | ||
registry: ecrCredentials.proxyEndpoint, | ||
address: ecrCredentials.proxyEndpoint, | ||
username: username, | ||
password: password, | ||
}; | ||
}); | ||
|
||
let cacheFrom: docker.types.input.CacheFromArgs[] = []; | ||
if (dockerInputs.cacheFrom !== undefined) { | ||
cacheFrom = dockerInputs.cacheFrom.map((c) => { | ||
return { | ||
registry: { | ||
ref: c, | ||
}, | ||
}; | ||
}); | ||
} | ||
// Use an inline cache by default. | ||
if (cacheFrom.length === 0) { | ||
cacheFrom.push({ registry: { ref: canonicalImageName } }); | ||
} | ||
|
||
let context = "."; | ||
if (dockerInputs.context !== undefined) { | ||
context = dockerInputs.context; | ||
} | ||
|
||
const dockerImageArgs: docker.ImageArgs = { | ||
imageName: canonicalImageName, | ||
build: { | ||
args: dockerInputs.args, | ||
builderVersion: dockerInputs.builderVersion, | ||
cacheFrom: dockerInputs.cacheFrom | ||
? { | ||
images: dockerInputs.cacheFrom, | ||
} | ||
: undefined, | ||
context: dockerInputs.context, | ||
dockerfile: dockerInputs.dockerfile, | ||
platform: dockerInputs.platform, | ||
target: dockerInputs.target, | ||
}, | ||
registry: registryCredentials, | ||
tags: [canonicalImageName], | ||
buildArgs: dockerInputs.args, | ||
cacheFrom: cacheFrom, | ||
cacheTo: [{ inline: {} }], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know if this will increase image sizes substantially? IIRC this is only metadata about the layers so the size increase shouldn't be substantial. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great question! It's actually full cache artifacts, so it can increase the size a bit. I think it would be best to take this out and stay as close to the existing behavior of not doing any caching stuff unless However this makes me realize we probably do want inline caching when Caching Docker images is hard enough as it is and the old provider's behavior was also easy to get wrong. For example new docker.Image("my-app-image", {
build: {
args: {
BUILDKIT_INLINE_CACHE: "1",
},
cacheFrom: {
images: ["foo:latest"],
},
context: "app/",
dockerfile: "app/Dockerfile",
},
imageName: "foo:latest",
}); So for us to stay as close to the existing behavior while also allowing caching to somewhat work, I think we basically just want to enable this |
||
context: { location: context }, | ||
dockerfile: { location: dockerInputs.dockerfile }, | ||
platforms: dockerInputs.platform ? [dockerInputs.platform as docker.Platform] : [], | ||
target: dockerInputs.target, | ||
push: true, | ||
registries: [registryCredentials], | ||
}; | ||
|
||
const image = new docker.Image(imageName, dockerImageArgs, { parent }); | ||
|
||
image.repoDigest.apply((d: any) => | ||
pulumi.log.debug(` build complete: ${imageName} (${d})`, parent), | ||
); | ||
image.ref.apply((ref) => { | ||
pulumi.log.debug(` build complete: ${ref}`, parent); | ||
}); | ||
|
||
return image.repoDigest; | ||
return image.ref; | ||
} | ||
|
||
function createUniqueImageName(inputs: pulumi.Unwrap<schema.DockerBuildInputs>): string { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ | |
"//": "Pulumi sub-provider dependencies must be pinned at an exact version because we extract this value to generate the correct dependency in the schema", | ||
"dependencies": { | ||
"@pulumi/aws": "6.47.0", | ||
"@pulumi/docker": "4.5.1", | ||
"@pulumi/docker-build": "0.0.5", | ||
"@pulumi/pulumi": "3.127.0", | ||
"@types/aws-lambda": "^8.10.23", | ||
"docker-classic": "npm:@pulumi/[email protected]", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,6 @@ const externalRefs = (() => { | |
}; | ||
}; | ||
addRef("aws"); | ||
addRef("docker"); | ||
return externalRefs; | ||
})(); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1666,13 +1666,12 @@ | |
mime "^2.0.0" | ||
resolve "^1.7.1" | ||
|
||
"@pulumi/docker@4.5.1": | ||
version "4.5.1" | ||
resolved "https://registry.yarnpkg.com/@pulumi/docker/-/docker-4.5.1.tgz#9058851bebbb358a1081c765d928fd6791c6c7ba" | ||
integrity sha512-2BTFycFLwSpHGQ4IFTsUHl8H5w81AgkrMHSLUQ8Zu6HBDgGhB5up6YsxVqLeaUeWAedEUrrSCY3xTCNbP4a0ag== | ||
"@pulumi/docker[email protected]": | ||
version "0.0.5" | ||
resolved "https://registry.yarnpkg.com/@pulumi/docker-build/-/docker-build-0.0.5.tgz#9e86ac0761b7fba4f24064095658bf4cf3a03a42" | ||
integrity sha512-oaPSvgwQ0FclGKz8WGdlPvSV4Iw1rg7HkL6dqrTRxNpXBnlPRodDuYRyk/hWCrvQx+dZchJna4j3urFXOWPCEw== | ||
dependencies: | ||
"@pulumi/pulumi" "^3.0.0" | ||
semver "^5.4.0" | ||
|
||
"@pulumi/[email protected]", "@pulumi/pulumi@^3.0.0": | ||
version "3.127.0" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,7 +37,7 @@ func GenerateSchema(packageDir string) schema.PackageSpec { | |
dependencies := readPackageDependencies(packageDir) | ||
awsSpec := getPackageSpec("aws", dependencies.Aws) | ||
awsNativeSpec := getPackageSpec("aws-native", awsNativeTypesVersion) | ||
dockerSpec := getPackageSpec("docker", dependencies.Docker) | ||
dockerSpec := getPackageSpec("docker-build", dependencies.Docker) | ||
|
||
packageSpec := schema.PackageSpec{ | ||
Name: "awsx", | ||
|
@@ -67,25 +67,25 @@ func GenerateSchema(packageDir string) schema.PackageSpec { | |
"generateResourceContainerTypes": true, | ||
"importBasePath": "github.com/pulumi/pulumi-awsx/sdk/v2/go/awsx", | ||
"liftSingleValueMethodReturns": true, | ||
"internalDependencies": []string{"github.com/pulumi/pulumi-docker/sdk/v4/go/docker"}, | ||
"internalDependencies": []string{"github.com/pulumi/pulumi-docker-build/sdk/go/dockerbuild"}, | ||
"respectSchemaVersion": true, | ||
}), | ||
"java": rawMessage(map[string]interface{}{ | ||
"dependencies": map[string]string{ | ||
"com.pulumi:aws": dependencies.Aws, | ||
"com.pulumi:docker": dependencies.Docker, | ||
"com.pulumi:aws": dependencies.Aws, | ||
"com.pulumi:docker-build": dependencies.Docker, | ||
}, | ||
}), | ||
"nodejs": rawMessage(map[string]interface{}{ | ||
"dependencies": map[string]string{ | ||
"@aws-sdk/client-ecs": "^3.405.0", | ||
"@pulumi/pulumi": "^3.0.0", | ||
"@pulumi/aws": "^" + dependencies.Aws, | ||
"@pulumi/docker": "^" + dependencies.Docker, | ||
"docker-classic": "npm:@pulumi/[email protected]", | ||
"@types/aws-lambda": "^8.10.23", | ||
"aws-sdk": "^2.1450.0", | ||
"mime": "^2.0.0", | ||
"@aws-sdk/client-ecs": "^3.405.0", | ||
"@pulumi/pulumi": "^3.0.0", | ||
"@pulumi/aws": "^" + dependencies.Aws, | ||
"@pulumi/docker-build": "^" + dependencies.Docker, | ||
"docker-classic": "npm:@pulumi/[email protected]", | ||
"@types/aws-lambda": "^8.10.23", | ||
"aws-sdk": "^2.1450.0", | ||
"mime": "^2.0.0", | ||
}, | ||
"devDependencies": map[string]string{ | ||
"@types/node": "^18", | ||
|
@@ -96,9 +96,9 @@ func GenerateSchema(packageDir string) schema.PackageSpec { | |
}), | ||
"python": rawMessage(map[string]interface{}{ | ||
"requires": map[string]string{ | ||
"pulumi": ">=3.91.1,<4.0.0", | ||
"pulumi-aws": ">=6.0.4,<7.0.0", | ||
"pulumi-docker": fmt.Sprintf(">=%s,<5.0.0", dependencies.Docker), | ||
"pulumi": ">=3.91.1,<4.0.0", | ||
"pulumi-aws": ">=6.0.4,<7.0.0", | ||
"pulumi-docker-build": fmt.Sprintf(">=%s,<1.0.0", dependencies.Docker), | ||
}, | ||
"usesIOClasses": true, | ||
"readme": "Pulumi Amazon Web Services (AWS) AWSX Components.", | ||
|
@@ -256,7 +256,7 @@ func rawMessage(v interface{}) schema.RawMessage { | |
|
||
type Dependencies struct { | ||
Aws string `json:"@pulumi/aws"` | ||
Docker string `json:"@pulumi/docker"` | ||
Docker string `json:"@pulumi/docker-build"` | ||
Pulumi string `json:"@pulumi/pulumi"` | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any new input properties we should expose as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably! There's a lot more we can do now, e.g. multiple tags, multiple platforms, more refined caching behavior, etc.
I didn't want to add any of those right now because (a) these are opinionated APIs and opinions take more time to get concensus on, and (b) it's not clear how much (if any) we want to continue investing in this wrapper versus giving users more composable building blocks (like your example of registry auth helpers). The latter is my preference, but it's not my call to make.