Unfortunately, the acceptance tests here were improperly passing, and
allowing Subnet updates on ELBs is not as straightfoward as simply
removing `ForceNew`.
Subnets on ELBs need to be managed by two explicit API calls:
* `AttachLoadBalancerToSubnets` - http://bit.ly/elbattachsubnet
* `DetachLoadBalanceFromSubnets` - http://bit.ly/elbdetachsubnet
We'll need to circle back and use these APIs to explicitly add support.
This fixes the failure of `TestAccAWSELB_AddSubnet` by removing the
test.
This reverts commit 61e91017be, reversing
changes made to 49b3afe452.
Was relying on old behavior of GetOk and therefore never properly seeing
a change from true -> false.
This fixes the acceptance test failure of
`TestAccAWSInstance_sourceDestCheck`.
AWS provides a single `BlockDeviceMapping` to manage three different
kinds of block devices:
(a) The root volume
(b) Ephemeral storage
(c) Additional EBS volumes
Each of these types has slightly different semantics [1].
(a) The root volume is defined by the AMI; it can only be customized
with `volume_size`, `volume_type`, and `delete_on_termination`.
(b) Ephemeral storage is made available based on instance type [2]. It's
attached automatically if _no_ block device mappings are specified, and
must otherwise be defined with block device mapping entries that contain
only DeviceName set to a device like "/dev/sdX" and VirtualName set to
"ephemeralN".
(c) Additional EBS volumes are controlled by mappings that omit
`virtual_name` and can specify `volume_size`, `volume_type`,
`delete_on_termination`, `snapshot_id`, and `encryption`.
After deciding to ignore root block devices to fix#859, we had users
with configurations that were attempting to manage the root block device chime
in on #913.
Terraform does not have the primitives to be able to properly handle a
single collection of resources that is partially managed and partially
computed, so our strategy here is to break out logical sub-resources for
Terraform and hide the BlockDeviceMapping inside the provider
implementation.
Now (a) is supported by the `root_block_device` sub-resource, and (b)
and (c) are still both merged together under `block_device`, though I
have yet to see ephemeral block devices working properly.
Looking into possibly separating out `ephemeral_block_device` and
`ebs_block_device` sub-resources as well, which seem like the logical
next step. We'll wait until the next big release for this, though, since
it will break backcompat.
[1] http://bit.ly/ec2bdmap
[2] http://bit.ly/instancestorebytypeFixes#913
Refs #858
Right now we yield a perpetual diff on ASGs because we're not reading
termination policies back out in the provider.
This depends on https://github.com/mitchellh/goamz/pull/218 and fixes
it.
An `InstanceDiff` will include `ResourceAttrDiff` entries for the
"length" / `#` field of maps. This makes sense, since for something like
`terraform plan` it's useful to see when counts are changing.
The `DiffFieldReader` was not taking these entries into account when
reading maps out, and was therefore incorrectly returning maps that
included an extra `'#'` field, which was causing all sorts of havoc
for providers (extra tags on AWS instances, broken google compute
instance launch, possibly others).
* fixes#914 - extra tags on AWS instances
* fixes#883 - general core issue sprouted from #757
* removes the hack+TODO from #757
This resource allows an existing Route Table to be assigned as the
"main" Route Table of a VPC. This means that the Route Table will be
used for any subnets within the VPC without an explicit Route Table
assigned [1].
This is particularly useful in getting an Internet Gateway in place as
the default for a VPC, since the automatically created Main Route Table
does not have one [2].
Note that this resource is an abstraction over an association and does not
map directly to a CRUD-able object in AWS. In order to retain a coherent
"Delete" operation for this resource, we remember the ID of the AWS-created
Route Table and reset the VPC's main Route Table to it when this
resource is deleted.
refs #843, #748
[1] http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Route_Tables.html#RouteTableDetails
[2] http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Internet_Gateway.html#Add_IGW_Routing
If map_public_ip_on_launch was not specified, AWS picks a default of
"0", which is different than the "" in the state file, triggerinng an
update each time. Mark that parameter as Computed, avoiding the update.
This is necessary to support creating parameter groups with parameters
that require a reboot, since the RDS API will return an error when
attempting to set those parameters with ApplyMethod "immediate".
If a subnet exists in the state file and a refresh is performed, the
read function for subnets would return an error. Now it updates the
state to indicate that the subnet no longer exists, so Terraform can
plan to recreate it.
Several of the arguments were optional, and if omitted, they are
calculated. Mark them as such in the schema to avoid triggering an
update.
Go back to storing the password in the state file. Without doing so,
there's no way for Terraform to know the password has changed. It should
be hashed, but then interpolating the password yields a hash instead of
the password.
Make the `name` parameter optional. It's not required in any engine, and
in some (MS SQL Server) it's not allowed at all.
Drop the `skip_final_snapshot` argument. If `final_snapshot_identifier`
isn't specified, then don't make a final snapshot. As things were, it
was possible to create a resource with neither of these arguments
specified which would later fail when it was to be deleted since the RDS
API requires exactly one of the two.
Resolves issue #689.
- 5.6.17 is no longer a valid mysql engine version, bumping to 5.6.21
- updating security_group_names assertion to match new set structure
introduce in #663
When DeleteInternetGateway is successful it returns a nil error value.
However, for a nil error value, the RetryFunc returns an error yielding a
unnecessary second call to DeleteInternetGateway in the retry logic.
The logic works because DeleteInternetGateway eventually returns an ec2.Error
with error code InvalidInternetGatewayID.NotFound since the internet gateway
has been deleted in the previous call. The return value of nil breaks the
retry logic and the deletion is deemed successful.
Fix the unnecessary second call to DeleteInternetGateway by short circuiting
with a nil error value when deletion of the internet gateway is successful on
the first try.
Add an acceptance test for internet gateway deletion and remove unreachable
code while here.
If not suppling the `availability_zones`, they will be computed
(meaning an update/refresh will retrieve the info and update the values
to the state file).
So without the `Computed = true` the diff will always flag this as a
change, even when it’s not.
Some instance types have a block device by default. So when selecting
such an instance type, you will not set a config for the block device,
but the update/refresh func will notice one and update the state
nonetheless.
So in those cases the `block_device` becomes a `computed` field.
1. The schema contained a few fields that where not marked as
`computed`, while they were updated inside the resource.
2. While updating the `volume_size` it was doing so with a `string`,
but in the schema this field is set as `int`.
3. The set func for calculating the hashes for the `block` set items,
also used computed values to calculate the hash. As these values will
not be in the config, but only in the state, this will always show as a
diff. The solution is to only use the fields that aren’t computed in
order to get consistent hashes.
These where all issues before, but weren’t visible as such. All should
be good again now.
For now this only supports importing a key pair (by specifying a
public_key) property. In the future it'd be fairly trivial to support
key pair creation, with the private key returned as a computed property.
In real world usage you'd probably want to provide that public_key
property via a variable rather than hard-coding it into a terraform
config that'd end up in source control.
Fixes the following vet reports:
builtin/providers/aws/resource_aws_network_acl.go:191: wrong number of args for format in Errorf call: 2 needed but 3 args
builtin/providers/aws/resource_aws_network_acl.go:264: wrong number of args for format in Errorf call: 1 needed but 2 args
builtin/providers/aws/resource_aws_network_acl.go:268: wrong number of args for format in Errorf call: 1 needed but 2 args
builtin/providers/aws/resource_aws_network_acl.go:286: arg m[to_port].(int) for printf verb %s of wrong type: int
builtin/providers/aws/resource_aws_network_acl_test.go:277: arg r.NetworkAcls for printf verb %s of wrong type: []github.com/mitchellh/goamz/ec2.NetworkAcl
builtin/providers/aws/resource_aws_subnet_test.go:21: arg v.MapPublicIpOnLaunch for printf verb %s of wrong type: bool
There was an error in the goamz package the prevented updating the
availability zones correctly. So PR #181 should be merged before this
one can be merged…
By using a set for the availability zones, you can use things like
`availability_zones = ["${aws_instance.web.*.availability_zone}"]`
where is very likely multiple of the same zones will be added to the
set. If you use a list here, the list will say it’s changed (even if
you add the same zone) which will force a new resource.
The resource is build so it can attach and detach the Internet Gateway
from a VPC, but as the schema has `Required` and `ForceNew` both set
to `true` for the vpc_id field it will never use these capabilities.
This is a refactored solution for PR #616. Functionally this is still
the same change, but it’s implemented a lot cleaner with less code and
less changes to existing parts of TF.
Running the tests without these changes results in this error first:
```
--- FAIL: TestAccAWSNetworkAclsOnlyIngressRulesChange (24.92 seconds)
testing.go:121: Step 0 error: Check failed: Invalid number of ingress
entries found; count = %!s(int=3)
FAIL
exit status 1
FAIL github.com/hashicorp/terraform/builtin/providers/aws 24.974s
```
And after fixing that one you also get a few unexpected values due to
an expected order mismatch between the items in the set versus the
items in the config.
Those are also fixed, so the test is passing now.
Otherwise it is impossible to get simple configurations with the
"default" routing table, which is only the "local" route.
The following contents of main.tf expose the bug, and boots an instance
fine after this patch is applied:
variable aws_access_key {}
variable aws_secret_key {}
variable aws_ubuntu_ami {}
provider "aws" {
access_key = "${var.aws_access_key}"
secret_key = "${var.aws_secret_key}"
region = "us-east-1"
}
resource "aws_vpc" "default" {
cidr_block = "10.0.0.0/20"
}
resource "aws_route_table" "private" {
vpc_id = "${aws_vpc.default.id}"
# Note the lack of "route" sub-key here.
}
resource "aws_subnet" "private" {
vpc_id = "${aws_vpc.default.id}"
cidr_block = "10.0.1.0/24"
}
resource "aws_route_table_association" "private" {
subnet_id = "${aws_subnet.private.id}"
route_table_id = "${aws_route_table.private.id}"
}
# Demonstrate an instance can be booted fine in this fashion.
resource "aws_instance" "sample" {
ami = "${var.aws_ubuntu_ami}"
instance_type = "t2.micro"
subnet_id = "${aws_subnet.private.id}"
}
terraform.tfvars for completeness:
aws_access_key = "..."
aws_secret_key = "..."
# A public Trusty AMI
aws_ubuntu_ami = "ami-9aaa1cf2"
builtin/providers/aws/tags_test.go:56: unrecognized printf verb 'i'
builtin/providers/aws/tags_test.go:59: unrecognized printf verb 'i'
config/config_test.go:101: possible formatting directive in Fatal call
config/config_test.go:157: possible formatting directive in Fatal call
config/module/get_file_test.go:91: missing argument for Fatalf(%s): format reads arg 1, have only 0 args
helper/schema/schema.go:341: arg v.Type for printf verb %s of wrong type: schema.ValueType
helper/schema/schema.go:656: missing argument for Errorf(%s): format reads arg 2, have only 1 args
helper/schema/schema.go:912: arg schema.Type for printf verb %s of wrong type: schema.ValueType
terraform/context.go:178: arg v.Type() for printf verb %s of wrong type: github.com/hashicorp/terraform/config.VariableType
terraform/context.go:486: arg c.Operation for printf verb %s of wrong type: terraform.walkOperation
terraform/diff_test.go💯 arg actual for printf verb %s of wrong type: terraform.DiffChangeType
terraform/diff_test.go:235: arg actual for printf verb %s of wrong type: terraform.DiffChangeType
Prior to this, the diff only contained changed set elements. The issue
with this is that `getSet`, the internal function that reads a set from
the ResourceData, expects that each level (state, config, diff, etc.)
has the _full set_ information. This change was done to fix merging
issues.
Because of this, we need to make sure the full set is visible in the
diff.
cc/ @deoxxa see diff for how to add tests – quite straight forward.
Normally, I would like to add a check to ensure that the attribute
was updated remotely (by seeing what it's value with AWS is) but
I don't think we have that value returned.
When trying to output the `endpoint` attribute of an `aws_db_instance`,
the port was not being emitted properly. Converting the port number from
an integer to a string has resolved the issue.
cc/ @mitchellh
This one caught me a bit, and I'm not sure if there's another AttrType
that could be useful here – perhaps a AttrTypeHidden or something? I
don't know...this feels sloppy to have in the diff func.
/cc @pearkes - A few things here:
First, this fixes the issue you mentioned to be in chat. Basically: if
there are no security groups, don't put it into flatten, because flatten
will include a "key.#" = "0".
Next, I transformed your test into a "table-driven" test which are really
nice to write and maintain. Basically, as you find bugs or edge cases, you can
just add to the table and you get the test for free. I recommend these
whereever you have a pure input to output sort of data transformation
function.
Mitchell Hashimoto