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

Support for TCP type & ExposedPort on azurerm_container_app #23752

Conversation

SpartakusMd
Copy link
Contributor

@SpartakusMd SpartakusMd commented Nov 1, 2023

@SpartakusMd SpartakusMd marked this pull request as ready for review November 1, 2023 10:32
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @SpartakusMd.

Overall this looks good, but the test needs to be updated to use the config that you added in this PR and the documentation would also need an update.

Once that's done I can run the test and see if we can get this merged.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

New test failure 🙃 - I'm no SME on ContainerApps so I'm not sure how specific the configuration will need to be in order to use exposed_port or how much validation we should be putting into the resource to prevent users from running into this kind of error.

@jackofallops perhaps you could provide a little bit of guidance here?

------- Stdout: -------
=== RUN   TestAccContainerAppResource_completeTcpExposedPort
=== PAUSE TestAccContainerAppResource_completeTcpExposedPort
=== CONT  TestAccContainerAppResource_completeTcpExposedPort
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        Error: creating Container App (Subscription: "*******"
        Resource Group Name: "acctestRG-CAEnv-231101121940949762"
        Container App Name: "acctest-capp-231101121940949762"): performing CreateOrUpdate: containerapps.ContainerAppsClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: Code="ContainerAppTcpRequiresVnet" Message="Applications with external TCP ingress can only be deployed to Container App Environments that have a custom VNET. Set ingress traffic to 'Limited to Container Apps Environment' if you want to deploy to a Container App Environment without a custom VNET. For more information on custom VNETs, please visit https://go.microsoft.com/fwlink/?linkid=2238174
          with azurerm_container_app.test,
          on terraform_plugin_test.tf line 130, in resource "azurerm_container_app" "test":
         130: resource "azurerm_container_app" "test" {
        creating Container App (Subscription: "*******"
        Resource Group Name: "acctestRG-CAEnv-231101121940949762"
        Container App Name: "acctest-capp-231101121940949762"): performing
        CreateOrUpdate: containerapps.ContainerAppsClient#CreateOrUpdate: Failure
        sending request: StatusCode=0 -- Original Error:
        Code="ContainerAppTcpRequiresVnet" Message="Applications with external TCP
        ingress can only be deployed to Container App Environments that have a custom
        VNET. Set ingress traffic to 'Limited to Container Apps Environment' if you
        want to deploy to a Container App Environment without a custom VNET. For more
        information on custom VNETs, please visit
        https://go.microsoft.com/fwlink/?linkid=2238174
--- FAIL: TestAccContainerAppResource_completeTcpExposedPort (1162.14s)
FAIL

@jackofallops
Copy link
Member

New test failure 🙃 - I'm no SME on ContainerApps so I'm not sure how specific the configuration will need to be in order to use exposed_port or how much validation we should be putting into the resource to prevent users from running into this kind of error.

@jackofallops perhaps you could provide a little bit of guidance here?

@SpartakusMd / @stephybun - The config is missing a user defined subnet to attach to the Container App Environment, using templateWithVnet in place of templatePlusExtras I think will resolve that. fwiw, a lot of the config in the completeTcpExposedPort test can be removed as it only needs to test the new property / ingress block (e.g. the extended container config for probes and volumes, since they're unrelated to the change and provide no additional benefit here)

@SpartakusMd
Copy link
Contributor Author

SpartakusMd commented Nov 1, 2023

Switched to use templateWithVnet and did some cleanup. Hopefully it's enough 🤞 But I believe it needs infrastructure_subnet_id on the azurerm_container_app_environment but it's missing. Am I rigth?

If it will fail again the test, I will another app environment with everything necessary for the Vnet.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

@SpartakusMd third times a charm - test passed! 🎉

Just need to fix up the terraform linting that the GitHub Action is flagging then this should be good to go!

@SpartakusMd
Copy link
Contributor Author

SpartakusMd commented Nov 2, 2023

@stephybun perfect. Fixed the linting too.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @SpartakusMd LGTM 💯

@stephybun stephybun merged commit 233c40d into hashicorp:main Nov 2, 2023
23 checks passed
@github-actions github-actions bot added this to the v3.79.0 milestone Nov 2, 2023
@SpartakusMd SpartakusMd deleted the feature/Support-for-ExposedPort-on-azurerm_container_app branch November 2, 2023 08:07
stephybun added a commit that referenced this pull request Nov 2, 2023
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Nov 6, 2023
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>&#34;hashicorp/azurerm&#34; updated from &#34;3.77.0&#34; to
&#34;3.79.0&#34; in file &#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.79.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.79.0&#xA;ENHANCEMENTS:&#xA;&#xA;*
provider: log instead of error when RPs are unavailable when validating
RP registrations
([#23380](https://github.com/hashicorp/terraform-provider-azurerm/issues/23380))&#xA;*
`azurerm_arc_kuberenetes_cluster_extension_resource` - the `version` and
`release_train` properties can now be set simultaneously
([#23692](https://github.com/hashicorp/terraform-provider-azurerm/issues/23692))&#xA;*
`azurerm_container_apps` - support for the `ingress.exposed_port`
property
([#23752](https://github.com/hashicorp/terraform-provider-azurerm/issues/23752))&#xA;*
`azurerm_cosmosdb_postgresql_cluster` - read replica clusters can be
created without specifying `administrator_login_password` property
([#23750](https://github.com/hashicorp/terraform-provider-azurerm/issues/23750))&#xA;*
`azurerm_managed_application` - arrays can be supplied in the
`parameter_values` property
([#23754](https://github.com/hashicorp/terraform-provider-azurerm/issues/23754))&#xA;*
`azurerm_storage_management_policy` - support for properties
`rule.*.actions.*.base_blob.0.tier_to_cold_after_days_since_{modification|last_access_time|creation}_greater_than
and
rule.*.actions.*.{snapshot|version}.0.tier_to_cold_after_days_since_creation_greater_than`
([#23574](https://github.com/hashicorp/terraform-provider-azurerm/issues/23574))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* `azurerm_api_management_diagnostic` - the
`operation_name_format` attribute will only be sent if `identifier` is
set to `applicationinsights`
([#23736](https://github.com/hashicorp/terraform-provider-azurerm/issues/23736))&#xA;*
`azurerm_backup_policy_vm` - fix payload by using current datetime
([#23586](https://github.com/hashicorp/terraform-provider-azurerm/issues/23586))&#xA;*
`azurerm_kubernetes_cluster` - the `custom_ca_trust_certificates_base64`
property can not be removed, only updated
([#23737](https://github.com/hashicorp/terraform-provider-azurerm/issues/23737))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
    </action>
</Actions>

---

<table>
  <tr>
    <td width="77">
<img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli
logo" width="50" height="50">
    </td>
    <td>
      <p>
Created automatically by <a
href="https://www.updatecli.io/">Updatecli</a>
      </p>
      <details><summary>Options:</summary>
        <br />
<p>Most of Updatecli configuration is done via <a
href="https://www.updatecli.io/docs/prologue/quick-start/">its
manifest(s)</a>.</p>
        <ul>
<li>If you close this pull request, Updatecli will automatically reopen
it, the next time it runs.</li>
<li>If you close this pull request and delete the base branch, Updatecli
will automatically recreate it, erasing all previous commits made.</li>
        </ul>
        <p>
Feel free to report any issues at <a
href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br
/>
If you find this tool useful, do not hesitate to star <a
href="https://github.com/updatecli/updatecli/stargazers">our GitHub
repository</a> as a sign of appreciation, and/or to tell us directly on
our <a
href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>!
        </p>
      </details>
    </td>
  </tr>
</table>

Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for ExposedPort on azurerm_container_app
3 participants