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.

Sign up for GitHub

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

Jump to bottom

Add microservice quick start template for Azure Spring Apps #13532

Merged
merged 16 commits into from
Sep 1, 2023
Merged

Add microservice quick start template for Azure Spring Apps #13532

merged 16 commits into from
Sep 1, 2023

Conversation

moarychan
Copy link
Member

@moarychan moarychan commented Aug 25, 2023 •
edited
Loading

PR Checklist

Check these items before submitting a PR...

Contribution Guide

Best Practice Guide

  • - Please check this box once you've submitted the PR if you've read through the Contribution Guide and best practices checklist.

Fixes Azure/Azure-Spring-Apps#28

Changelog

  • Remove the outdated quick start template quickstarts/microsoft.appplatform/azure-spring-cloud.
  • Add a quick start template for PetClinic sample deployment, targeted to the Azure Spring Apps enterprise plan.

Try this template in another branch:

Deploy To Azure

@moarychan moarychan marked this pull request as ready for review August 28, 2023 09:43
@moarychan moarychan mentioned this pull request Aug 28, 2023
Merged
@moarychan
Copy link
Member Author

Below are two best practices that can not be applied:

  • Template Should Not Contain Blanks - Empty property: {} Line: 196, Column: 28 
    Required property 'properties' not found in JSON. Path '', line 1, position 2. (Code: BadArgument, Target: properties)
  • IDs Should Be Derived From ResourceIDs - Property: "buildResultId" must use one of the following expressions for an resourceId property 
    Invalid arguments: properties.source.buildResultId should not be null or empty. (Code: BadArgument)

saragluna
saragluna suggested changes Aug 29, 2023
View reviewed changes
quickstarts/microsoft.appplatform/microservice-apps-enterprise-plan/README.md
languages:
- json
---
# Deploy a simple Azure Spring Apps microservice application

![Azure Public Test Date](https://azurequickstartsservice.blob.core.windows.net/badges/quickstarts/microsoft.appplatform/azure-spring-cloud/PublicLastTestDate.svg)
![Azure Public Test Result](https://azurequickstartsservice.blob.core.windows.net/badges/quickstarts/microsoft.appplatform/azure-spring-cloud/PublicDeployment.svg)
![Azure Public Test Date](https://azurequickstartsservice.blob.core.windows.net/badges/quickstarts/microsoft.appplatform/microservice-apps-enterprise-plan/PublicLastTestDate.svg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better if we call this folder microsoft.appplatform/azure-spring-apps/?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a common name, and we should add more quickstart template here.

quickstarts/microsoft.appplatform/microservice-apps-enterprise-plan/README.md Outdated Show resolved Hide resolved
...kstarts/microsoft.appplatform/microservice-apps-enterprise-plan/scripts/deploy-jar-to-asa.sh
@@ -0,0 +1,47 @@
#!/bin/bash

set -Eeuo pipefail
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this script for the enterprise plan?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, thought this is another file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the default builder will use jar to build and get the build id for deployment.

@moarychan
Copy link
Member Author

Hi @alex-frankel , could you help review this PR? Many thanks!

@alex-frankel
Copy link
Contributor

Hi @moarychan - we cannot review any PRs until all of the CI tests are passing.

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts azure-quickstarts removed the readme violations README violations during PR label Aug 30, 2023
@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@moarychan moarychan marked this pull request as draft August 31, 2023 03:36
@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@moarychan moarychan marked this pull request as ready for review August 31, 2023 05:32
@moarychan
Copy link
Member Author

moarychan commented Aug 31, 2023 •
edited
Loading

Hi @alex-frankel , could you help confirm the below best practice test case? I would like to make a clarification to see if this check can be skipped; if so, please help to review this PR.

The Microsoft.AppPlatform/Spring/apps/deployments's property properties.source.buildResultId can not be analyzed correctly. The actual value looks like [concat(resourceId('Microsoft.AppPlatform/Spring/buildServices/builds', variables('asaInstanceName'), 'default', concat(variables('adminServerAppName'), '-default')), '/results/1')], the default value is <default>, which will point to the latest build, such as xx/build/2, xxx/build/3.

The arm-ttk always says

IDs Should Be Derived From ResourceIDs - Property: "buildResultId" must use one of the following expressions for an resourceId property:            
extensionResourceId,resourceId,subscriptionResourceId,tenantResourceId,if,parameters,reference,variables,subscription,guid

Currently, the ARM template is not convenient for deploying source code, so we prepared the deployment script to use Azure CLI instead of the ARM template deployment ability. It has to configure <default>, because the template doesn't define the resource Microsoft.AppPlatform/Spring/buildServices/builds.

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@azure-quickstarts
Copy link
Collaborator

@moarychan - check this PR for updates that may be needed to documentation that references this sample. [This is an automated message. You are receiving it because you are listed as the docOwner in metadata.json.]

@alex-frankel
Copy link
Contributor

The arm-ttk checks are optional. Just need to make sure the quickstart deploys successfully, so this can be merged.

@alex-frankel alex-frankel merged commit 4604cc4 into Azure:master Sep 1, 2023
3 checks passed
@moarychan moarychan deleted the moary/add-microservice-quickstart-template branch September 4, 2023 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saragluna saragluna saragluna requested changes

Assignees
No one assigned
Labels
best practices violations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Azure Quickstart Templates for ASA to be provided
4 participants
@moarychan @alex-frankel @azure-quickstarts @saragluna