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.

Sign up for GitHub

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

Jump to bottom

Adding Ports To Container Groups #1930

Merged
merged 19 commits into from
Jan 11, 2019
Merged

Adding Ports To Container Groups #1930

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6e57154
Adding Ports To Container Groups
nathanjsweet Sep 14, 2018
b182715
Merge branch 'master' of github.com:terraform-providers/terraform-pro…
nathanjsweet Oct 16, 2018
d0886d7
merging with master, adding more robust tests, also deprecating protocol
nathanjsweet Oct 16, 2018
b2c6c2e
use port number validation, remove parts of test that are conflicting
nathanjsweet Oct 16, 2018
dba2ecc
fixing bad ports reference
nathanjsweet Oct 16, 2018
1ea9489
Merge branch 'master' of github.com:terraform-providers/terraform-pro…
nathanjsweet Oct 25, 2018
8f55e16
conflict with the specific element
nathanjsweet Oct 25, 2018
2c3eaa5
Merge remote-tracking branch 'origin/master'
nathanjsweet Nov 27, 2018
648e46a
removing unnecessary package
nathanjsweet Nov 27, 2018
7a1debf
fix set list issue
nathanjsweet Nov 28, 2018
1b22262
adding documentation, fixing tests
nathanjsweet Nov 29, 2018
48f0d65
Merge branch 'master' into master
metacpp Jan 5, 2019
a0727c3
update default to use constant
katbyte Jan 6, 2019
55b840b
ports are sets
nathanjsweet Jan 9, 2019
c584062
Merge branch 'master' of github.com:nathanjsweet/terraform-provider-a…
nathanjsweet Jan 9, 2019
93a5d1e
Merge branch 'master' into njs-container_portstest
katbyte Jan 11, 2019
6997434
fixed container group ports
katbyte Jan 11, 2019
04b5bf3
make fmt goimport
katbyte Jan 11, 2019
f80e912
add some validation
katbyte Jan 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 82 additions & 16 deletions azurerm/resource_arm_container_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
Expand Down Expand Up @@ -148,16 +149,48 @@ func resourceArmContainerGroup() *schema.Resource {
},

"port": {
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
ValidateFunc: validation.IntBetween(1, 65535),
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"container.0.ports"},
Deprecated: "Use `ports` instead.",
ValidateFunc: validate.PortNumber,
},
katbyte marked this conversation as resolved.
Show resolved Hide resolved

"ports": {
Type: schema.TypeSet,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"container.0.port", "container.0.protocol"},
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"protocol": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
Default: string(containerinstance.TCP),
ValidateFunc: validation.StringInSlice([]string{
string(containerinstance.TCP),
string(containerinstance.UDP),
}, true),
},
"port": {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
ValidateFunc: validate.PortNumber,
},
},
},
},
katbyte marked this conversation as resolved.
Show resolved Hide resolved

"protocol": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ConflictsWith: []string{"container.0.ports"},
Deprecated: "Use `ports` instead.",
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
ValidateFunc: validation.StringInSlice([]string{
string(containerinstance.TCP),
Expand Down Expand Up @@ -436,6 +469,25 @@ func expandContainerGroupContainers(d *schema.ResourceData) (*[]containerinstanc
containerGroupPorts = append(containerGroupPorts, containerGroupPort)
}

if v, ok := data["ports"]; ok {
s := v.(*schema.Set)
Copy link

Choose a reason for hiding this comment

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

As I also need to deploy a container instance with multiple open ports, I cherry-picked the change in this pull request to a forked repo: https://github.com/TechhubLisbon/terraform-provider-azurerm/tree/patched_master

I only had one issue: panic: interface conversion: interface {} is *schema.Set, not []interface {}

I fixed this by replacing this line with s := v.([]interface{})

Commit: tblxio@dc95c9e

Everything else looked good!

var ports []containerinstance.ContainerPort
for _, v := range s.List() {
portObj := v.(map[string]interface{})
port := int32(portObj["port"].(int))
proto := portObj["protocol"].(string)
ports = append(ports, containerinstance.ContainerPort{
Protocol: containerinstance.ContainerNetworkProtocol(strings.ToUpper(proto)),
Port: &port,
})
containerGroupPorts = append(containerGroupPorts, containerinstance.Port{
Protocol: containerinstance.ContainerGroupNetworkProtocol(strings.ToUpper(proto)),
Port: &port,
})
}
container.Ports = &ports
}

// Set both sensitive and non-secure environment variables
var envVars *[]containerinstance.EnvironmentVariable
var secEnvVars *[]containerinstance.EnvironmentVariable
Expand Down Expand Up @@ -617,10 +669,13 @@ func flattenContainerGroupContainers(d *schema.ResourceData, containers *[]conta

//map old container names to index so we can look up things up
nameIndexMap := map[string]int{}
var newPortsField bool
for i, c := range d.Get("container").([]interface{}) {
cfg := c.(map[string]interface{})
nameIndexMap[cfg["name"].(string)] = i

if _, ok := cfg["ports"]; ok {
newPortsField = true
}
}

containerCfg := make([]interface{}, 0, len(*containers))
Expand Down Expand Up @@ -651,19 +706,30 @@ func flattenContainerGroupContainers(d *schema.ResourceData, containers *[]conta
}

if len(*container.Ports) > 0 {
containerPort := *(*container.Ports)[0].Port
containerConfig["port"] = containerPort
// protocol isn't returned in container config, have to search in container group ports
protocol := ""
if containerGroupPorts != nil {
for _, cgPort := range *containerGroupPorts {
if *cgPort.Port == containerPort {
protocol = string(cgPort.Protocol)
if newPortsField {
ports := make([]interface{}, 0)
for _, p := range *container.Ports {
port := make(map[string]interface{})
port["port"] = int(*p.Port)
port["protocol"] = string(p.Protocol)
ports = append(ports, port)
}
containerConfig["ports"] = ports
} else {
containerPort := *(*container.Ports)[0].Port
containerConfig["port"] = containerPort
// protocol isn't returned in container config, have to search in container group ports
protocol := ""
if containerGroupPorts != nil {
for _, cgPort := range *containerGroupPorts {
if *cgPort.Port == containerPort {
protocol = string(cgPort.Protocol)
}
}
}
}
if protocol != "" {
containerConfig["protocol"] = protocol
if protocol != "" {
containerConfig["protocol"] = protocol
}
}
}

Expand Down
72 changes: 57 additions & 15 deletions azurerm/resource_arm_container_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func TestAccAzureRMContainerGroup_imageRegistryCredentialsUpdate(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.server", "mine.acr.io"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.username", "acrusername"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.password", "acrpassword"),
resource.TestCheckResourceAttr(resourceName, "container.0.port", "5443"),
resource.TestCheckResourceAttr(resourceName, "container.0.protocol", "UDP"),
),
},
{
Expand All @@ -81,6 +83,9 @@ func TestAccAzureRMContainerGroup_imageRegistryCredentialsUpdate(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.server", "hub.docker.com"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.username", "updatedusername"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.password", "updatedpassword"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "80"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail because with a set you can no longer use .0. notation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what can I do?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The best way is to simply check the count via #. You can calculate the hash and access it that way however that isn't required.

resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
),
},
},
Expand All @@ -104,6 +109,7 @@ func TestAccAzureRMContainerGroup_linuxBasic(t *testing.T) {
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "container.#", "1"),
resource.TestCheckResourceAttr(resourceName, "os_type", "Linux"),
resource.TestCheckResourceAttr(resourceName, "container.0.port", "80"),
),
},
{
Expand Down Expand Up @@ -172,6 +178,11 @@ func TestAccAzureRMContainerGroup_linuxBasicUpdate(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "container.#", "2"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.#", "2"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "80"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail because with a set you can no longer use .0. notation.

resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.1.port", "5443"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.1.protocol", "UDP"),
),
},
},
Expand All @@ -194,6 +205,9 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "container.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "80"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail because with a set you can no longer use .0. notation.

resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
resource.TestCheckResourceAttr(resourceName, "container.0.command", "/bin/bash -c ls"),
resource.TestCheckResourceAttr(resourceName, "container.0.commands.#", "3"),
resource.TestCheckResourceAttr(resourceName, "container.0.commands.0", "/bin/bash"),
Expand Down Expand Up @@ -245,6 +259,11 @@ func TestAccAzureRMContainerGroup_windowsBasic(t *testing.T) {
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "container.#", "1"),
resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.#", "2"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "80"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail because with a set you can no longer use .0. notation.

resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "443"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
),
},
{
Expand Down Expand Up @@ -272,6 +291,9 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "container.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.port", "80"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail because with a set you can no longer use .0. notation.

resource.TestCheckResourceAttr(resourceName, "container.0.ports.0.protocol", "TCP"),
resource.TestCheckResourceAttr(resourceName, "container.0.command", "cmd.exe echo hi"),
resource.TestCheckResourceAttr(resourceName, "container.0.commands.#", "3"),
resource.TestCheckResourceAttr(resourceName, "container.0.commands.0", "cmd.exe"),
Expand Down Expand Up @@ -320,7 +342,7 @@ resource "azurerm_container_group" "test" {
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = "80"
port = 80
}

tags {
Expand Down Expand Up @@ -372,11 +394,12 @@ resource "azurerm_container_group" "test" {
os_type = "Linux"

container {
name = "hw"
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = "80"
name = "hw"
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = 5443
protocol = "udp"
}

image_registry_credential {
Expand Down Expand Up @@ -424,7 +447,9 @@ resource "azurerm_container_group" "test" {
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = "80"
ports = {
port = 80
}
}

image_registry_credential {
Expand Down Expand Up @@ -466,7 +491,13 @@ resource "azurerm_container_group" "test" {
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = "80"
ports = {
port = 80
}
ports = {
port = 5443
protocol = "udp"
}
}

container {
Expand Down Expand Up @@ -502,7 +533,14 @@ resource "azurerm_container_group" "test" {
image = "microsoft/windowsservercore:latest"
cpu = "2.0"
memory = "3.5"
port = "80"
ports = {
port = 80
protocol = "tcp"
}
ports = {
port = 443
protocol = "tcp"
}
}

tags {
Expand Down Expand Up @@ -533,7 +571,10 @@ resource "azurerm_container_group" "test" {
image = "microsoft/windowsservercore:latest"
cpu = "2.0"
memory = "3.5"
port = "80"
ports = {
port = 80
protocol = "tcp"
}

environment_variables {
"foo" = "bar"
Expand Down Expand Up @@ -594,8 +635,10 @@ resource "azurerm_container_group" "test" {
cpu = "1"
memory = "1.5"

port = "80"
protocol = "TCP"
ports = {
port = 80
protocol = "tcp"
}

volume {
name = "logs"
Expand All @@ -604,11 +647,11 @@ resource "azurerm_container_group" "test" {
share_name = "${azurerm_storage_share.test.name}"

storage_account_name = "${azurerm_storage_account.test.name}"
storage_account_key = "${azurerm_storage_account.test.primary_access_key}"
storage_account_key = "${azurerm_storage_account.test.primary_access_key}"
}

environment_variables {
"foo" = "bar"
"foo" = "bar"
"foo1" = "bar1"
}

Expand Down Expand Up @@ -651,7 +694,6 @@ func testCheckAzureRMContainerGroupExists(resourceName string) resource.TestChec
}
return fmt.Errorf("Bad: Get on containerGroupsClient: %+v", err)
}

return nil
}
}
Expand Down
17 changes: 13 additions & 4 deletions website/docs/r/container_group.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ resource "azurerm_container_group" "aci-helloworld" {
image = "seanmckenna/aci-hellofiles"
cpu = "0.5"
memory = "1.5"
port = "80"
ports = {
port = 80
}
ports = {
port = 443
}

environment_variables {
"NODE_ENV" = "testing"
Expand Down Expand Up @@ -121,9 +126,7 @@ The `container` block supports:

* `memory` - (Required) The required memory of the containers in GB. Changing this forces a new resource to be created.

* `port` - (Optional) A public port for the container. Changing this forces a new resource to be created.

* `protocol` - (Optional) The protocol associated with port for the container. Allowed values are `TCP` and `UDP`.
* `ports` - (Optional) A set of public ports for the container. Changing this forces a new resource to be created. Set as documented in the `ports` block below.

* `environment_variables` - (Optional) A list of environment variables to be set on the container. Specified as a map of name/value pairs. Changing this forces a new resource to be created.

Expand Down Expand Up @@ -159,6 +162,12 @@ The `image_registry_credential` block supports:

* `server` - (Required) The address to use to connect to the registry without protocol ("https"/"http"). For example: "myacr.acr.io"

The `ports` block supports:

* `port` - (Required) The port number the container will expose.

* `protocol` - (Optional) The network protocol ("tcp"/"udp") the port will use.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should specify what the default is:

Suggested change
* `protocol` - (Optional) The network protocol ("tcp"/"udp") the port will use.
* `protocol` - (Optional) The network protocol associated with port. Possible values are `TCP`, `UDP` and the default is `TCP`.


## Attributes Reference

The following attributes are exported:
Expand Down