hashicorp / terraform-aws-consul

A Terraform Module for how to run Consul on AWS using Terraform and Packer
Apache License 2.0
401 stars 484 forks source link

Add environment variables to supervisord config #66

Closed lawliet89 closed 6 years ago

lawliet89 commented 6 years ago

To allow arbitrary environment variables to be used with Consul agent.

In particular, this is to enable the use of the new beta UI by setting the environment variable CONSUL_UI_BETA.

I have tested it with my setup and the environment variable is successfully passed to the Consul agent started by supervisord.

lawliet89 commented 6 years ago

I've made the changes according to your review.

lawliet89 commented 6 years ago

Trying to get Go running on my machine to run the automated tests.

The script by itself works fine. I have tested scenarios with zero value, one value, and more than one values.

I have also managed to bootstrap a new Consul cluster.

brikis98 commented 6 years ago

Code looks good, thanks! Please run the tests and paste the results. If things look good, this is good to merge!

lawliet89 commented 6 years ago

One test failed with a segfault

--- FAIL: TestConsulClusterWithEncryptionUbuntuAmi (492.09s)
    save_test_data.go:142: Marshalled JSON: "ap-south-1"
    save_test_data.go:142: Marshalled JSON: "ami-7de0cd12"
    save_test_data.go:142: Marshalled JSON: {"TerraformDir":"/tmp/TestConsulClusterWithEncryptionUbuntuAmi989509700/terraform-aws-consul/examples/example-with-encryption","Vars":{"ami_id":"ami-7de0cd12","cluster_name":"8F170g","num_clients":6,"num_servers":3},"EnvVars":{"AWS_DEFAULT_REGION":"ap-south-1"},"RetryableTerraformErrors":null,"MaxRetries":0,"TimeBetweenRetries":0}
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x74c2c9]

goroutine 22 [running]:
testing.tRunner.func1(0xc4201ec2d0)
    /usr/lib/go-1.10/src/testing/testing.go:742 +0x29d
panic(0x7af360, 0xa57b10)
    /usr/lib/go-1.10/src/runtime/panic.go:502 +0x229
terraform-aws-consul/test.createConsulClient(0xc4201ec2d0, 0xc420aa44f0, 0xd, 0xd)
    /home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:141 +0xe9
terraform-aws-consul/test.testConsulCluster(0xc4201ec2d0, 0xc420aa44f0, 0xd)
    /home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:107 +0x4f
terraform-aws-consul/test.checkConsulClusterIsWorking(0xc4201ec2d0, 0x827642, 0x10, 0xc420238a80, 0xc4204422b0, 0xa)
    /home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:98 +0xa5
terraform-aws-consul/test.runConsulClusterTest.func4()
    /home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:87 +0xdb
terraform-aws-consul/test/vendor/github.com/gruntwork-io/terratest/modules/test-structure.RunTestStage(0xc4201ec2d0, 0x824621, 0x8, 0xc420301ed0)
    /home/yongwen/go/src/terraform-aws-consul/test/vendor/github.com/gruntwork-io/terratest/modules/test-structure/test_structure.go:21 +0x20a
terraform-aws-consul/test.runConsulClusterTest(0xc4201ec2d0, 0x825e3d, 0xc, 0x82ede6, 0x20, 0x837cf2, 0x41)
    /home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:82 +0x28e
terraform-aws-consul/test.TestConsulClusterWithEncryptionUbuntuAmi(0xc4201ec2d0)
    /home/yongwen/go/src/terraform-aws-consul/test/consul_cluster_with_encryption_test.go:7 +0x78
testing.tRunner(0xc4201ec2d0, 0x83e1d8)
    /usr/lib/go-1.10/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
    /usr/lib/go-1.10/src/testing/testing.go:824 +0x2e0
exit status 2
FAIL    terraform-aws-consul/test   492.102s

Might be due to this:

TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: 
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: Error: Error refreshing state: 1 error(s) occurred:
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: 
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: * data.aws_ami.consul: 1 error(s) occurred:
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: 
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: * data.aws_ami.consul: data.aws_ami.consul: Your query returned no results. Please change your search criteria and try again.
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: 
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: 
TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 retry.go:71: Returning due to fatal error: FatalError{Underlying: exit status 1}

test.log

brikis98 commented 6 years ago

/home/yongwen/go/src/terraform-aws-consul/test/consul_helpers.go:141 +0xe9

That's a very odd place for a nil pointer dereference. Assuming the code hasn't changed, it's this line:

https://github.com/hashicorp/terraform-aws-consul/blob/master/test/consul_helpers.go#L141

Did you do dep ensure to install dependencies? Or is it possible you have some newer or older version of the Consul client installed locally where config.HttpClient defaults to nil?

TestConsulClusterWithUbuntuAmi 2018-05-18T14:12:47+08:00 command.go:98: * data.aws_ami.consul: data.aws_ami.consul: Your query returned no results. Please change your search criteria and try again.

Ah, this is just bad luck. We have public AMIs published in most regions, but this test happened to pick eu-west-3, which is a new AWS region where we haven't yet published public AMIs. If you look for calls to aws.GetRandomRegion, []string{"eu-west-3"} to the fobiddenRegions list (3rd param) as a workaround.

lawliet89 commented 6 years ago

I am on go version go1.10.1 linux/amd64 and I have used dep ensure again.

The same segfault still happens. test.log

brikis98 commented 6 years ago

Alright, I don't fully understand why it would be failing for you and not for me when I ran it before, but it seems like the easiest fix is to just rearrange the order a bit in the createConsulClient method of consul_helpers.go so that you call api.NewClient before trying to set config.HttpClient.Timeout.

lawliet89 commented 6 years ago

Rearranged the lines as you suggested (643fa5) and the test passes. test.log

brikis98 commented 6 years ago

https://github.com/hashicorp/terraform-aws-consul/releases/tag/v0.3.5