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

Containerapps 4/11 Release [ Supporting MSI for ACA & some bug fixes] #4660

Merged
merged 9 commits into from
Apr 12, 2022

Conversation

calvinsID
Copy link
Contributor

@calvinsID calvinsID commented Apr 9, 2022

  • P1 & P2 fixes
  • MSI feature

* Skeleton code

* az containerapp env show

* List kube/managed environments

* Create kube environment, wait doesn't work yet

* Update containerapp stubs (check if it is supported now)

* Containerapp env delete, polling not working yet

* Added polling for create and delete

* Use Microsoft.App RP for show, list, delete command

* Create containerapp env using Microsoft.App RP

* Add optional containerapp env create arguments

* Remove old kube environment code, naming fixes

* Containerapp create almost done

* Done containerapp create, except for --yaml. Need to test

* Containerapp show, list

* Fix helptext

* Containerapp delete

* Containerapp update. Needs secrets api to be implemented, and testing

* Add scale command

* Various validations, small fixes

* listSecrets API for updates, autogen log analytics for env

* Use space delimiter for secrets and env variables

* Verify sub is registered to Microsoft.ContainerRegistration if creating vnet enabled env, remove logs-type parameter

* Containerapp create --yaml

* Fix updating registry to do create or update

* Fix containerapp update command. Add image-name parameter to support multi container updates. Fix updating registries, containers and secrets

* started update with --yaml. Need to do create or update for when an attribute is a list of items

* use space delimiter for startup_command and args, instead of comma delimiter

* Traffic weights

* List and show revisions

* az containerapp revision restart, activate, deactivate

* Add ability for users to clear args/command in az containerapp update

* Various fixes, traffic weights fixes

* Verify subnet subscription is registered to Microsoft.ContainerServices

* GitHub Actions Update (#17)

* Added models. Finished transferring Calvin's previous work.

* Updated wrong models.

* Updated models in custom.py, added githubactionclient.

* Updated envelope to be correct.

* Small bug fixes.

* Updated error handling. Fixed bugs. Initial working state.

* Added better error handling.

* Added error messages for tokens with inappropriate access rights.

* Added back get_acr_cred.

* Fixed problems from merge conflict.

* Updated names of imports from ._models.py to fix pylance erros.

* Removed random imports.

Co-authored-by: Haroon Feisal <[email protected]>

* Remove --location since location must be same as managed env

* Add options for flag names: --env-vars and --registry-srever

* Empty string to clear env_vars

* Default revisions_mode to single

* Infer acr credentials if it is acr and credentials are not provided

* fix help msg

* if image is hosted on acr, and no registry server is supplied, infer the registry server

* Added subgroups (Ingress, Registry, Secret) and updated revisions (#18)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Modified update_containerapp_yaml to support updating from non-current revision.

Authored-by: Haroon Feisal <[email protected]>

* More p0 fixes (#20)

* Remove --registry-login-server, only allow --registry-server

* Rename --environment-variables to --env-vars

* If no image is supplied, use default quickstart image

* Update help text (#21)

* Update help text

* Update punctuation

* master -> main

* New 1.0.1 version

* Added identity commands + --assign-identity flag to containerapp create (#8)

* Added identity show and assign.

* Finisheed identity remove.

* Added helps, updated identity remove to work with identity names instead of requiring identity resource ids.

* Moved helper function to utils.

* Require --identities flag when removing identities.

* Added message for assign identity with no specified identity.

* Added --assign-identity flag to containerapp create.

* Moved assign-identity flag to containerapp create.

* Fixed small logic error on remove identities when passing duplicate identities. Added warnings for certain edge cases.

* Updated param definition for identity assign --identity default.

* Added identity examples in help.

* Made sure secrets were not removed when assigning identities. Added tolerance for [system] passed with capital letters.

* Fixed error from merge.

Co-authored-by: Haroon Feisal <[email protected]>

* Dapr Commands (#23)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Added dapr enable and dapr disable. Need to test more.

* Added list, show, set dapr component. Added dapr enable, disable.

* Added delete dapr delete.

* Added helps and param text.

* Changed dapr delete to dapr remove to match with dapr set.

* Commented out managed identity for whl file.

* Uncommented.

Co-authored-by: Haroon Feisal <[email protected]>

* Rename --image-name to --container-name

* Remove allowInsecure since it was messing with the api parsing

* Fix for env var being empty string

* Rename to --dapr-instrumentation-key, only infer ACR credentials if --registry-server is provided

* Remove az containerapp scale

* Fix delete containerapp errors

* Remove ingress, dapr flags from az containerapp update/revision copy

* Fix revision list -o table

* Help text fix

* Bump extension to 0.1.2

* Update managed identities and Dapr help text (#25)

* Update managed identities and Dapr help text

* Update Dapr flags

* Add secretref note

* Env var options + various bug fixes (#26)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

Co-authored-by: Haroon Feisal <[email protected]>

* Fixed style issues, various bug fixes (#27)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

Co-authored-by: Haroon Feisal <[email protected]>

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_scenario.py

Co-authored-by: Xing Zhou <[email protected]>

* Specific Error Types + Bugfixes (Help, remove app-subnet-resource-id, removed env-var alias, added help text for --name) (#28)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

* Update helps for identity, revision. Removed env-var alias for set-env-vars. Added name param help.

* Removed app-subnet-resource-id.

* Updated infrastructure subnet param help.

* Check if containerapp resource exists before attempting to delete.

* Added check before deleting managed env.

* Changed error types to be more specific.

* Removed check before deletion. Removed comments.

Co-authored-by: Haroon Feisal <[email protected]>

* Reset to 0.1.0 version, remove unneeded options-list

* Update min cli core version

* Fixed style issues. (#30)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix linter issues

* Use custom-show-command

* Removed --ids from revision, secret, registry list.

* Add linter exclusions

* Fix polling on delete containerapp

* Fix error handling

* Add Container App Service

* Fix flake linter

* Fix help text

* Mark extension as preview

* Add python 3.9 and 3.10 as supported

* Remove registries and secrets from az containerapp update, in favor of registry and secret subgroup

* Fix YAML not working

* Move import to inside deserialize function

* Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing.

* Fixed dapr in create.

* Revert "Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing."

This reverts commit 51bc543.

* Revert "Fixed dapr in create."

This reverts commit 37030ad.

* Ingress enable --transport default. Secret list returns empty array. Secret update prints message saying user needs to restart their apps. Added show-values flag to secret list. Fixed yaml datetime field issues, replaced x00 values that also came up during testing.

* Skeleton code

* az containerapp env show

* List kube/managed environments

* Create kube environment, wait doesn't work yet

* Update containerapp stubs (check if it is supported now)

* Containerapp env delete, polling not working yet

* Added polling for create and delete

* Use Microsoft.App RP for show, list, delete command

* Create containerapp env using Microsoft.App RP

* Add optional containerapp env create arguments

* Remove old kube environment code, naming fixes

* Containerapp create almost done

* Done containerapp create, except for --yaml. Need to test

* Containerapp show, list

* Fix helptext

* Containerapp delete

* Containerapp update. Needs secrets api to be implemented, and testing

* Add scale command

* Various validations, small fixes

* listSecrets API for updates, autogen log analytics for env

* Use space delimiter for secrets and env variables

* Verify sub is registered to Microsoft.ContainerRegistration if creating vnet enabled env, remove logs-type parameter

* Containerapp create --yaml

* Fix updating registry to do create or update

* Fix containerapp update command. Add image-name parameter to support multi container updates. Fix updating registries, containers and secrets

* started update with --yaml. Need to do create or update for when an attribute is a list of items

* use space delimiter for startup_command and args, instead of comma delimiter

* Traffic weights

* List and show revisions

* az containerapp revision restart, activate, deactivate

* Add ability for users to clear args/command in az containerapp update

* Various fixes, traffic weights fixes

* Verify subnet subscription is registered to Microsoft.ContainerServices

* GitHub Actions Update (#17)

* Added models. Finished transferring Calvin's previous work.

* Updated wrong models.

* Updated models in custom.py, added githubactionclient.

* Updated envelope to be correct.

* Small bug fixes.

* Updated error handling. Fixed bugs. Initial working state.

* Added better error handling.

* Added error messages for tokens with inappropriate access rights.

* Added back get_acr_cred.

* Fixed problems from merge conflict.

* Updated names of imports from ._models.py to fix pylance erros.

* Removed random imports.

Co-authored-by: Haroon Feisal <[email protected]>

* Remove --location since location must be same as managed env

* Add options for flag names: --env-vars and --registry-srever

* Empty string to clear env_vars

* Default revisions_mode to single

* Infer acr credentials if it is acr and credentials are not provided

* fix help msg

* if image is hosted on acr, and no registry server is supplied, infer the registry server

* Added subgroups (Ingress, Registry, Secret) and updated revisions (#18)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Modified update_containerapp_yaml to support updating from non-current revision.

Authored-by: Haroon Feisal <[email protected]>

* More p0 fixes (#20)

* Remove --registry-login-server, only allow --registry-server

* Rename --environment-variables to --env-vars

* If no image is supplied, use default quickstart image

* Update help text (#21)

* Update help text

* Update punctuation

* master -> main

* New 1.0.1 version

* Added identity commands + --assign-identity flag to containerapp create (#8)

* Added identity show and assign.

* Finisheed identity remove.

* Added helps, updated identity remove to work with identity names instead of requiring identity resource ids.

* Moved helper function to utils.

* Require --identities flag when removing identities.

* Added message for assign identity with no specified identity.

* Added --assign-identity flag to containerapp create.

* Moved assign-identity flag to containerapp create.

* Fixed small logic error on remove identities when passing duplicate identities. Added warnings for certain edge cases.

* Updated param definition for identity assign --identity default.

* Added identity examples in help.

* Made sure secrets were not removed when assigning identities. Added tolerance for [system] passed with capital letters.

* Fixed error from merge.

Co-authored-by: Haroon Feisal <[email protected]>

* Dapr Commands (#23)

* Added ingress subgroup.

* Added help for ingress.

* Fixed ingress traffic help.

* Added registry commands.

* Updated registry remove util to clear secrets if none remaining. Added warning when updating existing registry. Added registry help.

* Changed registry delete to remove.

* Added error message if user tries to remove non assigned registry.

* Changed registry add back to registry set.

* Added secret subgroup commands.

* Removed yaml support from secret set.

* Changed secret add to secret set. Updated consistency between secret set and secret delete. Added secret help. Require at least one secret passed with --secrets for secret commands.

* Changed param name for secret delete from --secrets to --secret-names. Updated help.

* Changed registry remove to registry delete.

* Fixed bug in registry delete.

* Added revision mode set and revision copy.

* Added dapr enable and dapr disable. Need to test more.

* Added list, show, set dapr component. Added dapr enable, disable.

* Added delete dapr delete.

* Added helps and param text.

* Changed dapr delete to dapr remove to match with dapr set.

* Commented out managed identity for whl file.

* Uncommented.

Co-authored-by: Haroon Feisal <[email protected]>

* Rename --image-name to --container-name

* Remove allowInsecure since it was messing with the api parsing

* Fix for env var being empty string

* Rename to --dapr-instrumentation-key, only infer ACR credentials if --registry-server is provided

* Remove az containerapp scale

* Fix delete containerapp errors

* Remove ingress, dapr flags from az containerapp update/revision copy

* Fix revision list -o table

* Help text fix

* Bump extension to 0.1.2

* Update managed identities and Dapr help text (#25)

* Update managed identities and Dapr help text

* Update Dapr flags

* Add secretref note

* Env var options + various bug fixes (#26)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

Co-authored-by: Haroon Feisal <[email protected]>

* Fixed style issues, various bug fixes (#27)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

Co-authored-by: Haroon Feisal <[email protected]>

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_scenario.py

Co-authored-by: Xing Zhou <[email protected]>

* Specific Error Types + Bugfixes (Help, remove app-subnet-resource-id, removed env-var alias, added help text for --name) (#28)

* Moved dapr arguments to env as a subgroup.

* Added env variable options.

* Changed revision mode set to revision set-mode.

* Added env var options to revision copy.

* Fixed revision copy bug related to env secret refs.

* Changed registry and secret delete to remove. Added registry param helps. Removed replica from table output and added trafficWeight.

* Updating warning text.

* Updated warning text once more.

* Made name optional for revision copy if from-revision flag is passed.

* Fixed whitespace style issues.

* Styled clients and utils to pass pylint.

* Finished client.py pylint fixes.

* Fixed pylint issues.

* Fixed flake8 commands and custom.

* Fixed flake issues in src.

* Added license header to _sdk_models.

* Added confirmation for containerapp delete.

* Update helps for identity, revision. Removed env-var alias for set-env-vars. Added name param help.

* Removed app-subnet-resource-id.

* Updated infrastructure subnet param help.

* Check if containerapp resource exists before attempting to delete.

* Added check before deleting managed env.

* Changed error types to be more specific.

* Removed check before deletion. Removed comments.

Co-authored-by: Haroon Feisal <[email protected]>

* Reset to 0.1.0 version, remove unneeded options-list

* Update min cli core version

* Fixed style issues. (#30)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix linter issues

* Use custom-show-command

* Removed --ids from revision, secret, registry list.

* Add linter exclusions

* Fix polling on delete containerapp

* Fix error handling

* Add Container App Service

* Fix flake linter

* Fix help text

* Mark extension as preview

* Add python 3.9 and 3.10 as supported

* Remove registries and secrets from az containerapp update, in favor of registry and secret subgroup

* Fix YAML not working

* Move import to inside deserialize function

* Dapr moved from Template to Configuration

* Use aka.ms link for containerapps yaml

* Updated dapr enable/disable to current spec.

* Fixed oversight.

* Remove revisions-mode from containerapp update

* Fixed dapr enable property names. (#47)

Co-authored-by: Haroon Feisal <[email protected]>

* Fix exceptions with using --yaml in containerapp create/update

* Rename history msg

* Include fqdn in containerapp table output

* Added ingress messages.

* Revert history msg

* Reduced redundant code between revision copy and containerapp update.

* Fixed merge issues.

* Fixed merge conflicts, moved helper function

Co-authored-by: Calvin Chan <[email protected]>
Co-authored-by: Haroon Feisal <[email protected]>
Co-authored-by: Anthony Chu <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
@yonzhan
Copy link
Collaborator

yonzhan commented Apr 9, 2022

Containerapps

@panchagnula
Copy link
Contributor

@zhoxing-ms can we get your review on this as well. We want to release this version on 4/11 (PST morning is better). Thanks!

Copy link
Contributor

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

Some generic help changes (not updated in this PR)
text: |
az containerapp env create -n MyContainerappEnvironment -g MyResourceGroup \
--location "Canada Central"
Is there a command to get the list of regions supported on Container apps? If so I would not use an location value here as See: command to get list of supported locations. If not change this to East US2 (Canada Central has some capacity constraints I believe)

In general, avoid using "Canada Central" across help examples.

@panchagnula
Copy link
Contributor

@haroonf FYI, incase you plan to make the changes here before the Calvin can get to it, Thanks!

@panchagnula
Copy link
Contributor

@calvinsID , @haroonf have you tried this example as is on your individual tests? Mainly this line can cause confusion - since based on a shell this can be parsed differently --secrets mysecret=secretvalue1 anothersecret="secret value 2"

I would expect this to be --secrets "sec1=val1 sec2='spaced val2'" Ex. see this https://docs.microsoft.com/en-us/cli/azure/use-cli-effectively#use-quotation-marks-in-arguments. In general I would remove this example from our help, since most users will try to copy paste a help command & expect it to work.

az containerapp create -n MyContainerapp -g MyResourceGroup \
--image my-app:v1.0 --environment MyContainerappEnv \
--secrets mysecret=secretvalue1 anothersecret="secret value 2" \
--env-vars GREETING="Hello, world" SECRETENV=secretref:anothersecret

@panchagnula
Copy link
Contributor

All the examples for --location has the "region name" instead of location. I assume we support both on the API, for consistency purposes use the location name instead of region name (avoiding the need to quotations around the region name) Ex. az group create --name RGlocalContext --location westeurope

Copy link
Contributor

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

@calvinsID let discuss these YAML specific changes on Monday, while we wait for review from @zhoxing-ms as well & target releasing this 4/11 evening PST. I will make sure to work with @zhoxing-ms

@panchagnula
Copy link
Contributor

@calvinsID we also need to update the version on this extension (please do a major version update because the argument vs YAML is a breaking change behavior)

@panchagnula
Copy link
Contributor

Lets make the non warning logger.warning - logger.info & add the name of the resource i.e instead of saying updating app say updating app "xyz".

@zhoxing-ms
Copy link
Contributor

Could you add some tests for those changes?

@panchagnula
Copy link
Contributor

Could you add some tests for those changes?

@zhoxing-ms we are reverting some major changes & will add tests as part of upcoming changes. Thanks!

@panchagnula
Copy link
Contributor

Lets make the non warning logger.warning - logger.info & add the name of the resource i.e instead of saying updating app say updating app "xyz".

Will create a bug to track this later.

@panchagnula
Copy link
Contributor

@calvinsID & @haroonf looks good. Just a couple more minor active comments plus please update the version. Thanks!

@panchagnula panchagnula changed the title Containerapps 4/11 Release Containerapps 4/11 Release [ Supporting MSI for ACA & some bug fixes] Apr 11, 2022
calvinsID and others added 3 commits April 11, 2022 14:53
* Added more specific MSI help text.

* Updated help text.

Co-authored-by: Haroon Feisal <[email protected]>
@zhoxing-ms zhoxing-ms merged commit 7c06c70 into Azure:main Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants