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

Azure Batch Pool Container Configuration #3411

Closed
casparjespersen opened this issue May 9, 2019 · 10 comments
Closed

Azure Batch Pool Container Configuration #3411

casparjespersen opened this issue May 9, 2019 · 10 comments

Comments

@casparjespersen
Copy link

casparjespersen commented May 9, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Azure Batch Pools can be Docker compatible if they have a container configuration supplied to them. Unfortunately, the current Terraform API version does not support this property. It would be a simple task to include this setup.

New or Affected Resource(s)

  • azurerm_batch_pool

Potential Terraform Configuration

resource "azurerm_batch_pool" "test" {
  name                = "testaccpool"
  resource_group_name = "${azurerm_resource_group.test.name}"
  account_name        = "${azurerm_batch_account.test.name}"
  display_name        = "Test Acc Pool Auto"
  vm_size             = "Standard_A1"
  node_agent_sku_id   = "batch.node.ubuntu 16.04"

  storage_image_reference {
    publisher = "microsoft-azure-batch"
    offer     = "ubuntu-server-container"
    sku       = "16-04-lts"
    version   = "latest"
  }

  container_configuration {
    container_image_names = ["ubuntu", "hello-world"]
    container_registries = [
      {
        registry_server = "registry-server-url"
        user_name = "registry-username"
        password = "registry-password"
      }
    ]
  }

}

References

@tshauck
Copy link
Contributor

tshauck commented May 11, 2019

There's still work to do on #3311, but once it's merged it'll provide the basic functionality of saying a pool is configured for docker, otherwise the other attributes of the container_configuration are optional (e.g. container_image_names). Next step would be to add those optional keys.

@ennui93
Copy link
Contributor

ennui93 commented Aug 2, 2019

I took a first stab at adding the container_registries attribute. See the add_batch_container_configs branch in my fork. However, I'm unable to create a schema with a list of dictionaries with specific keys. Can anyone suggest how to write this schema correctly?

@ennui93
Copy link
Contributor

ennui93 commented Aug 13, 2019

OK, I did the thing. Please see PR #4072.

I was unable to create a proper block syntax for the elements in the container_registries block, even trying to use the ConfigMode: schema.SchemaConfigModeAttr workaround described in hashicorp/terraform#20626. Instead, I resorted to a set of maps to hold each of the container registry settings. This has some undesirable side effects such as allowing users to create spurious 'labels' for each of the container registry declarations, as well as overly-aggressive recreation of the pool on updates.

Comments/suggestions on the PR are welcome.

@ennui93
Copy link
Contributor

ennui93 commented Aug 21, 2019

I was able to get this working properly in the end with feedback from @katbyte in PR #4072, which is now merged as a9a9872.

This means that all that is left to complete this issue is adding support for container_image_names to container_configuration.

@dprateek1991
Copy link

@ennui93 Any idea by when this feature will be merged. I think we missed a very important feature in terraform by not adding container_image_names.

I'm actually working on a use case where we need to use Azure Batch Service within ADF to do some data transfer between Azure and AWS and thus realise since now Pools can be docker compatible but no way to pass container_image_names in it.

Without this feature, how currently its used ?

@ennui93
Copy link
Contributor

ennui93 commented Jan 7, 2020

PRs #3311 & #4702 are both merged and available in version 1.33.0 of the provider and later.

@tshauck, as I recall, this resource can provision working batch jobs without container_image_names. Is this correct? or do we need this key to use containers in Azure Batch service from Terraform?

@tshauck
Copy link
Contributor

tshauck commented Jan 9, 2020

@ennui93 @dprateek1991 See this comment: #3411 (comment) -- the PR makes the pools docker compatible, but doesn't allow the configuration of the image or registry. To really be usable that should be added.

My hope was to get the ball rolling, so thanks @ennui93 for adding registries... @dprateek1991 maybe you could add container_image_names then I think this ticket would be complete(?)

@ennui93
Copy link
Contributor

ennui93 commented Feb 10, 2020

Initial iteration of the code to add container_image_names can be found at the add_batch_container_image_names branch in my fork. However, it is failing unit tests with a runtime exception at present. I'll investigate further as I have time.

@favoretti
Copy link
Collaborator

Since this issue has been reported a long time ago and relates to the version of provider we no longer support - I'm going to close it. Please open a new updated bug report if this is still relevant. Thank you.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants