Azure web app - support multicontainer #9943

20shivangi · 2019-03-27T05:22:02Z

No description provided.

vincent1173 · 2019-03-29T06:53:30Z

Tasks/AzureWebAppContainerV1/task.json

@@ -95,12 +95,20 @@
        },
        {
            "name": "imageName",


Please syncup with @azooinmyluggage or @RoopeshNair for updating the help strings.

@azooinmyluggage , Can you please provide help strings for this too? The help text should convey to provide the image name for single container and "replaceable images" for multi-containers.
Should we add an alias as "images" for multi-container?

vincent1173 · 2019-03-29T06:53:48Z

Tasks/AzureWebAppContainerV1/task.json

            "label": "Image name",
            "defaultValue": "",
            "required": true,
            "helpMarkDown": "A globally unique top-level domain name for your specific registry or namespace.<br/> Note: Fully qualified image name will be of the format: '<b>`<registry or namespace`></b>/`<repository`>:`<tag`>'. For example, '<b>myregistry.azurecr.io</b>/nginx:latest'."
        },
+        {
+            "name": "configFilePath",


confirm the name with PMs

Rename it to multicontainerConfigFile

multicontainerConfigFile is inline with the az cli parameter

az webapp config container set –resource-group [resource group] –name [app name] –multicontainer-config-type “compose” –multicontainer-config-file

Tasks/AzureWebAppContainerV1/taskparameters.ts

vincent1173 · 2019-03-29T06:55:52Z

Tasks/AzureWebAppContainerV1/taskparameters.ts

@@ -36,6 +39,18 @@ export class TaskParametersUtility {
        var endpointTelemetry = '{"endpointId":"' + taskParameters.connectedServiceName + '"}';
        console.log("##vso[telemetry.publish area=TaskEndpointId;feature=AzureRmWebAppDeployment]" + endpointTelemetry);

+        taskParameters.isMultiContainer = false;
+        taskParameters.isMultiContainer = taskParameters.ImageName && taskParameters.ImageName.indexOf("\n") !=-1;


if there is a valid file provided in config file, then also it is multicontainer app

It is done.

This is something we should discuss with PMs. Because then imageName input can be optional.

It should be optional. In KubernetesManifest task as well, the containers input is optional. One needs to provide a value for this input only if they want to perform tag/sha256 substitutions for the images specified in the docker compose file.

We had in mandatory in our task since for single container scenarios it is required. Making it optional will allow the user to execute the task with invalid inputs though we will catch it at run time.

Tasks/Common/AzureRmDeploy-common/operations/ContainerBasedDeploymentUtility.ts

Tasks/AzureWebAppContainerV1/taskparameters.ts

SumiranAgg · 2019-04-01T06:42:34Z

Tasks/Common/AzureRmDeploy-common/operations/ContainerBasedDeploymentUtility.ts

+        return newFilePath;
+    }
+
+    private getTempDirectory(): string {


Move this function to azure-pipelines-tasks/Tasks/Common/AzureRmDeploy-common/webdeployment-common/utility.ts

it is done.

SumiranAgg · 2019-04-01T06:44:14Z

Tasks/Common/AzureRmDeploy-common/operations/ContainerBasedDeploymentUtility.ts

+        return tl.getVariable('agent.tempDirectory') || os.tmpdir();
+    }
+
+    private tokenizeImages(currentString: string, imageName: string, imageNameWithNewTag: string) {


change current string to content

It is done.

SumiranAgg · 2019-04-01T06:51:07Z

Tasks/Common/AzureRmDeploy-common/operations/ContainerBasedDeploymentUtility.ts

        tl.debug("Updating the webapp configuration.");
-        await this._updateConfigurationDetails(properties["ConfigurationSettings"], properties["StartupCommand"], imageName, isLinuxApp);
+        await this._updateConfigurationDetails(properties["ConfigurationSettings"], properties["StartupCommand"], imageName, isLinuxApp, isMultiContainer, configFilePath);


Should you be using updatedFilePath here? I couldnt find where you updated original file content.
It seems this will deploy provided config file content without transformation.

It is done.

SumiranAgg · 2019-04-01T06:52:40Z

Tasks/AzureWebAppContainerV1/Tests/utils.ts

@@ -50,6 +50,10 @@ export function mockTaskArgument():  ma.TaskLibAnswers{
                },
                "webAppPkg": {
                    "isDirectory": true


Why do we have changes in this file? Does this PR contain L0s as well?

no, it was made to fix some L0,
Existing L0 is sufficient here.

azooinmyluggage · 2019-04-09T08:27:15Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

@@ -17,6 +17,8 @@
  "loc.input.help.slotName": "Enter or Select an existing Slot other than the Production slot.",
  "loc.input.label.imageName": "Image name",
  "loc.input.help.imageName": "A globally unique top-level domain name for your specific registry or namespace.<br/> Note: Fully qualified image name will be of the format: '<b>`<registry or namespace`></b>/`<repository`>:`<tag`>'. For example, '<b>myregistry.azurecr.io</b>/nginx:latest'.",
+  "loc.input.label.configFilePath": "Configuration File",
+  "loc.input.help.configFilePath": "Path of the configuration. Should be fully qualified path or relative to the default working directory.",


Path of the Docker-Compose file. Should be a fully qualified path or relative to the default working directory.

It is done.

azooinmyluggage · 2019-04-09T08:29:27Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

@@ -175,5 +177,10 @@
  "loc.messages.RestartedAppService": "App Service '%s' restarted successfully.",
  "loc.messages.RestartedAppServiceSlot": "App Service '%s-%s' restarted successfully.",
  "loc.messages.FailedToRestartAppService": "Failed to restart App Service '%s'. Error: %s",
-  "loc.messages.FailedToRestartAppServiceSlot": "Failed to restart App Service '%s-%s'. Error: %s"
+  "loc.messages.FailedToRestartAppServiceSlot": "Failed to restart App Service '%s-%s'. Error: %s",
+  "loc.messages.FailedToGetConfigurationFile": "Failed to get configuration file for multi containers. Please enter the valid configuration file path.",


Failed to locate the Docker-Compose file which is required for deploying multi-container apps. Please enter a valid Docker-Compose file path.

It is done.

azooinmyluggage · 2019-04-09T08:31:02Z

Tasks/AzureWebAppContainerV1/task.json

            "label": "Image name",
            "defaultValue": "",
            "required": true,
            "helpMarkDown": "A globally unique top-level domain name for your specific registry or namespace.<br/> Note: Fully qualified image name will be of the format: '<b>`<registry or namespace`></b>/`<repository`>:`<tag`>'. For example, '<b>myregistry.azurecr.io</b>/nginx:latest'."
        },
+        {
+            "name": "configFilePath",


Rename it to multicontainerConfigFile

azooinmyluggage · 2019-04-09T08:31:36Z

Tasks/AzureWebAppContainerV1/task.json

            "label": "Image name",
            "defaultValue": "",
            "required": true,
            "helpMarkDown": "A globally unique top-level domain name for your specific registry or namespace.<br/> Note: Fully qualified image name will be of the format: '<b>`<registry or namespace`></b>/`<repository`>:`<tag`>'. For example, '<b>myregistry.azurecr.io</b>/nginx:latest'."
        },
+        {
+            "name": "configFilePath",


multicontainerConfigFile is inline with the az cli parameter

az webapp config container set –resource-group [resource group] –name [app name] –multicontainer-config-type “compose” –multicontainer-config-file

azooinmyluggage · 2019-04-09T08:33:23Z

Tasks/AzureWebAppContainerV1/task.json

+            "label": "Configuration File",
+            "defaultValue": "",
+            "required": false,
+            "helpMarkDown": "Path of the configuration. Should be fully qualified path or relative to the default working directory."


Path of the Docker-Compose file. Should be a fully qualified path or relative to the default working directory.

It is done.

azooinmyluggage · 2019-04-09T08:37:00Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

-  "loc.messages.FailedToRestartAppServiceSlot": "Failed to restart App Service '%s-%s'. Error: %s"
+  "loc.messages.FailedToRestartAppServiceSlot": "Failed to restart App Service '%s-%s'. Error: %s",
+  "loc.messages.FailedToGetConfigurationFile": "Failed to get configuration file for multi containers. Please enter the valid configuration file path.",
+  "loc.messages.FailedToDeployToWebApp": "No deployment can happen to the webapp '%s'. For single-container, provide a valid image name and for multi-container, config file is mandatory and image names can also be provided if you want tags to be substituted.",


Deployment to the webapp '%s' failed. For single-container, just specify a valid image name. For multi-container specifying a Docker-Compose file is mandatory and specifying image names is optional. Provided images names if the tags in Docker-Compose file need to be substituted.

It is done.

azooinmyluggage · 2019-04-09T08:37:46Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

+  "loc.messages.FailedToRestartAppServiceSlot": "Failed to restart App Service '%s-%s'. Error: %s",
+  "loc.messages.FailedToGetConfigurationFile": "Failed to get configuration file for multi containers. Please enter the valid configuration file path.",
+  "loc.messages.FailedToDeployToWebApp": "No deployment can happen to the webapp '%s'. For single-container, provide a valid image name and for multi-container, config file is mandatory and image names can also be provided if you want tags to be substituted.",
+  "loc.messages.SingleContainerDeployment": "Single Container Deployment to the webapp '%s' because only image name is given",


Single-container Deployment to the webapp '%s' as only the image detail was sepcified.

It is done.

azooinmyluggage · 2019-04-09T08:38:57Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

+  "loc.messages.FailedToGetConfigurationFile": "Failed to get configuration file for multi containers. Please enter the valid configuration file path.",
+  "loc.messages.FailedToDeployToWebApp": "No deployment can happen to the webapp '%s'. For single-container, provide a valid image name and for multi-container, config file is mandatory and image names can also be provided if you want tags to be substituted.",
+  "loc.messages.SingleContainerDeployment": "Single Container Deployment to the webapp '%s' because only image name is given",
+  "loc.messages.MultiContainerDeploymentWithTransformation": "Multi Container Deployment to the webapp '%s' with transformation of configuration file because both image name and configuration file are given",


"Multi-container deployment to the webapp '%s' with the transformation of Docker-Compose file as both image name and Docker-Compose file was specified"

It is done.

azooinmyluggage · 2019-04-09T08:39:49Z

Tasks/AzureWebAppContainerV1/Strings/resources.resjson/en-US/resources.resjson

+  "loc.messages.FailedToDeployToWebApp": "No deployment can happen to the webapp '%s'. For single-container, provide a valid image name and for multi-container, config file is mandatory and image names can also be provided if you want tags to be substituted.",
+  "loc.messages.SingleContainerDeployment": "Single Container Deployment to the webapp '%s' because only image name is given",
+  "loc.messages.MultiContainerDeploymentWithTransformation": "Multi Container Deployment to the webapp '%s' with transformation of configuration file because both image name and configuration file are given",
+  "loc.messages.MultiContainerDeploymentWithoutTransformation": "Multi Container Deployment to the webapp '%s' without transformation of configuration file because only configuration file is given"


"Multi-container deployment to the webapp '%s' without transformation of Docker-Compose file because only Docker-Compose file was specified."

It is done.

SumiranAgg · 2019-04-12T08:02:59Z

Tasks/Common/AzureRmDeploy-common/operations/ContainerBasedDeploymentUtility.ts

+        let updatedConfigFilePath: string = configFilePath;
+
+        if(imageName) {
+            tl.debug("Deploying image " + imageName + " to the webapp " + this._appService.getName());


This statment is correct for single container deployment but for multi-container mention config file name instead.

SumiranAgg

Please fix one comment I recently provided before merging

Shivangi added 2 commits March 25, 2019 12:58

Multiple images loading

a20c1f6

publishing log file

0cfef74

20shivangi requested review from kmkumaran, SumiranAgg and vincent1173 as code owners March 27, 2019 05:22

Adding few more changes

eb1afaa

vincent1173 changed the title ~~Users/shmittal/multiple containers~~ Azure web app - support multicontainer Mar 29, 2019

vincent1173 added Area: Release Area: AzureAppService Label to monitor Azure App Service issues labels Mar 29, 2019

Changing patch version to pass the checks

a6f6acc

vincent1173 reviewed Mar 29, 2019

View reviewed changes

Resolving comments

6600994

vincent1173 approved these changes Mar 29, 2019

View reviewed changes

Tasks/AzureWebAppContainerV1/taskparameters.ts Outdated Show resolved Hide resolved

Shivangi added 2 commits March 29, 2019 15:02

Resolving more comments

4e54323

Passing the checks

ba89d0f

SumiranAgg reviewed Apr 1, 2019

View reviewed changes

Resolving comments

3ef5435

vincent1173 self-requested a review April 2, 2019 10:26

Making imageName not mandatory

0a079c2

azooinmyluggage reviewed Apr 9, 2019

View reviewed changes

Shivangi added 5 commits April 9, 2019 15:49

Editing tooltips

a8fabeb

Resolved merge conflicts

bf2d6aa

Changing tooltip of imageName

8d9596c

Merge branch 'master' into users/shmittal/multipleContainers

5fb01a6

git commit -m "resolving checks"

0a919b7

SumiranAgg reviewed Apr 12, 2019

View reviewed changes

SumiranAgg approved these changes Apr 12, 2019

View reviewed changes

Resolving last comment

93f1341

20shivangi merged commit 815f926 into master Apr 12, 2019

Azure web app - support multicontainer #9943

Azure web app - support multicontainer #9943

Conversation

20shivangi commented Mar 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SumiranAgg left a comment

Choose a reason for hiding this comment