r/aws_instance: add T2 unlimited to aws instance resources #2619

kl4w · 2017-12-10T22:24:13Z

I think this should cover: #2491

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstance_t2Unlimited'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_t2Unlimited -timeout 120m
=== RUN   TestAccAWSInstance_t2Unlimited_invalidInstanceType
--- PASS: TestAccAWSInstance_t2Unlimited_invalidInstanceType (24.21s)
=== RUN   TestAccAWSInstance_t2Unlimited_unlimitedCredits
--- PASS: TestAccAWSInstance_t2Unlimited_unlimitedCredits (127.75s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	151.986s

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_t2Unlimited'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_t2Unlimited -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_t2Unlimited
--- PASS: TestAccAWSInstanceDataSource_t2Unlimited (147.84s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	147.866s

radeksimko

Hi @kl4w
thanks for the PR.

Is there any reason to drift away from the API specs?

How would you feel about modelling it like

resource "aws_instance" "..." {
...
  credit_specification {
    cpu_credits = "unlimited"
  }
}

It may look safe to simplify the experience for the user now, but we have learnt over time that Amazon is constantly adding new features to the API and if we build abstractions we effectively block ourselves from implementing future changes or at the very least make it more difficult.

For the same reason I think we shouldn't try to be too clever and validate instance_type - we have no idea how is the API going to change in the future and I think (correct me if I'm wrong) we can just rely on the API validation here?

kl4w · 2017-12-12T12:20:11Z

Yes absolutely, I've been going back and forth about how to implement this portion and I believe your suggestion makes complete sense.

Also considering that the launch template also takes the same structure in its data.

I'll implement the change and get back to you

kl4w · 2017-12-12T14:48:19Z

@radeksimko Hopefully this covers your review.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstance_creditSpecification'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_creditSpecification -timeout 120m
=== RUN   TestAccAWSInstance_creditSpecification_standardCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (220.17s)
=== RUN   TestAccAWSInstance_creditSpecification_unlimitedCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (231.52s)
=== RUN   TestAccAWSInstance_creditSpecification_updateCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (129.54s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	581.273s


make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_creditSpecification'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_creditSpecification -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_creditSpecification
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (124.64s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	124.673s

kylelaverty · 2018-01-03T18:43:09Z

@radeksimko Was there anything else missing from this PR? I was looking at preping the change for launch configurations for once that update is done in the ASW SDK/CLI but without this implementation confirmed I didn't want to get too far into anything.

radeksimko · 2018-01-03T17:31:11Z

aws/resource_aws_instance.go

@@ -1024,6 +1048,26 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}

+	if d.HasChange("credit_specification") {
+		creditSpecifications := d.Get("credit_specification").([]interface{})
+		if len(creditSpecifications) == 1 {


I think we're limiting ourselves here to cases when credit_specification is always in the config, which means we're not covering the case of removal of credit_specification (i.e. user basically can't disable it once it's enabled).

I'm not sure what's the best approach, but I assume we'll need to call ModifyInstanceCreditSpecification too with empty slice?

@radeksimko I have a question about HasChange. It looks like it doesn't detect removal of the attributes. Has this always been the case?

It looks like in order to disable the credit_specification once it's enabled means setting the value back to standard.

What would your recommendation on how to handle this be?

radeksimko · 2018-01-04T09:25:36Z

aws/resource_aws_instance.go

+		cs := v.([]interface{})
+		if len(cs) > 1 {
+			return nil, errors.New("Cannot specify more than one credit_specification.")
+		}


No big deal, but this is already guaranteed by the MaxItems: 1 in the schema, so this check is redundant.

radeksimko · 2018-01-04T09:27:36Z

aws/data_source_aws_instance_test.go

@@ -20,6 +20,7 @@ func TestAccAWSInstanceDataSource_basic(t *testing.T) {
 					resource.TestCheckResourceAttr("data.aws_instance.web-instance", "ami", "ami-4fccb37f"),
 					resource.TestCheckResourceAttr("data.aws_instance.web-instance", "tags.%", "1"),
 					resource.TestCheckResourceAttr("data.aws_instance.web-instance", "instance_type", "m1.small"),
+					resource.TestCheckResourceAttr("data.aws_instance.web-instance", "t2_unlimited", "false"),


radeksimko · 2018-01-04T09:27:49Z

website/docs/r/instance.html.markdown

@@ -233,6 +242,7 @@ The following attributes are exported:
 * `security_groups` - The associated security groups.
 * `vpc_security_group_ids` - The associated security groups in non-default VPC
 * `subnet_id` - The VPC subnet ID.
+* `t2_unlimited` - Whether [T2 Unlimited](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/t2-unlimited.html) is enabled.


kl4w · 2018-01-06T01:50:08Z

@radeksimko aside from your comment on the HasChange, i've taken account of the others, rebased and squashed the commits.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstance_creditSpecification'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_creditSpecification -timeout 120m
=== RUN   TestAccAWSInstance_creditSpecification_standardCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (166.60s)
=== RUN   TestAccAWSInstance_creditSpecification_unlimitedCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (171.80s)
=== RUN   TestAccAWSInstance_creditSpecification_updateCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (218.47s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	556.902s

make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_creditSpecification'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_creditSpecification -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_creditSpecification
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (176.01s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	176.046s

dmitrypol · 2018-01-22T17:47:04Z

sorry to be a pest but any timeline on when this will be released?

fender4645 · 2018-02-22T17:32:21Z

Is there any reason why this hasn't been pulled yet? We're hitting a very obscure yet very real limitation in AWS where you can only spin up ~100 t2 instances with full credits in a 24 hour period and t2.unlimited will solve this for us.

curator · 2018-03-01T22:17:44Z

My company's in serious need of Terraform-based support for this, can we please get this pulled?

andycognitoiq · 2018-03-02T16:23:44Z

We really want to make use of this too. Please can it be merged and released ASAP.

Thank you!

omeid · 2018-03-19T11:32:29Z

@kl4w Could you please rebase this?

appilon · 2018-03-19T15:49:18Z

I've actually taken the liberty of rebasing the branch, I'm just running the acceptance tests now. We will see if I can push directly to the branch (allow edits from maintainers).

Affected tests from rebase

=== RUN   TestAccAWSInstanceDataSource_getPasswordData_trueToFalse
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_trueToFalse (271.27s)
=== RUN   TestAccAWSInstanceDataSource_getPasswordData_falseToTrue
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_falseToTrue (260.96s)
=== RUN   TestAccAWSInstanceDataSource_creditSpecification
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (125.04s)

=== RUN   TestAccAWSInstance_getPasswordData_falseToTrue
--- PASS: TestAccAWSInstance_getPasswordData_falseToTrue (218.31s)
=== RUN   TestAccAWSInstance_getPasswordData_trueToFalse
--- PASS: TestAccAWSInstance_getPasswordData_trueToFalse (215.76s)

=== RUN   TestAccAWSInstance_creditSpecification_standardCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (112.00s)
=== RUN   TestAccAWSInstance_creditSpecification_unlimitedCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (100.97s)
=== RUN   TestAccAWSInstance_creditSpecification_updateCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (148.91s)

paultyng · 2018-03-19T16:30:52Z

aws/resource_aws_instance.go

+						"cpu_credits": {
+							Type:     schema.TypeString,
+							Optional: true,
+							Computed: true,


Should this have validation? I'm not familiar with the API, but I only seen unlimited and standard in the results? Is it really free form?

I believe the API complains if you use something other than unlimited and standard, leaving this opaque in terraform gives out of the box support for new cpu_credit types in the future.

appilon · 2018-03-19T16:51:34Z

@kl4w I took the liberty rebasing your branch (I only solved the merge conflicts I didn't change anything), this means you will need to do a reset locally. I'm new to the TF team so I might have jumped the gun on rebasing your work to try and get it across the finish line, my sincerest apologies if that's the case. Feel free to ping me if there is anything I can do on this PR going forward. Otherwise amazing work 🎉

kl4w · 2018-03-21T10:47:48Z

@appilon that’s perfect, thanks. I’m actually on break and wouldn’t have been able to get to this for a few more days.

_{Sent with GitHawk}

GoodMirek · 2018-04-10T18:00:07Z

This feature is pretty important for me too. I do not understand why this feature is pushed back and why related comments are marked as off-topic.

rbnrye · 2018-04-10T18:19:21Z

Can't tell who marked the comments as off-topic, but maybe @bflad can elaborate on why it keeps getting pushed out to the next milestone when seemingly, from this pull requests, all is ready to go?

bflad · 2018-04-17T18:26:10Z

@kl4w we just ran into a similar situation with aws_codebuild_project and its cache.0.type attribute -- basically we opted for:

Removing Computed: true from parent and child attribute, adding a Default: X to the child attribute, and adding a DiffSuppressFunc on the parent attribute to prevent the perpetual difference when not defined in the configuration:

https://github.com/terraform-providers/terraform-provider-aws/blob/7bcafda1f20b56838e514e68148af11d2e3d8190/aws/resource_aws_codebuild_project.go#L74-L79

Then using this behavior for handle the update when it was removed:

https://github.com/terraform-providers/terraform-provider-aws/blob/7bcafda1f20b56838e514e68148af11d2e3d8190/aws/resource_aws_codebuild_project.go#L577-L585

Maybe worth giving this a shot here too?

bflad

Left some feedback below. Please let us know if you have any questions.

bflad · 2018-04-17T18:29:47Z

aws/data_source_aws_instance.go

+		if err != nil {
+			return err
+		}
+		d.Set("credit_specification", creditSpecifications)


We should return errors when attempting to set non-scalar attributes in the state (otherwise it will silently use the value of the configuration or in the case of the data source, not set it at all):

if err := d.Set("credit_specification", creditSpecifications); err != nil { return fmt.Errorf("error setting credit_specification: %s", err) }

bflad · 2018-04-17T18:30:20Z

aws/resource_aws_instance.go

+		if err != nil {
+			return err
+		}
+		d.Set("credit_specification", creditSpecifications)


Same here -- we should return errors when attempting to set non-scalar attributes in the state (otherwise it will silently use the value of the configuration or in the case of the data source, not set it at all):

if err := d.Set("credit_specification", creditSpecifications); err != nil { return fmt.Errorf("error setting credit_specification: %s", err) }

bflad · 2018-04-17T18:32:46Z

aws/resource_aws_instance.go

@@ -1644,6 +1689,18 @@ func buildAwsInstanceOpts(
 		InstanceType:          aws.String(d.Get("instance_type").(string)),
 	}

+	if v, ok := d.GetOk("credit_specification"); ok {
+		cs := v.([]interface{})
+		for _, csValue := range cs {


Since there will only be one element if its defined, we can skip the loop and directly access cs[0]

bflad · 2018-04-17T18:34:26Z

aws/resource_aws_instance_test.go

+					resource.TestCheckResourceAttr(resName, "credit_specification.#", "1"),
+					resource.TestCheckResourceAttr(resName, "credit_specification.0.cpu_credits", "unlimited"),
+				),
+			},


Please add a TestStep that removes the credit_specification configuration and expects to find credit_specification.0.cpu_credits set to standard

kl4w · 2018-04-20T10:40:08Z

@bflad I've rebased (from some time yesterday iirc) and made changes based on your review

TF_SCHEMA_PANIC_ON_ERROR=1 make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstance_creditSpecification_'
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_creditSpecification_ -timeout 120m
=== RUN   TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard
--- PASS: TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard (97.88s)
=== RUN   TestAccAWSInstance_creditSpecification_standardCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (106.56s)
=== RUN   TestAccAWSInstance_creditSpecification_unlimitedCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (89.98s)
=== RUN   TestAccAWSInstance_creditSpecification_updateCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (113.34s)
=== RUN   TestAccAWSInstance_creditSpecification_removalReturnsStandard
--- PASS: TestAccAWSInstance_creditSpecification_removalReturnsStandard (88.56s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	496.363s

However I'm getting some failures on tests (even on master and also unrelated to my changes) with some aws_instance (both resource and data resource). How would you like me to proceed?

bflad · 2018-04-20T12:51:35Z

However I'm getting some failures on tests (even on master and also unrelated to my changes) with some aws_instance (both resource and data resource). How would you like me to proceed?

Are they related to the aws_security_group and aws_route resources? They are known to be flakey due to EC2 eventual consistency. Some are already noted in bug reports across this repository. It'd be helpful to know what you're seeing to make a determination though.

I'll kick off a full acceptance test run now and ensure nothing here is breaking backwards compatibility.

kl4w · 2018-04-20T12:58:22Z

Yeah, both of those resources. Also, this is on latest master (all the data resource tests fail)

TF_SCHEMA_PANIC_ON_ERROR=1 make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_basic'gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_basic -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_basic
panic: Invalid address to set: []string{"disable_api_termination"}

bflad · 2018-04-20T13:25:41Z

panic: Invalid address to set: []string{"disable_api_termination"}

We'll address this separately 😉

bflad

LGTM! Another great PR, @kl4w! 🚀

Test failures unrelated:

Tests failed: 2 (2 new), passed: 67
=== RUN   TestAccAWSInstance_importInDefaultVpcBySgId
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgId (171.13s)
=== RUN   TestAccAWSInstanceDataSource_gp2IopsDevice
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (200.82s)
=== RUN   TestAWSInstanceMigrateState
--- PASS: TestAWSInstanceMigrateState (0.00s)
=== RUN   TestAWSInstanceMigrateState_empty
--- PASS: TestAWSInstanceMigrateState_empty (0.00s)
=== RUN   TestAccAWSInstanceDataSource_rootInstanceStore
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (206.63s)
=== RUN   TestAccAWSInstanceDataSource_blockDevices
--- PASS: TestAccAWSInstanceDataSource_blockDevices (217.94s)
=== RUN   TestAccAWSInstanceDataSource_AzUserData
--- PASS: TestAccAWSInstanceDataSource_AzUserData (241.73s)
=== RUN   TestAccAWSInstance_importInEc2Classic
--- PASS: TestAccAWSInstance_importInEc2Classic (101.30s)
=== RUN   TestAccAWSInstanceDataSource_keyPair
--- PASS: TestAccAWSInstanceDataSource_keyPair (276.50s)
=== RUN   TestAccAWSInstanceDataSource_tags
--- PASS: TestAccAWSInstanceDataSource_tags (282.41s)
=== RUN   TestAccAWSInstanceDataSource_VPC
--- PASS: TestAccAWSInstanceDataSource_VPC (290.85s)
=== RUN   TestAccAWSInstanceDataSource_privateIP
--- PASS: TestAccAWSInstanceDataSource_privateIP (307.23s)
=== RUN   TestAccAWSInstance_GP2IopsDevice
--- PASS: TestAccAWSInstance_GP2IopsDevice (101.54s)
=== RUN   TestAccAWSInstanceDataSource_basic
--- PASS: TestAccAWSInstanceDataSource_basic (340.56s)
=== RUN   TestAccAWSInstanceDataSource_VPCSecurityGroups
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (343.86s)
=== RUN   TestAccAWSInstance_GP2WithIopsValue
--- PASS: TestAccAWSInstance_GP2WithIopsValue (102.76s)
=== RUN   TestAccAWSInstance_blockDevices
--- PASS: TestAccAWSInstance_blockDevices (96.66s)
=== RUN   TestAccAWSInstance_rootInstanceStore
--- PASS: TestAccAWSInstance_rootInstanceStore (101.73s)
=== RUN   TestAccAWSInstanceDataSource_creditSpecification
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (378.84s)
=== RUN   TestAccAWSInstance_importInDefaultVpcBySgName
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgName (396.66s)
=== RUN   TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError
--- PASS: TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError (77.83s)
=== RUN   TestAccAWSInstance_userDataBase64
--- PASS: TestAccAWSInstance_userDataBase64 (239.05s)
=== RUN   TestAccAWSInstance_vpc
--- PASS: TestAccAWSInstance_vpc (150.44s)
=== RUN   TestAccAWSInstanceDataSource_SecurityGroups
--- PASS: TestAccAWSInstanceDataSource_SecurityGroups (486.93s)
=== RUN   TestAccAWSInstance_importBasic
--- PASS: TestAccAWSInstance_importBasic (538.37s)
=== RUN   TestAccAWSInstance_placementGroup
--- PASS: TestAccAWSInstance_placementGroup (235.32s)
=== RUN   TestAccAWSInstance_multipleRegions
--- PASS: TestAccAWSInstance_multipleRegions (198.34s)
=== RUN   TestAccAWSInstance_ipv6_supportAddressCountWithIpv4
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (222.06s)
=== RUN   TestAccAWSInstance_basic
--- PASS: TestAccAWSInstance_basic (395.98s)
=== RUN   TestAccAWSInstance_tags
--- PASS: TestAccAWSInstance_tags (170.62s)
=== RUN   TestAccAWSInstancesDataSource_basic
--- PASS: TestAccAWSInstancesDataSource_basic (626.53s)
=== RUN   TestAccAWSInstance_ipv6_supportAddressCount
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (298.70s)
=== RUN   TestAccAWSInstance_keyPairCheck
--- PASS: TestAccAWSInstance_keyPairCheck (89.06s)
=== RUN   TestAccAWSInstance_volumeTags
--- PASS: TestAccAWSInstance_volumeTags (271.76s)
=== RUN   TestAccAWSInstance_volumeTagsComputed
--- PASS: TestAccAWSInstance_volumeTagsComputed (310.21s)
=== RUN   TestAccAWSInstance_withIamInstanceProfile
--- PASS: TestAccAWSInstance_withIamInstanceProfile (235.00s)
=== RUN   TestAccAWSInstance_privateIP
--- PASS: TestAccAWSInstance_privateIP (262.07s)
=== RUN   TestAccAWSInstance_rootBlockDeviceMismatch
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (232.90s)
=== RUN   TestAccAWSInstance_forceNewAndTagsDrift
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (253.23s)
=== RUN   TestAccAWSInstance_instanceProfileChange
--- PASS: TestAccAWSInstance_instanceProfileChange (372.28s)
=== RUN   TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck
--- PASS: TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck (172.18s)
=== RUN   TestAccAWSInstance_disableApiTermination
--- PASS: TestAccAWSInstance_disableApiTermination (696.36s)
=== RUN   TestAccAWSInstance_associatePublicIPAndPrivateIP
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (446.04s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPrivate
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (198.72s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPublic
--- FAIL: TestAccAWSInstance_associatePublic_defaultPublic (236.10s)
    testing.go:518: Step 0 error: Check failed: Check 2/3 error: aws_instance.foo: Attribute 'associate_public_ip_address' expected "true", got "false"

=== RUN   TestAccAWSInstance_changeInstanceType
--- PASS: TestAccAWSInstance_changeInstanceType (444.06s)
=== RUN   TestAccAWSInstance_NetworkInstanceRemovingAllSecurityGroups
--- PASS: TestAccAWSInstance_NetworkInstanceRemovingAllSecurityGroups (690.35s)
=== RUN   TestAccAWSInstance_addSecondaryInterface
--- PASS: TestAccAWSInstance_addSecondaryInterface (333.47s)
=== RUN   TestAccAWSInstance_primaryNetworkInterface
--- PASS: TestAccAWSInstance_primaryNetworkInterface (447.25s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPublic
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (278.83s)
=== RUN   TestAccAWSInstance_associatePublic_overridePublic
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (253.69s)
=== RUN   TestAccAWSInstanceDataSource_PlacementGroup
--- PASS: TestAccAWSInstanceDataSource_PlacementGroup (1216.15s)
=== RUN   TestAccAWSInstance_creditSpecification_standardCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (133.81s)
=== RUN   TestAccAWSInstance_noAMIEphemeralDevices
--- PASS: TestAccAWSInstance_noAMIEphemeralDevices (977.35s)
=== RUN   TestAccAWSInstance_addSecurityGroupNetworkInterface
--- PASS: TestAccAWSInstance_addSecurityGroupNetworkInterface (460.06s)
=== RUN   TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard
--- PASS: TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard (193.76s)
=== RUN   TestAccAWSInstanceDataSource_getPasswordData_falseToTrue
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_falseToTrue (1288.35s)
=== RUN   TestAccAWSInstance_creditSpecification_updateCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (206.50s)
=== RUN   TestAccAWSInstanceDataSource_getPasswordData_trueToFalse
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_trueToFalse (1348.50s)
=== RUN   TestAccAWSInstance_associatePublic_overridePrivate
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (345.78s)
=== RUN   TestAccAWSInstance_NetworkInstanceSecurityGroups
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (982.66s)
=== RUN   TestAccAWSInstance_creditSpecification_removalReturnsStandard
--- PASS: TestAccAWSInstance_creditSpecification_removalReturnsStandard (257.10s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPrivate
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (492.94s)
=== RUN   TestAccAWSInstance_creditSpecification_unlimitedCpuCredits
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (328.52s)
=== RUN   TestAccAWSInstance_sourceDestCheck
--- PASS: TestAccAWSInstance_sourceDestCheck (1173.94s)
=== RUN   TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs
--- FAIL: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (1080.88s)
    testing.go:518: Step 0 error: After applying this step, the plan was not empty:
        
        DIFF:
        
        UPDATE: aws_security_group.tf_test_foo
          ingress.#:                             "0" => "1"
          ingress.1799340084.cidr_blocks.#:      "0" => "1"
          ingress.1799340084.cidr_blocks.0:      "" => "0.0.0.0/0"
          ingress.1799340084.description:        "" => ""
          ingress.1799340084.from_port:          "" => "-1"
          ingress.1799340084.ipv6_cidr_blocks.#: "0" => "0"
          ingress.1799340084.protocol:           "" => "icmp"
          ingress.1799340084.security_groups.#:  "0" => "0"
          ingress.1799340084.self:               "" => "false"
          ingress.1799340084.to_port:            "" => "-1"

=== RUN   TestAccAWSInstancesDataSource_tags
--- PASS: TestAccAWSInstancesDataSource_tags (1537.26s)
=== RUN   TestAccAWSInstance_getPasswordData_trueToFalse
--- PASS: TestAccAWSInstance_getPasswordData_trueToFalse (543.83s)
=== RUN   TestAccAWSInstance_getPasswordData_falseToTrue
--- PASS: TestAccAWSInstance_getPasswordData_falseToTrue (568.82s)

radeksimko · 2018-04-23T07:11:58Z

Just to reiterate on a couple of questions in this thread, specifically from @GoodMirek and @rbnrye :

I did mark a couple of comments as off-topic and didn't provide further explanation - apologies for that. I'll be sure to do this in future where these comments are non-trivial.

Generally speaking using reactions is always a preferred way to express interest (or other emotions) as we can use these when sorting PRs and issues easily.

Comments along the lines of "+1" or "when will this ship" whilst seeming helpful send everybody involved a notification which causes issues across a larger repository (such that we ask that folks use the reactions to indicate you're interested in a feature). We're taking advantage of the new "off-topic" feature in PR's and Issues to try and make it clearer what the remaining state of the PR is (e.g. any comments from PR review, discussions about how it should be implemented etc.).

TL;DR: We do appreciate people expressing interest in PRs and helping us prioritise, but comments are not the best way to do so, for the reasons mentioned.

bflad · 2018-04-25T15:54:07Z

This has been released in version 1.16.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

jleonar · 2018-04-26T18:45:14Z

I just wanted to let you know that this change breaks deployments into the GovCloud region of us-gov-west-1.

Haven't tested elsewhere in GovCloud

This is the error that I receive.

Response ec2/DescribeInstanceCreditSpecifications Details: 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: ---[ RESPONSE ]-------------------------------------- 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: HTTP/1.1 400 Bad Request 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: Connection: close 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: Transfer-Encoding: chunked 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: Date: Thu, 26 Apr 2018 18:17:02 GMT 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: Server: AmazonEC2 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: 2018-04-26T14:17:02.994-0400 [DEBUG] plugin.terraform-provider-aws_v1.16.0_x4: -----------------------------------------------------

bflad · 2018-04-26T20:04:59Z

The fix for GovCloud and China (or really anything throwing UnsupportedOperation with the new call) was made in #4362, merged into master, and will release with v1.17.0 of the AWS provider middle of next week.

ghost · 2020-04-06T17:29:46Z

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

kl4w changed the title ~~add T2 unlimited to aws instance resources~~ r/aws_instance: add T2 unlimited to aws instance resources Dec 12, 2017

radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 12, 2017

radeksimko reviewed Dec 12, 2017

View reviewed changes

radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Dec 12, 2017

kylelaverty mentioned this pull request Jan 3, 2018

Support t2.unlimited #2491

Closed

radeksimko reviewed Jan 4, 2018

View reviewed changes

kl4w force-pushed the t2_unlimited branch from 4804caa to 270e83a Compare January 6, 2018 01:47

bflad added service/ec2 Issues and PRs that pertain to the ec2 service. and removed waiting-response Maintainers are waiting on response from community or contributor. labels Jan 23, 2018

This comment has been minimized.

Sign in to view

bflad added this to the v1.12.0 milestone Mar 2, 2018

appilon force-pushed the t2_unlimited branch from 270e83a to ac3d2ac Compare March 19, 2018 16:23

ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 19, 2018

paultyng reviewed Mar 19, 2018

View reviewed changes

bflad modified the milestones: v1.12.0, v1.13.0, v1.14.0 Mar 23, 2018

This comment has been minimized.

Sign in to view

bflad modified the milestones: v1.14.0, v1.15.0 Apr 6, 2018

bflad reviewed Apr 17, 2018

View reviewed changes

bflad modified the milestones: v1.15.0, v1.16.0 Apr 18, 2018

kl4w added 2 commits April 19, 2018 08:17

add credit specification to instance resource and data

95f85ce

cater for removal of attribute block

071ee81

kl4w force-pushed the t2_unlimited branch from ac3d2ac to 071ee81 Compare April 20, 2018 10:38

ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 20, 2018

bflad approved these changes Apr 20, 2018

View reviewed changes

bflad merged commit 633f9bd into hashicorp:master Apr 20, 2018

bflad added a commit that referenced this pull request Apr 20, 2018

Update CHANGELOG for #2619

d40aeef

kl4w deleted the t2_unlimited branch April 20, 2018 13:57

kl4w mentioned this pull request Apr 23, 2018

d/aws_instance: add missing disable_api_termination attribute #4314

Merged

DrFaust92 mentioned this pull request Apr 2, 2020

r/glacier_vault: fix policy diff + read after create + refactor tests #12645

Merged

ghost locked and limited conversation to collaborators Apr 6, 2020

r/aws_instance: add T2 unlimited to aws instance resources #2619

r/aws_instance: add T2 unlimited to aws instance resources #2619

Conversation

kl4w commented Dec 10, 2017

radeksimko left a comment

Choose a reason for hiding this comment

kl4w commented Dec 12, 2017

kl4w commented Dec 12, 2017

kylelaverty commented Jan 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kl4w commented Jan 6, 2018

dmitrypol commented Jan 22, 2018

fender4645 commented Feb 22, 2018

curator commented Mar 1, 2018

andycognitoiq commented Mar 2, 2018

This comment has been minimized.

omeid commented Mar 19, 2018

appilon commented Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

appilon commented Mar 19, 2018

kl4w commented Mar 21, 2018

This comment has been minimized.

This comment has been minimized.

GoodMirek commented Apr 10, 2018

rbnrye commented Apr 10, 2018

bflad commented Apr 17, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kl4w commented Apr 20, 2018 • edited Loading

bflad commented Apr 20, 2018

kl4w commented Apr 20, 2018

bflad commented Apr 20, 2018

bflad left a comment

Choose a reason for hiding this comment

radeksimko commented Apr 23, 2018

bflad commented Apr 25, 2018

jleonar commented Apr 26, 2018 • edited Loading

bflad commented Apr 26, 2018

ghost commented Apr 6, 2020

kylelaverty commented Jan 3, 2018 •

edited

Loading

appilon commented Mar 19, 2018 •

edited

Loading

kl4w commented Apr 20, 2018 •

edited

Loading

jleonar commented Apr 26, 2018 •

edited

Loading