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.

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

[containerapp] az containerapp add-on create: Support add-on commands; deprecation of service commands #6960

Merged

Conversation

bgashirabake
Copy link
Contributor


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)

For new extensions:

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.

Copy link

azure-client-tools-bot-prd bot commented Nov 7, 2023

⚠️Azure CLI Extensions Breaking Change Test
⚠️containerapp
rule cmd_name rule_message suggest_message
⚠️ 1011 - SubgroupAdd containerapp add-on sub group containerapp add-on added

Copy link

Hi @bgashirabake,
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.

Copy link

Hi @bgashirabake,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@yonzhan
Copy link
Collaborator

yonzhan commented Nov 7, 2023

containerapp

Copy link
Contributor

Thank you for your contribution bgashirabake! We will review the pull request and get back to you soon.

@Greedygre
Copy link
Contributor

Please not mark test to live_only, for error log workspace api-version does not correct, please pull the newest code in azure-cli, the setup again with command:
azdev setup --cli path-for\azure-cli --repo path-for\azure-cli-extensions
:

except CannotOverwriteExistingCassetteException as ex:
>           raise AssertionError(ex)
E           AssertionError: Can't overwrite existing cassette ('/mnt/vss/_work/1/s/src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_dev_service_binding_e2e.yaml') in your current record mode ('once').
E           No match for the request (<Request (PUT) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2022-10-01>](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2022-10-01%3E)) was found.
E           Found 1 similar requests with 1 different matcher(s) :
E           
E           1 - (<Request (PUT) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2021-12-01-preview>).](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2021-12-01-preview%3E).)
E           Matchers succeeded : ['method', 'scheme', 'host', 'port', 'path']
E           Matchers failed :
E           _custom_request_query_matcher - assertion failure :
E           None

@bgashirabake
Copy link
Contributor Author

bgashirabake commented Nov 8, 2023

Please not mark test to live_only, for error log workspace api-version does not correct, please pull the newest code in azure-cli, the setup again with command: azdev setup --cli path-for\azure-cli --repo path-for\azure-cli-extensions :

except CannotOverwriteExistingCassetteException as ex:
>           raise AssertionError(ex)
E           AssertionError: Can't overwrite existing cassette ('/mnt/vss/_work/1/s/src/containerapp/azext_containerapp/tests/latest/recordings/test_containerapp_dev_service_binding_e2e.yaml') in your current record mode ('once').
E           No match for the request (<Request (PUT) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2022-10-01>](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2022-10-01%3E)) was found.
E           Found 1 similar requests with 1 different matcher(s) :
E           
E           1 - (<Request (PUT) [https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2021-12-01-preview>).](https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/clitest.rg000001/providers/Microsoft.OperationalInsights/workspaces/containerapp-env000004?api-version=2021-12-01-preview%3E).)
E           Matchers succeeded : ['method', 'scheme', 'host', 'port', 'path']
E           Matchers failed :
E           _custom_request_query_matcher - assertion failure :
E           None

yea the cli-repo re-setup was a blindspot. synced and all's back to recording state!

@bgashirabake
Copy link
Contributor Author

ready for review cc: @tawalke @Greedygre @yanzhudd . thank you all!!

ADDON_LIST = ["redis", "postgres", "kafka", "mariadb", "qdrant"]
create_containerapp_env(self, env_name, resource_group)

self.cmd('containerapp addon redis create -g {} -n {} --environment {}'.format(
Copy link
Contributor

Choose a reason for hiding this comment

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

please check the provisiongState for result. Same as blow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the provisioningState of all addons is checked in the for loop

Greedygre

This comment was marked as outdated.

@Greedygre
Copy link
Contributor

@Juliehzl Could you help to review this PR?

@bgashirabake bgashirabake force-pushed the user/bgashirabake/service-to-addon branch from 85127b1 to 2f4d7f1 Compare November 8, 2023 21:06
g.custom_command('list', 'list_all_services')

with self.command_group('containerapp service redis') as g:
with self.command_group('containerapp addon', is_preview=True) as g:
Copy link
Contributor

Choose a reason for hiding this comment

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

what using containerapp add-on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, i don't get your question...help explain more?

Copy link
Contributor Author

@bgashirabake bgashirabake Nov 9, 2023

Choose a reason for hiding this comment

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

but by the section that you commented on, the addon command group is the equivalent of the service one that's right above it

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is should we use az containerapp add-on instead of using az containerapp addon ? @bgashirabake

Copy link
Contributor Author

@bgashirabake bgashirabake Nov 9, 2023

Choose a reason for hiding this comment

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

oh got it got it. in discussions with @SimonJakesch, addon was what was decided on, but i just checked the docs and add-on is what's being referenced. i can push the change for add-on in the morning but if release time is before then, i say we go with addon. the change is straight-forward so should be seamless if made in time

@bgashirabake bgashirabake changed the title [containerapp] az containerapp addon create: Support deprecation of service command in favor of addon [containerapp] az containerapp add-on create: Support add-on commands; deprecation of service commands Nov 9, 2023
@Greedygre
Copy link
Contributor

Hi @zhoxing-ms

Could you please help to review this PR? Thanks.

@bgashirabake
Copy link
Contributor Author

Hi @zhoxing-ms Xing Zhou FTE

Could you please help to review this PR? Thanks.

+

@zhoxing-ms zhoxing-ms merged commit fbb8641 into Azure:main Nov 10, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot ContainerApp customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants