Azure Functions maven plugin quarkized #31780

patriot1burke · 2023-03-11T01:55:02Z

Quarkus implementation for the azure-functions-maven-plugin

Added quarkus:run capabilities.
Added quarkus:deploy capabilities to make it more generic. Kept backward compatible with kubernetes deploy. Kubernetes needs to be refactored to use new generic deploy mechanism IMO.

devtools/maven/src/main/java/io/quarkus/maven/RunMojo.java

maxandersen · 2023-03-23T14:01:51Z

devtools/maven/src/main/java/io/quarkus/maven/RunMojo.java

+import io.quarkus.deployment.run.RunCommandLaunchResultBuildItem;
+
+@Mojo(name = "run")
+public class RunMojo extends QuarkusBootstrapMojo {


mvn quarkus:run fails with:Error: Unable to access jarfile /Users/manderse/temp/myj/target/quarkus-app/quarkus-run.jar

can we make it require project build first or at least tell the user to have it built?

Wouldn't this require changes to pom.xml

maxandersen · 2023-03-23T14:03:15Z

devtools/maven/src/main/java/io/quarkus/maven/RunMojo.java

+    }
+
+    @Override
+    protected void doExecute() throws MojoExecutionException, MojoFailureException {


the execution have what looks like a copy of all system properties on it. is that intended?

-Djava.specification.version=17 -Dsun.jnu.encoding=UTF-8 -Djava.class.path=/Users/manderse/temp/myj/.mvn/wrapper/maven-wrapper.jar -Djava.vm.vendor=GraalVM Community -Dsun.arch.data.model=64 -Djava.vendor.url=https://www.graalvm.org/ -Duser.timezone=Europe/Copenhagen -Dmaven.conf=/Users/manderse/.m2/wrapper/dists/apache-maven-3.8.7-bin/678cc9d4/apache-maven-3.8.7/conf -Djava.vm.specification.version=17 -Dos.name=Mac OS X -Dsun.java.launcher=SUN_STANDARD -Duser.country=DK -Dsun.boot.library.path=/Users/manderse/.sdkman/candidates/java/22.3.r17-grl/lib -Dsun.java.command=org.apache.maven.wrapper.MavenWrapperMain -f /Users/manderse/temp/myj/pom.xml verify -DskipTests quarkus:run -Dhttp.nonProxyHosts=local|*.local|169.254/16|*.169.254/16 -Djdk.debug=release -Dmaven.home=/Users/manderse/.m2/wrapper/dists/apache-maven-3.8.7-bin/678cc9d4/apache-maven-3.8.7 -Dsun.cpu.endian=little -Duser.home=/Users/manderse -Duser.language=en -Dsun.stderr.encoding=UTF-8 -Djava.specification.vendor=Oracle Corporation -Djava.version.date=2022-10-18 -Djava.home=/Users/manderse/.sdkman/candidates/java/22.3.r17-grl -Dfile.separator=/ -Djava.vm.compressedOopsMode=Zero based -Djdk.internal.vm.ci.enabled=true -Dline.separator= -Dsun.stdout.encoding=UTF-8 -Djava.vm.specification.vendor=Oracle Corporation -Djava.specification.name=Java Platform API Specification -Dapple.awt.application.name=MavenWrapperMain -Dsun.management.compiler=HotSpot 64-Bit Tiered Compilers -Dftp.nonProxyHosts=local|*.local|169.254/16|*.169.254/16 -Djava.runtime.version=17.0.5+8-jvmci-22.3-b08 -Duser.name=manderse -Dpath.separator=: -Dos.version=13.2.1 -Djava.runtime.name=OpenJDK Runtime Environment -Dfile.encoding=UTF-8 -Dguice.disable.misplaced.annotation.check=true -Djava.vm.name=OpenJDK 64-Bit Server VM -DskipTests=true -Djava.vendor.version=GraalVM CE 22.3.0 -Djava.vendor.url.bug=https://github.com/oracle/graal/issues -Djava.io.tmpdir=/var/folders/mm/z7zzmyl15bd8byr8vsdxf1740000gn/T/ -Djava.version=17.0.5 -Duser.dir=/Users/manderse/temp/myj -Dos.arch=aarch64 -Dmaven.multiModuleProjectDirectory=/Users/manderse/temp/myj -Djava.vm.specification.name=Java Virtual Machine Specification -Dnative.encoding=UTF-8 -Djava.library.path=/Users/manderse/Library/Java/Extensions:/Library/Java/Extensions:/Network/Library/Java/Extensions:/System/Library/Java/Extensions:/usr/lib/java:. -Djava.vm.info=mixed mode, sharing -Djava.vendor=GraalVM Community -Dvertx.disableTCCL=true -Djava.vm.version=17.0.5+8-jvmci-22.3-b08 -Dclassworlds.conf=/Users/manderse/.m2/wrapper/dists/apache-maven-3.8.7-bin/678cc9d4/apache-maven-3.8.7/bin/m2.conf -Dsun.io.unicode.encoding=UnicodeBig -DsocksNonProxyHosts=local|*.local|169.254/16|*.169.254/16 -Djava.class.version=61.0 -jar /Users/manderse/temp/myj/target/quarkus-app/quarkus-run.jar

?

Do you think its a good idea to have a separate switch for passing sys props to spawned process?

github-actions · 2023-05-18T16:44:27Z

🙈 The PR is closed and the preview is expired.

patriot1burke · 2023-05-22T14:22:07Z

@maxandersen @iocanel Finally finished everything would love to get a review.

"Deploy" should be backward compatible. A separate PR should be done to get the Kubernetes extensions deploy to work similarly to Azure Functions.
This is all looking very nice, ordered, and simple. Azure functions requires no special build plugins now. :) Will be nice to get Lambda working in the same way.

devtools/maven/src/main/java/io/quarkus/maven/AbstractDeploymentMojo.java

iocanel · 2023-05-23T10:07:37Z

I went through the code changes and I really like where its going. Especially the part with the deployment command declaration.
I want to checkout the PR and try it locally before I finalize my review.
Hopefully, later today or tommorow.

devtools/maven/src/main/java/io/quarkus/maven/DeployMojo.java

iocanel · 2023-05-24T10:42:21Z

devtools/maven/src/main/java/io/quarkus/maven/DeployMojo.java

+                        "Too many installed extensions support quarkus:deploy.  You must choose one by setting quarkus.deploy.target.");
+                getLog().error("Extensions: " + targets.stream().collect(Collectors.joining(" ")));
+            } else if (target != null && !targets.contains(target)) {
+                getLog().error(


In this case, we would like a matching extension (if found) to be added as a forcedDependency.

For example in any quarkus project the following should always work:

mvn quarkus:deploy -Dquarkus.deploy.target=knative mvn quarkus:deploy -Dquarkus.deploy.target=openshift

Besides the hard coding approach which is what was originally used, I can think of two approaches that could possibly work:

Express deployment capabilites in the extension metadata and perform a registry lookup.

Use a convention like adding quarkus-${quarkus.deploy.target| to forced dependencies.

Option 1: has the downsides that its slow.
Option 2: is not the most reliable (e.g. in the existing codebase knative would still not work as there is no quarkus-knative extension).

Forcing dependencies, IMO, is a mediocre solution, at best, when using CLI, but I don't like it at all from the mvn/gradle command line. Forcing dependencies doesn't even work with Gradle, correct? For Lambda and Azure Functions there are packaging requirements for quarkus:run and quarkus:deploy to work, so, forcing dependencies makes no sense for those plugins.

Any specific hardcoding should be removed from the maven/gradle code base, and only CLI should be able to force dependencies, IMO. I would even go as far to say that forced dependencies should be removed as a solution as it doesn't work for Gradle at all. I don't think its a burden to force users to pull in the dependency.

Also, the kube extensions were not meant to use the quarkus.deploy.target until they are refactored to use the new deployment mechanism.

Forcing dependencies doesn't even work with Gradle, correct?

Forcing dependecnies from within a gradle task doesn't work. However, when using the CLI we can force dependencies for gradle project by generating a gralde init script. This is something that we already do and it works fine.

Any specific hardcoding should be removed from the maven/gradle code base,

I agree that we can remove hardcoding from maven/gralde and what you've put there is a great step towards this goal.

I don't think its a burden to force users to pull in the dependency.

No, it is not. But then again, the kubernetes bits do not affect runtime, so there is not reason for them to be added to the project. And given that different people within a team may be using different tools (especially for the container image bits and local kube development like quarkus-minikube and quarkus-kind) it can cause uneeded noise git.

I don't want to refactor the Kube extensions within this PR please. Can we just get this approved and merged? Is it not backward compatible as-is right now?

I am not asking you to refactor the kube extensions. I am just trying to understand what you have in mind.

@patriot1burke, @maxandersen : How would you like us to proceed? Given that the code doesn't break compatibility I am ok with this PR.

We can deal with quarkus.deploy.target=xxxx vs quarkus.xxxx.deploy=true in a follow up as long as we agree that we will somehow retain a way of forcing dependencies. What do you think?

Will try again tomorrow after my talk.

Up to you on work. I can make the change to quarkus.xxx.deploy -> quarkus.deploy.target if you want. Forced dependencies should be done before lookup of deploy options too. (if it isn't already)

Hmmm, I don't think that we can completely remove quarkus.xxx.deploy at this point. At best we could deprecate it.

iocanel

Pull request looks good to me.
Furhter aligning and improvement can be done in a follow up PR.

integration testing finish quarkus:run and integration tests quarkus:run gradle cli run command implement deploy afk rebase' azf deploy azf deploy working azf more fixes azf service-principal azf fix codestarts and path config azf fix codestarts and path config azf gradle and codestart azf new docs fix pom azf docs and imports fix gradle quarkusRun

quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven area/testing labels Mar 11, 2023