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

[ContainerApp] Added support for Cloud Patching #7571

Open

harryli0108 wants to merge 12 commits into Azure:main from harryli0108:harrli/cloud-patch-cli

Member

harryli0108 commented May 2, 2024 •

edited

Loading

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
My extension version conforms to the Extension version schema

For new extensions:

My extension description/summary conforms to the Extension Summary Guidelines.

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.


          Added support for cloud patching

7cdadb6

azure-client-tools-bot-prd bot commented May 2, 2024 •

edited

Loading

⚠️Azure CLI Extensions Breaking Change Test

⚠️containerapp

rule cmd_name rule_message suggest_message

⚠️ 1006 - ParaAdd containerapp patch apply cmd containerapp patch apply added parameter container_app_name

⚠️ 1006 - ParaAdd containerapp patch apply cmd containerapp patch apply added parameter patch_name

⚠️ 1011 - SubgroupAdd containerapp patch configure sub group containerapp patch configure added

⚠️ 1001 - CmdAdd containerapp patch delete cmd containerapp patch delete added

⚠️ 1006 - ParaAdd containerapp patch list cmd containerapp patch list added parameter container_app_name

⚠️ 1001 - CmdAdd containerapp patch show cmd containerapp patch show added

azure-client-tools-bot-prd bot added this to the May 2024 (2024-05-21)❗ Breaking change release ❗ milestone

azure-client-tools-bot-prd bot commented May 2, 2024

Hi @harryli0108,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

Collaborator

yonzhan commented May 2, 2024

ContainerApp

microsoft-github-policy-service bot requested review from yonzhan, wangzelin007, yanzhudd, zhoxing-ms, jsntcy, ruslany, sanchitmehta, ebencarek, JennyLawrance, howang-ms, vinisoto, chinadragon0515, vturecek, torosent, pagariyaalok, Juliehzl, jijohn14 and Greedygre

May 2, 2024 08:50

microsoft-github-policy-service bot added the Auto-Assign label

microsoft-github-policy-service bot assigned zhoxing-ms

microsoft-github-policy-service bot added the ContainerApp label

github-actions bot commented May 2, 2024 •

edited

Loading

⚠️ Release Suggestions

Module: containerapp

Update VERSION to 1.0.0b1 in src/containerapp/setup.py

Notes

Stable/preview tag is inherited from last release. If needed, please add stable/preview label to modify it.
Major/minor/patch/pre increment of version number is calculated by pull request code changes automatically. If needed, please add major/minor/patch/pre label to adjust it.
For more info about extension versioning, please refer to Extension version schema


          Pipline errors

812b5d6

Moazzem-Hossain-pixel approved these changes

View reviewed changes

Moazzem-Hossain-pixel left a comment

Please take the necessary steps.

Moazzem-Hossain-pixel suggested changes

View reviewed changes

Moazzem-Hossain-pixel left a comment

Help to resolve

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

                   return update_managed_environment(cmd, env_name, resource_group_name, workload_profile_type=workload_profile_type, workload_profile_name=workload_profile_name, min_nodes=min_nodes, max_nodes=max_nodes)
-              def patch_list(cmd, resource_group_name=None, managed_env=None, show_all=False):
+              def patch_list(cmd, resource_group_name=None, managed_env=None, container_app_name=None, show_all=False):
+                  from azure.cli.command_modules.containerapp._utils import format_location

Contributor

snehapar9 May 14, 2024

Move this import statement to the top of this file

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

                   return results
+              def patch_show(cmd, resource_group_name=None, container_app_name=None, patch_name=None):
+                  from azure.cli.command_modules.containerapp._utils import format_location

Contributor

snehapar9 May 14, 2024

Move this import statement to the top and re-use across methods.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if format_location(app["location"]) != format_location("North Central US (Stage)"):
+                      logger.warning("Cloud patching is not supported in the location of the container app.")
+                      return
+                  from ._clients import PatchClient

Contributor

snehapar9 May 14, 2024

Same comment - Move all imports to the top.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py



		def patch_delete(cmd, resource_group_name=None, container_app_name=None, patch_name=None):
		from azure.cli.command_modules.containerapp._utils import format_location

Contributor

snehapar9 May 14, 2024

Same comment about moving this import statement to the top

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  from azure.cli.command_modules.containerapp._utils import format_location
+                  logger.warning("This command is currently only available for container app cloud patches in North Central US (Stage).")
+                  if patch_name is None:
+                      logger.error("Please provide the name of the patch to delete.")

Contributor

snehapar9 May 14, 2024

This should throw a validation error instead of just logging as an error?

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if format_location(app["location"]) != format_location("North Central US (Stage)"):
+                      logger.warning("Cloud patching is not supported in the location of the container app.")
+                      return
+                  from ._clients import PatchClient

Contributor

snehapar9 May 14, 2024

same comment about import statement

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if app is None:
+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))
+                      return
+                  if format_location(app["location"]) != format_location("North Central US (Stage)"):

Contributor

snehapar9 May 14, 2024

use safe_get instead of just app["location"]

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py



		def patch_mode_configure(cmd, resource_group_name=None, container_app_name=None, patch_mode=None):
		from azure.cli.command_modules.containerapp._utils import format_location

Contributor

snehapar9 May 14, 2024

Move import

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if patch_mode not in ["Automatic", "Manual", "Disabled"]:
+                      logger.error("Invalid patch mode provided. Please provide 'Automatic', 'Manual', or 'Disabled'.")
+                      return
+                  from ._clients import PatchClient

Contributor

snehapar9 May 14, 2024

Move imort

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

                   patch_apply_handle_input(cmd, patchable_check_results, "y", pack_exec_path)
+              def use_cloud_patch(cmd, container_app_name, resource_group_name, patch_name):
+                  try:
+                      app = show_containerapp(cmd, container_app_name, resource_group_name)

Contributor

snehapar9 May 14, 2024

Will this throw a not found exception if container app does not exist? Probably need to handle it separately? by check if not found exception is thrown in a separate method and return None if it is

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if app is None:
+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))
+                      return
+                  if app["location"] == "North Central US (Stage)":

Contributor

snehapar9 May 14, 2024 •

edited

Loading

Use safe_get() here for getting the app's location and let's use STAGE_LOCATION from the constants file instead of hardcoding it here.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                      patch_client = PatchClient()
+                      is_patch_success = patch_client.apply(cmd, resource_group_name, container_app_name, patch_name)
+                      if is_patch_success:
+                          print("Patch {0} for container app {1} is queued successfully to be applied.".format(patch_name, container_app_name))

Contributor

snehapar9 May 14, 2024

logger.warning if we want this to show up on cx console?

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if app["location"] == "North Central US (Stage)":
+                      logger.warning("Using cloud patching...")
+                      logger.warning("Cloud patching is only supported in North Central US (Stage) now.")
+                      from ._clients import PatchClient

Contributor

snehapar9 May 14, 2024

Move import.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))
+                      return
+                  if app["location"] == "North Central US (Stage)":
+                      logger.warning("Using cloud patching...")

Contributor

snehapar9 May 14, 2024

Let's combine this into a single line.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  patch_client = PatchClient()
+                  is_patch_mode_configured = patch_client.patch_mode_configure(cmd, resource_group_name, container_app_name, patch_mode)
+                  if is_patch_mode_configured:
+                      print("Patch mode for container app {0} is set to {1}.".format(container_app_name, patch_mode))

Contributor

snehapar9 May 14, 2024

logger.warning if we want this to show up on the console.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                      logger.error("Failed to fetch container app {0} in resource group {1}. Error: {2}".format(container_app_name, resource_group_name, str(e)))
+                      return
+                  if app is None:
+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))

Contributor

snehapar9 May 14, 2024 •

edited

Loading

Change to Container App

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  patch_client = PatchClient()
+                  is_patch_deleted = patch_client.delete(cmd, resource_group_name, container_app_name, patch_name)
+                  if is_patch_deleted:
+                      print("Patch {0} for container app {1} is deleted successfully.".format(patch_name, container_app_name))

Contributor

snehapar9 May 14, 2024

logger.warning instead of print

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if app is None:
+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))
+                      return
+                  if format_location(app["location"]) != format_location("North Central US (Stage)"):

Contributor

snehapar9 May 14, 2024

Use safe_get()

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                      logger.error("Please provide the name of the patch to delete.")
+                      return
+                  try:
+                      app = show_containerapp(cmd, container_app_name, resource_group_name)

Contributor

snehapar9 May 14, 2024

This could throw a not found exception if container app does not exist.

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/custom.py

+                  if app is None:
+                      logger.error("Container app {0} not found in resource group {1}.".format(container_app_name, resource_group_name))
+                      return
+                  if format_location(app["location"]) != format_location("North Central US (Stage)"):

Contributor

snehapar9 May 14, 2024

Use safe_get() to get app's location

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/_help.py

+              helps["containerapp patch configure mode"] = """
+                  type: command
+                  short-summary: Configure the patching mode for a container app.

Contributor

snehapar9 May 14, 2024

Container App instead of container app

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/_help.py

@@ @@ -847,6 +871,20 @@ @@
                         az containerapp patch interactive -g MyResourceGroup --environment MyContainerAppEnv --show-all
               """
+              helps["containerapp patch configure"] = """
+                  type: group
+                  short-summary: Commands to configure the patching settings for a container app.

Contributor

snehapar9 May 14, 2024

Replace container app by Container App

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/_help.py

@@ @@ -827,6 +830,27 @@ @@
                   - name: List patchable and unpatchable container apps by managed environment with the show-all option and apply patch for patchable container apps.
                     text: |
                         az containerapp patch apply -g MyResourceGroup --environment MyContainerAppEnv --show-all
+                  - name: Apply a patch for a container app by patch name using Cloud Patching.

Contributor

snehapar9 May 14, 2024

Container App

snehapar9 reviewed

View reviewed changes

src/containerapp/azext_containerapp/_help.py

@@ @@ -809,6 +809,9 @@ @@
                   - name: List patchable and unpatchable container apps by managed environment with the show-all option.
                     text: |
                         az containerapp patch list -g MyResourceGroup --environment MyContainerAppEnv --show-all
+                  - name: List available patches for a container app.

Contributor

snehapar9 May 14, 2024

Container App


          Merge branch 'main' into harrli/cloud-patch-cli

f068700

MoazzemHossain-bot suggested changes

View reviewed changes

MoazzemHossain-bot left a comment

Pls do as well

harryli0108 and others added 4 commits

May 16, 2024 12:35


          Merge branch 'main' into harrli/cloud-patch-cli

0aef23b


          Resolve CI failure

670b342


          Merge branch 'main' into harrli/cloud-patch-cli

b7544e2


          Changed history.rst

07de3ad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Greedygre Greedygre left review comments

snehapar9 snehapar9 left review comments

MoazzemHossain-bot MoazzemHossain-bot requested changes

Moazzem-Hossain-pixel Moazzem-Hossain-pixel approved these changes

yonzhan Awaiting requested review from yonzhan

wangzelin007 Awaiting requested review from wangzelin007

yanzhudd Awaiting requested review from yanzhudd

zhoxing-ms Awaiting requested review from zhoxing-ms

jsntcy Awaiting requested review from jsntcy

ruslany Awaiting requested review from ruslany

sanchitmehta Awaiting requested review from sanchitmehta

ebencarek Awaiting requested review from ebencarek

JennyLawrance Awaiting requested review from JennyLawrance

howang-ms Awaiting requested review from howang-ms

vinisoto Awaiting requested review from vinisoto

chinadragon0515 Awaiting requested review from chinadragon0515

vturecek Awaiting requested review from vturecek

torosent Awaiting requested review from torosent

pagariyaalok Awaiting requested review from pagariyaalok

Juliehzl Awaiting requested review from Juliehzl

jijohn14 Awaiting requested review from jijohn14

At least 1 approving review is required to merge this pull request.

Labels

Auto-Assign ContainerApp