Azure / azure-cli-extensions

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

containerapp: CLI parameters should override yaml API spec file #5636

Open zioproto opened 1 year ago

zioproto commented 1 year ago

Related command

az containerapp env create \
  --name $CONTAINERAPPS_ENVIRONMENT \
  --resource-group $RESOURCE_GROUP \
  --location $LOCATION

az containerapp create --yaml echoserver.yaml

Where echoserver.yaml is:

---
kind: containerapp
location: eastus
name: echoserver
resourceGroup: aca
type: Microsoft.App/containerApps
tags:
  tagname: value
properties:
  managedEnvironmentId: /subscriptions/12345678-0abc-defg-hijk-1234567890/resourceGroups/aca/providers/Microsoft.App/managedEnvironments/myaca
  configuration:
    activeRevisionsMode: Multiple
    ingress:
      external: true
      allowInsecure: false
      targetPort: 8080
      traffic:
        - latestRevision: true
          weight: 100
      transport: auto
  template:
    revisionSuffix: myrevision
    containers:
      - image: gcr.io/google_containers/echoserver:1.10
        name: echoserver
        env:
          - name: HTTP_PORT
            value: 8080
        resources:
          cpu: 0.5
          memory: 1Gi
        probes:
          - type: liveness
            httpGet:
              path: "/health"
              port: 8080
              httpHeaders:
                - name: "Custom-Header"
                  value: "liveness probe"
            initialDelaySeconds: 7
            periodSeconds: 3
          - type: readiness
            tcpSocket:
              port: 8080
            initialDelaySeconds: 10
            periodSeconds: 3
          - type: startup
            httpGet:
              path: "/startup"
              port: 8080
              httpHeaders:
                - name: "Custom-Header"
                  value: "startup probe"
            initialDelaySeconds: 3
            periodSeconds: 3
    scale:
      minReplicas: 1
      maxReplicas: 3

Extension name (the extension in question)

containerapp

Description of issue (in as much detail as possible)

I tried to create a "yaml" to share on GitHub to create a sample Azure Container App.

I was expecting this to work because I had the value managedEnvironmentId and the name information in the file, but with my surprise the CLI wanted again the name and resource group.

I understand this is documented, name and resource group must be passed as CLI arguments:

https://learn.microsoft.com/en-us/cli/azure/containerapp?view=azure-cli-latest#az-containerapp-create-required-parameters

Then I run:

az containerapp create -n echoserver -g aca --yaml echoserver.yaml

And it worked, but with my surprise the name passed in the CLI is mandatory but ignored:

Example:

az containerapp create -n null -g aca --yaml echoserver.yaml
Additional flags were passed along with --yaml. These flags will be ignored, and the configuration defined in the yaml will be used instead
The app name provided in the --yaml file "echoserver" does not match the one provided in the --name flag "null". The one provided in the --yaml file will be used.
| Running ..

Why the value found in the yaml file is used instead of the value passed as a command line argument ?

The order of evaluation of the parameters is wrong in my opinion.

In Linux the expected behaviour is to first check for any command-line arguments, then check for environment variables, and finally check for values in a configuration file. This approach allows for the most flexibility.

For example, if a user specifies a configuration value as a command-line argument, that value will be used and will override any other values that may be set in an environment variable or configuration file. This allows the user to quickly and easily specify custom values for their specific use case.

What should happen when evaluating configuration parameters is to first check for any command-line arguments, then check for environment variables, and finally check for values in a configuration file.

Please understand that Kubernetes is doing exactly this, and this is what everyone expects. Also Helm has the same behaviour, where the --set parameters in the command line are overwriting the values from the config files.

Also if a parameter is passed in the command line, the user should be able to omit it in the configuration file.

yonzhan commented 1 year ago

route to CXP team

henrikwh commented 1 year ago

Precedence is also the same for .NET: https://github.com/aspnet/MetaPackages/blob/rel/2.0.0/src/Microsoft.AspNetCore/WebHost.cs configfile->env settings-> cmd line args.

cmd line args wins.

ghost commented 1 year ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @calvinsID.

Issue Details
### Related command ``` az containerapp env create \ --name $CONTAINERAPPS_ENVIRONMENT \ --resource-group $RESOURCE_GROUP \ --location $LOCATION az containerapp create --yaml echoserver.yaml ``` Where `echoserver.yaml` is: ``` --- kind: containerapp location: eastus name: echoserver resourceGroup: aca type: Microsoft.App/containerApps tags: tagname: value properties: managedEnvironmentId: /subscriptions/12345678-0abc-defg-hijk-1234567890/resourceGroups/aca/providers/Microsoft.App/managedEnvironments/myaca configuration: activeRevisionsMode: Multiple ingress: external: true allowInsecure: false targetPort: 8080 traffic: - latestRevision: true weight: 100 transport: auto template: revisionSuffix: myrevision containers: - image: gcr.io/google_containers/echoserver:1.10 name: echoserver env: - name: HTTP_PORT value: 8080 resources: cpu: 0.5 memory: 1Gi probes: - type: liveness httpGet: path: "/health" port: 8080 httpHeaders: - name: "Custom-Header" value: "liveness probe" initialDelaySeconds: 7 periodSeconds: 3 - type: readiness tcpSocket: port: 8080 initialDelaySeconds: 10 periodSeconds: 3 - type: startup httpGet: path: "/startup" port: 8080 httpHeaders: - name: "Custom-Header" value: "startup probe" initialDelaySeconds: 3 periodSeconds: 3 scale: minReplicas: 1 maxReplicas: 3 ``` ### Extension name (the extension in question) [containerapp](https://github.com/Azure/azure-cli-extensions/tree/main/src/containerapp) ### Description of issue (in as much detail as possible) I tried to create a "yaml" to share on GitHub to create a sample Azure Container App. I was expecting this to work because I had the value `managedEnvironmentId` and the `name` information in the file, but with my surprise the CLI wanted again the name and resource group. I understand this is documented, name and resource group must be passed as CLI arguments: https://learn.microsoft.com/en-us/cli/azure/containerapp?view=azure-cli-latest#az-containerapp-create-required-parameters Then I run: ``` az containerapp create -n echoserver -g aca --yaml echoserver.yaml ``` And it worked, but with my surprise the name passed in the CLI is mandatory but ignored: Example: ``` az containerapp create -n null -g aca --yaml echoserver.yaml Additional flags were passed along with --yaml. These flags will be ignored, and the configuration defined in the yaml will be used instead The app name provided in the --yaml file "echoserver" does not match the one provided in the --name flag "null". The one provided in the --yaml file will be used. | Running .. ``` Why the value found in the yaml file is used instead of the value passed as a command line argument ? The order of evaluation of the parameters is wrong in my opinion. In Linux the expected behaviour is to first check for any command-line arguments, then check for environment variables, and finally check for values in a configuration file. This approach allows for the most flexibility. For example, if a user specifies a configuration value as a command-line argument, that value will be used and will override any other values that may be set in an environment variable or configuration file. This allows the user to quickly and easily specify custom values for their specific use case. What should happen when evaluating configuration parameters is to first check for any command-line arguments, then check for environment variables, and finally check for values in a configuration file. Please understand that Kubernetes is doing exactly this, and this is what everyone expects. Also Helm has the same behaviour, where the --set parameters in the command line are overwriting the values from the config files. Also if a parameter is passed in the command line, the user should be able to omit it in the configuration file.
Author: zioproto
Assignees: -
Labels: `Service Attention`, `Auto-Assign`, `ContainerApp`
Milestone: -
navba-MSFT commented 1 year ago

Adding Service team to look into this ask.

zioproto commented 1 year ago

Also terraform gives priority to command line options: https://developer.hashicorp.com/terraform/language/values/variables#variable-definition-precedence

bc-csewell commented 11 months ago

Is there any update on this, not being able to blend cli and yaml is quite annoying

jason-berk-k1x commented 7 months ago

I'll be blunt, this really sucks. Especially after fighting #5391 Both that issue and this one seem dead in the water....what is the expected turn around time. I'm really starting to wonder if ACA is "production ready" given the pile of issues / bugs I've found and the lack of response / feedback from the support team

ali-akdag commented 3 weeks ago

Any update on this? The investigation label was assigned almost 2 years ago. Using the YAML as input has greater value over the CLI command in terms of CI/CD deployments using templates and passing parameters. The CLI is missing some features such as volume mounts etc.

Also, please merge with https://github.com/microsoft/azure-container-apps/issues/43