Feature aca maven plugin #2501

RuoyuWang-MS · 2024-10-31T06:33:29Z

What does this implement/fix? Explain your changes.

…

Implement aca maven plugin
Support 3 paths:
deploy image
deploy code
deploy artifact

Have breaking change in the azure-toolkit-containerapp-lib:
Will use generate dockerfile and use ACR to build if dockerfile doesn't exist instead of oryx. The intellij plugin can reuse this logic later.

Does this close any currently open issues?

Any relevant logs, screenshots, error output, etc.?

Any other comments?

…

Has this been tested?

Tested

* init aca maven plugin * remove useless items * fix comments

* init aca maven plugin * remove useless items * init acr build * reuse timestamp * fix comments * add todo

* init aca maven plugin * remove useless items * init acr build * reuse timestamp * fix comments * add todo * support generate dockerfile * remove useless comments * fix comments

* init aca maven plugin * remove useless items * init acr build * reuse timestamp * fix comments * add todo * support generate dockerfile * remove useless comments * add unit tests and remove todo * merge fix comments * rename folder * fix * fix pom * small fix * small word fix

allxiao · 2024-11-01T05:49:41Z

azure-container-apps-maven-plugin/pom.xml

+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-plugin-plugin</artifactId>
+                <configuration>
+                    <goalPrefix>aca</goalPrefix>


As discussed offline, let's remove this short prefix for now according to: Azure Container Apps branding guidelines

allxiao · 2024-11-01T05:54:06Z

...iner-apps-maven-plugin/src/main/java/com/microsoft/azure/maven/containerapps/DeployMojo.java

+        if (MavenConfigUtils.isJarPackaging(project)) {
+            return true;
+        } else if (MavenConfigUtils.isPomPackaging(project)) {
+            log.info(PROJECT_SKIP);


It's not a common practice to extract the log contents to constants, especially when they are not reused. Suggest to use plain text here and for the other 3 log constants.

allxiao · 2024-11-01T05:56:38Z

...maven-plugin-lib/src/main/resources/bundles/com/microsoft/azure/toolkit/operation.properties

@@ -8,13 +8,15 @@ user/functionapp.package=generate configuration files and prepare staging direct
 user/functionapp.run=run function app locally
 user/webapp.deploy_app=deploy to Azure Web App with resource creation or updating
 user/webapp.config=generate configuration for web app maven plugin
+user/containerapps.deploy_mojo=deploy Container app to Azure from config


When referring to a single Container Apps instance, use lower case singular container app

allxiao · 2024-11-01T05:57:04Z

...maven-plugin-lib/src/main/resources/bundles/com/microsoft/azure/toolkit/operation.properties

 ######################end mojo action operations######################
 ######################start auto action operations######################
 auto/$resource.preload.type=preload {0}s
 auto/$resource.refresh_on_subscription_changed.type=refresh {0}s on subscription changed
 ######################end auto action operations######################
 ######################start common service operations######################
 internal/account.login.type=login with ({0})
+internal/containerapps.create_update_app.app=create or update Container app({0}) from config


container app for single instance reference

* use mariner image * ifx * fix comments * use java 17 as default * modify dockerfile comments

allxiao · 2024-11-01T06:11:16Z

...src/main/java/com/microsoft/azure/toolkit/lib/containerapps/task/DeployContainerAppTask.java

+    }
+
+    private void preCheck(ContainerAppsEnvironmentConfig environmentConfig) {
+        Optional.ofNullable(environmentConfig).map(ContainerAppsEnvironmentConfig::getSubscriptionId).filter(StringUtils::isNotBlank).orElseThrow(() -> new AzureToolkitRuntimeException("'subscriptionId' is required"));


Extract the common check to a method, and avoid repeated, too long lines.

<T> String checkNotBlank(T config, Function<T, String> getter, String name) { if (config != null) { String value = getter.apply(config); if (StringUtils.isNotBlank(value)) { return value; } } throw new AzureToolkitRuntimeException(String.format("'%s' is required", name)); }

Call with

checkNotBlank(environmentConfig, ContainerAppsEnvironmentConfig::getSubscriptionId, "subscriptionId");

allxiao · 2024-11-01T06:13:50Z

...src/main/java/com/microsoft/azure/toolkit/lib/containerapps/task/DeployContainerAppTask.java

+            .getOrDraft(config.getResourceGroup(), config.getResourceGroup());
+
+        if (resourceGroup.isDraftForCreating() && !resourceGroup.exists()) {
+            final String region = Optional.ofNullable(config.getRegion()).filter(StringUtils::isNotBlank).orElseThrow(() -> new AzureToolkitRuntimeException("'region' is required"));


same here, to use common check.

Flanker32 · 2024-11-03T07:18:32Z

...iner-apps-maven-plugin/src/main/java/com/microsoft/azure/maven/containerapps/DeployMojo.java

+     * Boolean flag to control whether to wait the deployment status to be ready after deployment
+     */
+    @Parameter(property = "noWait")
+    private Boolean noWait;


Did we use the two parameters anywhere else? Shall we remove them?

Flanker32 · 2024-11-03T07:35:32Z

...-maven-plugin/src/main/java/com/microsoft/azure/maven/containerapps/parser/ConfigParser.java

+            return null;
+        }
+        final String defaultImageName = String.format("%s%s/%s:%s", config.getRegistryConfig().getRegistryName(), ContainerRegistry.ACR_IMAGE_SUFFIX, mojo.getAppName(), timestamp);
+        final String fullImageName = Optional.ofNullable(containers.get(0).getImage()).orElse(defaultImageName);


It seems that we only accept the first container configuration, if so, why not change mojo.getContainers to AppContainerMavenConfig instead of a List?

Here we will spike if we should support multi container payload later. For the first edition, we can just keep the list for future scalability

Flanker32 · 2024-11-03T07:35:47Z

...-maven-plugin/src/main/java/com/microsoft/azure/maven/containerapps/parser/ConfigParser.java

+            return null;
+        }
+        final ResourceConfiguration resourceConfiguration = new ResourceConfiguration();
+        resourceConfiguration.setCpu(containers.get(0).getCpu());


Same as above

Flanker32 · 2024-11-03T07:37:10Z

.../main/java/com/microsoft/azure/toolkit/lib/containerapps/containerapp/ContainerAppDraft.java

+            .map(action -> action.bind("https://aka.ms/azure-container-apps-java"))
+            .orElse(null);
+
+        final Action<String> openPortal = Optional.ofNullable(AzureActionManager.getInstance().getAction(Action.OPEN_URL).withLabel("Open Portal"))


Before code is to open the portal page of this container app, do we need to change it to open portal instead? Besides, we should read the portal url from AzureEnvironment

Revert the URL. Sorry for the mistake.

Flanker32 · 2024-11-03T13:39:55Z

@wangmingliang-ms Please help check the logic changes in aca deployments~

wangmingliang-ms · 2024-11-04T08:25:44Z

.../main/java/com/microsoft/azure/toolkit/lib/containerapps/containerapp/ContainerAppDraft.java

+            .orElse(null);
+
+        final Action<String> openPortal = Optional.ofNullable(AzureActionManager.getInstance().getAction(Action.OPEN_URL).withLabel("Open Portal"))
+            .map(action -> action.bind("https://portal.azure.com"))


any reason to change the url?

this action is to open the portal page of this specific resource.

https://portal.azure.com is not the right url of all Azure Environments' portal pages.

Revert the URL. Sorry for the mistake.

wangmingliang-ms · 2024-11-04T08:27:24Z

.../main/java/com/microsoft/azure/toolkit/lib/containerapps/containerapp/ContainerAppDraft.java

-        final Action<String> learnMore = AzureActionManager.getInstance().getAction(Action.OPEN_URL).withLabel("Learn More").bind("https://aka.ms/azuretools-aca-stack");
-        final Action<String> openPortal = AzureActionManager.getInstance().getAction(Action.OPEN_URL).withLabel("Open Portal").bind(this.getPortalUrl());
+        final Action<String> learnMore = Optional.ofNullable(AzureActionManager.getInstance().getAction(Action.OPEN_URL).withLabel("Learn More"))
+            .map(action -> action.bind("https://aka.ms/azure-container-apps-java"))


any reason to change this url?

Revert the URL. Sorry for the mistake.

wangmingliang-ms · 2024-11-04T08:36:25Z

...kit-libs/azure-toolkit-containerapps-lib/src/main/resources/template/aca/artifact-dockerfile

@@ -0,0 +1,5 @@
+ARG JAVA_VERSION=17


what if user's project is developped upon Java 21?

It will use Java 21 and 17 is a default version if no java version detected.
Here we detect the java version and add it into buildImageConfig.sourceBuildEnv (we just reuse this map). code link
And then we put this into ACR build args code link
These build args will override the JAVA_VERSION=17 in the dockerfile.

wangmingliang-ms

LGTM

Flanker32 · 2024-11-06T02:53:13Z

...en-plugin/src/test/java/com/microsoft/azure/maven/containerapps/parser/ConfigParserTest.java

@@ -128,7 +128,8 @@ public void testGetContainerAppConfigImage() {
    }

    @Test
-    public void testGetContainerAppConfigCode() {
+    public void


Flanker32

LGTM

Flanker32

LGTM

RuoyuWang-MS added 4 commits October 23, 2024 13:54

Init aca maven plugin (#2493)

6dfc328

* init aca maven plugin * remove useless items * fix comments

Support code build path with dockerfile (#2495)

48dc430

* init aca maven plugin * remove useless items * init acr build * reuse timestamp * fix comments * add todo

Support generating dockerfile for code/artifact path (#2497)

74f8e97

* init aca maven plugin * remove useless items * init acr build * reuse timestamp * fix comments * add todo * support generate dockerfile * remove useless comments * fix comments

RuoyuWang-MS changed the title ~~Feature aca maven~~ Feature aca maven plugin Oct 31, 2024

allxiao reviewed Nov 1, 2024

View reviewed changes

use mariner image (#2502)

ff2a97a

* use mariner image * ifx * fix comments * use java 17 as default * modify dockerfile comments

allxiao reviewed Nov 1, 2024

View reviewed changes

fix comments

79872af

Flanker32 reviewed Nov 3, 2024

View reviewed changes

Flanker32 requested a review from wangmingliang-ms November 3, 2024 13:40

wangmingliang-ms requested changes Nov 4, 2024

View reviewed changes

fix comments

714739b

RuoyuWang-MS requested review from wangmingliang-ms and Flanker32 November 5, 2024 03:05

wangmingliang-ms previously approved these changes Nov 5, 2024

View reviewed changes

small fix for the deployment type case sensitivity issue

2b91535

RuoyuWang-MS dismissed wangmingliang-ms’s stale review via 2b91535 November 5, 2024 07:56

RuoyuWang-MS added 2 commits November 5, 2024 17:01

fix pom for tests

ca2a361

Merge remote-tracking branch 'origin/develop' into feature-aca-maven

518c738

Flanker32 reviewed Nov 6, 2024

View reviewed changes

Flanker32 previously approved these changes Nov 6, 2024

View reviewed changes

fix typo

262714b

RuoyuWang-MS dismissed Flanker32’s stale review via 262714b November 6, 2024 02:58

Flanker32 approved these changes Nov 6, 2024

View reviewed changes

RuoyuWang-MS merged commit f33a173 into develop Nov 6, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature aca maven plugin #2501

Feature aca maven plugin #2501

RuoyuWang-MS commented Oct 31, 2024

allxiao Nov 1, 2024 •

edited

Loading

allxiao Nov 1, 2024

allxiao Nov 1, 2024

allxiao Nov 1, 2024

allxiao Nov 1, 2024 •

edited

Loading

allxiao Nov 1, 2024

Flanker32 Nov 3, 2024

RuoyuWang-MS Nov 5, 2024

Flanker32 Nov 3, 2024

RuoyuWang-MS Nov 5, 2024

Flanker32 Nov 3, 2024

Flanker32 Nov 3, 2024

RuoyuWang-MS Nov 5, 2024

Flanker32 commented Nov 3, 2024

wangmingliang-ms Nov 4, 2024

RuoyuWang-MS Nov 5, 2024

wangmingliang-ms Nov 4, 2024

RuoyuWang-MS Nov 5, 2024

wangmingliang-ms Nov 4, 2024

RuoyuWang-MS Nov 5, 2024

wangmingliang-ms left a comment

Flanker32 Nov 6, 2024

Flanker32 left a comment

Flanker32 left a comment

Feature aca maven plugin #2501

Feature aca maven plugin #2501

Conversation

RuoyuWang-MS commented Oct 31, 2024

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any relevant logs, screenshots, error output, etc.?

Any other comments?

Has this been tested?

allxiao Nov 1, 2024 • edited Loading

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

allxiao Nov 1, 2024 • edited Loading

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

Flanker32 commented Nov 3, 2024

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

wangmingliang-ms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Flanker32 left a comment

Choose a reason for hiding this comment

Flanker32 left a comment

Choose a reason for hiding this comment

allxiao Nov 1, 2024 •

edited

Loading

allxiao Nov 1, 2024 •

edited

Loading