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

Fixing Docker task issues - Dockerfile in push command, and lowercasing of image names. #10506

Merged
merged 5 commits into from
May 30, 2019
Merged
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
8 changes: 4 additions & 4 deletions Tasks/Common/docker-common/containerconnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,23 +71,23 @@ export default class ContainerConnection {
return imageName;
}

public getQualifiedImageName(repository: string): string {
public getQualifiedImageName(repository: string, enforceDockerNamingConvention?: boolean): string {
let imageName = repository ? repository : "";
if (repository && this.registryAuth) {
imageName = this.prefixRegistryIfRequired(this.registryAuth["registry"], repository);
}

return imageName;
return enforceDockerNamingConvention ? imageUtils.generateValidImageName(imageName) : imageName;
}

public getQualifiedImageNamesFromConfig(repository: string) {
public getQualifiedImageNamesFromConfig(repository: string, enforceDockerNamingConvention?: boolean) {
let imageNames: string[] = [];
if (repository) {
let regUrls = this.getRegistryUrlsFromDockerConfig();
if (regUrls && regUrls.length > 0) {
regUrls.forEach(regUrl => {
let imageName = this.prefixRegistryIfRequired(regUrl, repository);
imageNames.push(imageName);
imageNames.push(enforceDockerNamingConvention ? imageUtils.generateValidImageName(imageName) : imageName);
ajinkya599 marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion Tasks/Common/docker-common/fileutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function writeFileSync(filePath: string, data: string): number {
}
}

export function findDockerFile(dockerfilepath : string) : string {
export function findDockerFile(dockerfilepath: string) : string {
if (dockerfilepath.indexOf('*') >= 0 || dockerfilepath.indexOf('?') >= 0) {
tl.debug(tl.loc('ContainerPatternFound'));
let workingDirectory = tl.getVariable('System.DefaultWorkingDirectory');
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerComposeV0/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerComposeV0/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV0/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV0/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV1/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 1,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "Simplified the task by:<br/>&nbsp;- Providing an option to simply select or type a command.<br/>&nbsp;- Retaining the useful input fields and providing an option to pass the rest as an argument to the command.",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV1/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 1,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "ms-resource:loc.releaseNotes",
Expand Down
9 changes: 5 additions & 4 deletions Tasks/DockerV2/dockerbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function useDefaultBuildContext(buildContext: string): boolean {
return buildContext === defaultPath;
}

export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, ignoreArguments?: boolean): any {
export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, isBuildAndPushCommand?: boolean): any {
// find dockerfile path
let dockerfilepath = tl.getInput("Dockerfile", true);
let dockerFile = fileUtils.findDockerFile(dockerfilepath);
Expand All @@ -25,21 +25,22 @@ export function run(connection: ContainerConnection, outputUpdate: (data: string
}

// get command arguments
let commandArguments = ignoreArguments ? "" : dockerCommandUtils.getCommandArguments(tl.getInput("arguments", false));
// ignore the arguments input if the command is buildAndPush, as it is ambiguous
let commandArguments = isBuildAndPushCommand ? "" : dockerCommandUtils.getCommandArguments(tl.getInput("arguments", false));

// get qualified image names by combining container registry(s) and repository
let repositoryName = tl.getInput("repository");
let imageNames: string[] = [];
// if container registry is provided, use that
// else, use the currently logged in registries
if (tl.getInput("containerRegistry")) {
let imageName = connection.getQualifiedImageName(repositoryName);
let imageName = connection.getQualifiedImageName(repositoryName, true);
if (imageName) {
imageNames.push(imageName);
}
}
else {
imageNames = connection.getQualifiedImageNamesFromConfig(repositoryName);
imageNames = connection.getQualifiedImageNamesFromConfig(repositoryName, true);
}

// get label arguments
Expand Down
27 changes: 17 additions & 10 deletions Tasks/DockerV2/dockerpush.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@ function pushMultipleImages(connection: ContainerConnection, imageNames: string[
return promise;
}

export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, ignoreArguments?: boolean): any {
let commandArguments = ignoreArguments ? "" : dockerCommandUtils.getCommandArguments(tl.getInput("arguments", false));
export function run(connection: ContainerConnection, outputUpdate: (data: string) => any, isBuildAndPushCommand?: boolean): any {
// ignore the arguments input if the command is buildAndPush, as it is ambiguous
let commandArguments = isBuildAndPushCommand ? "" : dockerCommandUtils.getCommandArguments(tl.getInput("arguments", false));

// get tags input
let tags = tl.getDelimitedInput("tags", "\n");

// get qualified image name from the containerRegistry input
// get repository input
let repositoryName = tl.getInput("repository");
if (!repositoryName) {
tl.warning("No repository is specified. Nothing will be pushed.");
Expand All @@ -66,19 +67,25 @@ export function run(connection: ContainerConnection, outputUpdate: (data: string
// if container registry is provided, use that
// else, use the currently logged in registries
if (tl.getInput("containerRegistry")) {
let imageName = connection.getQualifiedImageName(repositoryName);
let imageName = connection.getQualifiedImageName(repositoryName, true);
if (imageName) {
imageNames.push(imageName);
}
}
else {
imageNames = connection.getQualifiedImageNamesFromConfig(repositoryName);
imageNames = connection.getQualifiedImageNamesFromConfig(repositoryName, true);
}

const dockerfilepath = tl.getInput("dockerFile", true);
const dockerFile = findDockerFile(dockerfilepath);
if (!tl.exist(dockerFile)) {
throw new Error(tl.loc('ContainerDockerFileNotFound', dockerfilepath));
let dockerFile = "";
if (isBuildAndPushCommand) {
// For buildAndPush command, to find out the base image name, we can use the
// Dockerfile returned by findDockerfile as we are sure that this is used
// for building.
dockerFile = findDockerFile(dockerfilepath);
if (!tl.exist(dockerFile)) {
throw new Error(tl.loc('ContainerDockerFileNotFound', dockerfilepath));
}
}

// push all tags
Expand All @@ -92,7 +99,7 @@ export function run(connection: ContainerConnection, outputUpdate: (data: string
let digest = extractDigestFromOutput(commandOutput, matchPatternForDigestAndSize);
tl.debug("outputImageName: " + outputImageName + "\n" + "commandOutput: " + commandOutput + "\n" + "digest:" + digest + "imageSize:" + imageSize);
publishToImageMetadataStore(connection, outputImageName, tags, digest, dockerFile).then((result) => {
tl.debug("ImageDetailsApiResponse: " + result);
tl.debug("ImageDetailsApiResponse: " + JSON.stringify(result));
}, (error) => {
tl.warning("publishToImageMetadataStore failed with error: " + error);
});
Expand All @@ -115,7 +122,7 @@ export function run(connection: ContainerConnection, outputUpdate: (data: string
async function publishToImageMetadataStore(connection: ContainerConnection, imageName: string, tags: string[], digest: string, dockerFilePath: string): Promise<any> {
// Getting imageDetails
const imageUri = getResourceName(imageName, digest);
const baseImageName = getBaseImageNameFromDockerFile(dockerFilePath);
const baseImageName = dockerFilePath ? getBaseImageNameFromDockerFile(dockerFilePath) : "NA";
ajinkya599 marked this conversation as resolved.
Show resolved Hide resolved
const layers = await dockerCommandUtils.getLayers(connection, imageName);
const imageSize = dockerCommandUtils.getImageSize(layers);

Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV2/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 2,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "Simplified the task YAML by:<br/>&nbsp;- Removing the Container registry type input<br/>&nbsp;- Removing complex inputs as they can be passed as arguments to the command.",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/DockerV2/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 2,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "ms-resource:loc.releaseNotes",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesManifestV0/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 2
"Patch": 3
},
"demands": [],
"groups": [],
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesManifestV0/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 2
"Patch": 3
},
"demands": [],
"groups": [],
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesV0/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesV0/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 0,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"preview": "false",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesV1/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 1,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "What's new in Version 1.0:<br/>&nbsp;Added new service connection type input for easy selection of Azure AKS cluster.<br/>&nbsp;Replaced output variable input with output variables section that we had added in all tasks.",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/KubernetesV1/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"version": {
"Major": 1,
"Minor": 153,
"Patch": 1
"Patch": 2
},
"demands": [],
"releaseNotes": "ms-resource:loc.releaseNotes",
Expand Down