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

provider/aws: AWS Redshift #3862

Merged
merged 4 commits into from
Jan 13, 2016
Merged

provider/aws: AWS Redshift #3862

merged 4 commits into from
Jan 13, 2016

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Nov 11, 2015

The first version of this PR does not support tags. The following will be supported:

Redshift Cluster

  • schema
  • CRUD functionality
  • acceptance tests
  • documentation

Redshift Subnet Groups

  • schema
  • CRUD functionality
  • acceptance tests
  • documentation
make testacc TEST=./builtin/providers/aws TESTARGS='-run=RedshiftSubnetGroup_' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=RedshiftSubnetGroup_ -timeout 90m
=== RUN   TestAccAWSRedshiftSubnetGroup_basic
--- PASS: TestAccAWSRedshiftSubnetGroup_basic (19.78s)
=== RUN   TestAccAWSRedshiftSubnetGroup_updateSubnetIds
--- PASS: TestAccAWSRedshiftSubnetGroup_updateSubnetIds (30.73s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    50.532s

Redshift Parameter Groups

  • schema
  • CRUD functionality
  • acceptance tests
  • documentation
make testacc TEST=./builtin/providers/aws TESTARGS='-run=RedshiftParameterGroup_with' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=RedshiftParameterGroup_with -timeout 90m
=== RUN   TestAccAWSRedshiftParameterGroup_withParameters
--- PASS: TestAccAWSRedshiftParameterGroup_withParameters (9.41s)
=== RUN   TestAccAWSRedshiftParameterGroup_withoutParameters
--- PASS: TestAccAWSRedshiftParameterGroup_withoutParameters (8.86s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    18.283s

Please note - there is no ability to apply parameter modifications immediately. Redshift needs a reboot to handle this

Redshift Security Groups

  • schema
  • CRUD functionality
  • acceptance tests
  • documentation
make testacc TEST=./builtin/providers/aws TESTARGS='-run=RedshiftSecurityGroup_ingress' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=RedshiftSecurityGroup_ingress -timeout 90m
=== RUN   TestAccAWSRedshiftSecurityGroup_ingressCidr
--- PASS: TestAccAWSRedshiftSecurityGroup_ingressCidr (7.15s)
=== RUN   TestAccAWSRedshiftSecurityGroup_ingressSecurityGroup
--- PASS: TestAccAWSRedshiftSecurityGroup_ingressSecurityGroup (12.21s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    19.384s

This is going to be a longer running PR as there is a lot of functionality in place. I may break this down into smaller PRs to make it easier to review - that can happen later

@stack72 stack72 changed the title [WIP] provider/aws: AWS Redshift provider/aws: AWS Redshift Nov 13, 2015
@@ -426,7 +427,36 @@ func TestExpandParameters(t *testing.T) {
}
}

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

Some unresolved conflicts here I think?

@stack72
Copy link
Contributor Author

stack72 commented Nov 26, 2015

@radeksimko apologies for this - this has now been rectified

@stack72 stack72 force-pushed the aws-redshift branch 2 times, most recently from 2dd2a64 to 1493aaf Compare November 26, 2015 10:41
@jonbca
Copy link

jonbca commented Dec 20, 2015

Just wondering if this is still "alive," or you need a hand?

Finalising the schema and acceptance tests for the Redshift Security Group's
Creation of the schema, CRUD and acceptance tests for Redshift Parameter Group
Changed the aws_redshift_security_group and aws_redshift_parameter_group
to remove the tags from the schema. Tags are a little bit more
complicated than originally though - I will revisit this later

Then added the schema, CRUD functionality and basic acceptance tests for
aws_redshift_subnet_group

Adding an acceptance test for the Update of subnet_ids in AWS Redshift Subnet Group
also removed the notion of tags from the redshift security group and
parameter group documentation until that has been implemented

Redshift Cluster CRUD and acceptance tests

Removing the Acceptance test for the Cluster Updates. You cannot delete
a cluster immediately after performing an operation on it. We would need
to add a lot of retry logic to the system to get this test to work

Adding some schema validation for RedShift cluster

Adding the last of the pieces of a first draft of the Redshift work - this is the documentation
@stack72
Copy link
Contributor Author

stack72 commented Dec 20, 2015

@jonbca this will be merged very soon

@stack72 stack72 closed this Dec 20, 2015
@stack72 stack72 reopened this Dec 20, 2015
@catsby
Copy link
Contributor

catsby commented Dec 21, 2015

@stack72 all ready for review then?

@stack72
Copy link
Contributor Author

stack72 commented Dec 21, 2015

@catsby I believe that @jen20 had already started reviewing this but he can confirm

if awsErr, ok := err.(awserr.Error); ok {
if "ClusterNotFound" == awsErr.Code() {
d.SetId("")
log.Printf("[DEBUG] Redshift Cluster (%s) not found", d.Id())
Copy link
Contributor

Choose a reason for hiding this comment

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

need this log statement before you call d.SetId(""), otherwise you'll log an "" 😄

@catsby
Copy link
Contributor

catsby commented Jan 13, 2016

Wow, a lot of work done here, very nice @stack72!

I did a review and things look good. The tests pass, pending a few tweaks (I'll link the commit in a follow up).

Going to pull this in, thanks!

catsby added a commit that referenced this pull request Jan 13, 2016
@catsby catsby merged commit 7f6624e into hashicorp:master Jan 13, 2016
catsby added a commit that referenced this pull request Jan 13, 2016
@catsby
Copy link
Contributor

catsby commented Jan 13, 2016

Touchups: 8181a4e

catsby added a commit that referenced this pull request Jan 14, 2016
* master: (721 commits)
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  Handle external state changes for Packet resources gracefully.
  provider/aws: Add profile to provider config
  Bug fixes for aws_autoscaling_schedule resource
  Update CHANGELOG.md
  provider/docker: Fix flaky integration tests
  Update CHANGELOG.md
  minor clean ups after #3862
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/google: Content field for bucket objects
  provider/google: SQL user resource, documentation & tests
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  move the 'color' assignment
  ...
omeid pushed a commit to omeid/terraform that referenced this pull request Mar 30, 2018
…h_compute_environment-sweeper

resource/aws_batch_*: Add test sweepers
@ghost
Copy link

ghost commented Apr 28, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants