Azure / azure-cli-extensions

Public Repository for Extensions of Azure CLI.
https://docs.microsoft.com/en-us/cli/azure
MIT License
380 stars 1.19k forks source link

az containerapp update : tcp support #6002

Closed merou closed 1 year ago

merou commented 1 year ago

Description

When enabling Tcp transport and exposedPort in yaml deployment file - exposedPort is ignored.

Command Name az containerapp update Extension Name: containerapp. Version: 0.3.23.

Errors:

n/a

To Reproduce:

...
...
    ingress:
      allowInsecure: false
      customDomains: null
      exposedPort: 9115
      external: true
      fqdn: xxxxxxx.wonderfulmeadow-91556248.westeurope.azurecontainerapps.io
      ipSecurityRestrictions: null
      targetPort: 9115
      traffic:
      - latestRevision: true
        weight: 100
      transport: Tcp
...
...

Expected Behavior

exposedPort is set and visible in Azure Portal.

Environment Summary

Linux-5.15.0-67-generic-x86_64-with-glibc2.31, Ubuntu 20.04.4 LTS
Python 3.10.10
Installer: DEB

azure-cli 2.46.0

Extensions:
containerapp 0.3.23
resource-graph 2.1.0
log-analytics 0.2.2
storage-preview 0.8.3
cloud-service 0.2.0
azure-devops 0.26.0

Dependencies:
msal 1.20.0
azure-mgmt-resource 21.1.0b1

Additional Context

yonzhan commented 1 year ago

route to CXP team

zioproto commented 1 year ago

Hello @merou

TCP ingress is in public preview and is only supported in Container Apps environments that use a custom VNET.

Can you confirm you were using a customer VNET in your test ?

The feature seems to be implemented since version 0.3.12, and merged in this PR: https://github.com/Azure/azure-cli-extensions/pull/5392

thanks

zioproto commented 1 year ago

@merou why you have Tcp with the capital T ?

From the docs should be tcp https://learn.microsoft.com/en-us/azure/container-apps/ingress?tabs=bash#configuration

Having the wrong capitalization is a problem because exposed_port is used only for transport == "tcp" https://github.com/Azure/azure-cli-extensions/blob/de78ea0f46f1ef43e83ce571289d90db1acd18dc/src/containerapp/azext_containerapp/custom.py#L1844

navba-MSFT commented 1 year ago

@merou Apologies for the late reply. Thanks for reaching out to us and reporting this issue. Please let us know if you had a chance to look at the suggestions in the above comments.

zioproto commented 1 year ago

@navba-MSFT

It is very confusing how in the command line the customer should use --transport tcp but in the values returned by the API it is returned Tcp.

https://github.com/Azure/azure-cli-extensions/blob/15174230f10b5f3ce5f998e0146cf043caac8af9/src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py#L405-L412

What is the correct value to specify in the yaml file then ? tcp or Tcp ?

navba-MSFT commented 1 year ago

@zioproto I just looked at the REST API Specs for the Ingress transportproperty and it expects the enum value to be tcp.

https://github.com/Azure/azure-rest-api-specs/blob/main/specification/app/resource-manager/Microsoft.App/stable/2022-10-01/ContainerApps.json#L736

merou commented 1 year ago

Apologies I was away. I also tried tcp but I get the same result, it is a custom vnet.

Is it possible to have a validator before submitting to API, e.g:

$ az containerapp validate --yaml deployment.yml
navba-MSFT commented 1 year ago

@merou Thanks for your reply. Could you please run the same CLI command again with--debug switch and share the detailed output here ? Awaiting your reply.

merou commented 1 year ago

@navba-MSFT I cannot submit the whole debug - but I noticed exposedPort had a value of 0 in the API response. In the yaml file I have:

exposedPort: 9115

I also tried to quote it - in your code format is int32 so it should work without.

"ingress":{"fqdn":"xxxxxxxxx.westeurope.azurecontainerapps.io",
"external":true,"targetPort":9115,"exposedPort":0,"transport":"Tcp","traffic":[{"weight":100,"latestRevision":true}],
"customDomains":null,"allowInsecure":false,"ipSecurityRestrictions":null,"corsPolicy":null,"clientCertificateMode":null}
,"registries":[{"server":"xxxxxx.azurecr.io","username":"nmbrs","passwordSecretRef":"xxxxxx","identity":""}],"dapr":null,"maxInactiveRevisions":null},
"template":{"revisionSuffix":"","containers":[{"image":"xxxxxxx/monitoring/blackbox-exporter:latest","name":"blackbox-exporter","resources":{"cpu":1.0,"memory":"2Gi","ephemeralStorage":"4Gi"}}],"initContainers":null,"scale":{"minReplicas":1,"maxReplicas":10,"rules":null},"volumes":null},
"eventStreamEndpoint":"https://westeurope.azurecontainerapps.dev/subscriptions/xxxxxxx/resourceGroups/rg-aca-dev/containerApps/blackbox-exporter/eventstream"},"identity":{"type":"None"}}
\ Running ..cli.azure.cli.core.util: Found subscription ID xxxxxxxxxx in the URL https://management.azure.com/subscriptions/xxxxxxxxxx/resourceGroups/rg-aca-dev/providers/Microsoft.App/containerApps/blackbox-exporter?api-version=2022-10-01
zioproto commented 1 year ago

In the above comment in the yaml file you have Tcp with the capital letter, correct ?

merou commented 1 year ago

correct but also tried tcp and same output

merou commented 1 year ago

I also did the following test:

Problem remains.

navba-MSFT commented 1 year ago

@merou I have discussed this issue internally with our Product owners. They have confirmed that the exposedPortis not supported in Azure CLI and ContainerApp PS cmdlet. We will support it soon. Until then you could leverage the below workaround:

You can use the following CLI command as alternative solution:

• Fix Exposed port via Azure Portal • Export yaml current state:

$ az rest -u /subscriptions//resourceGroups//providers/Microsoft.App/containerApps/?api-version=2022-10-01 > deployment.json

• Revert change in Azure Portal • Push generated json:

$ az rest -u /subscriptions//resourceGroups//providers/Microsoft.App/containerApps/?api-version=2022-10-01 -m PATCH -b @deployment.json

navba-MSFT commented 1 year ago

@merou I hope you had a chance to look at my above comment. The above workaround should be followed until the support is provided for the exposedPort property. If you need any further assistance on this issue in future, please feel free to reopen this thread. We would be happy to help.

zioproto commented 1 year ago

How do we track when the feature is released ? Should we keep this GitHub issue open until the release ?

merou commented 1 year ago

Apologizes @navba-MSFT I am not tracking those notifications via email - thank you for the workaround but for us, it is not that urgent ATM - I will wait for the fix

navba-MSFT commented 1 year ago

How do we track when the feature is released ? Should we keep this GitHub issue open until the release ?

@zioproto @merou I will update this thread once the fix is released.