Skip to content

Commit

Permalink
[Port to m153]: Fixing Docker task issues - Dockerfile in push comman…
Browse files Browse the repository at this point in the history
…d, and lowercasing of image names. (#10506) (#10645)

* [Port to m153]: Fixing Docker task issues - Dockerfile in push command, and lowercasing of image names. (#10506)

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

* Use NA as baseImageName in case of only push command. Minor change in debug logs.

* Bumping up task versions

* change as suggested in PR review comment

* rebasing with releases/m153
  • Loading branch information
ajinkya599 authored Jun 17, 2019
1 parent f713764 commit 2d4aa0a
Show file tree
Hide file tree
Showing 18 changed files with 44 additions and 32 deletions.
10 changes: 7 additions & 3 deletions Tasks/Common/docker-common/containerconnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,26 @@ 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);
if (enforceDockerNamingConvention) {
imageName = imageUtils.generateValidImageName(imageName);
}

imageNames.push(imageName);
});
}
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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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";
const layers = await dockerCommandUtils.getLayers(connection, imageName);
if (!layers) {
return null;
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": 4
"Patch": 5
},
"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": 4
"Patch": 5
},
"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": 5
"Patch": 6
},
"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": 5
"Patch": 6
},
"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": 5
"Patch": 6
},
"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": 5
"Patch": 6
},
"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": 5
"Patch": 6
},
"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": 5
"Patch": 6
},
"demands": [],
"releaseNotes": "ms-resource:loc.releaseNotes",
Expand Down

0 comments on commit 2d4aa0a

Please sign in to comment.