Skip to content

Commit

Permalink
Upgrade ContainerPilot to Go 1.9.x and Consul 1.0.0 (#527)
Browse files Browse the repository at this point in the history
* Update dependencies and peg Consul at v1.0.0
* tools: Switch builds and tooling to use Go 1.9.x and Consul 1.0.0
* testing: Don't cause NPEs by ignoring errors when configuring Consul
* testing: Custom TestServer under `discovery` for exec'ing `consul`
* discovery: Deprecate usage of Consul's internal `testutil` package

This commit upgrades ContainerPilot to be built with Go 1.9 and tested against 
Consul 1.0.0.

While upgrading Consul to version 1.0.0 I found that our tests caused a null
pointer exception if there's an error configuring a Consul process to test
against. This was due to the fact that we were ignoring errors when configuring
Consul and continuing forward as if Consul was functional. This commit properly
causes a fatal return from the test and includes the actual error returned by
Consul's own internal testing framework (which we use for bootstrapping our
own testing).

Finally, this commit removes the use case within the discovery package's tests that
depended on an internal Consul test package, `testutil`. We replace this with
our own TestServer object which is responsible for executing a locally installed
`consul` binary. Consul is installed by our Makefile target `tools` for local
development use.

Fixes: #528
  • Loading branch information
Justin Reagor authored Nov 14, 2017
1 parent bda8eba commit 4f9845a
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 112 deletions.
9 changes: 6 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
FROM golang:1.8
FROM golang:1.9

ENV CONSUL_VERSION=1.0.0
ENV GLIDE_VERSION=0.12.3

RUN apt-get update \
&& apt-get install -y unzip \
&& go get github.com/golang/lint/golint \
&& curl -Lo /tmp/glide.tgz "https://github.com/Masterminds/glide/releases/download/v0.12.3/glide-v0.12.3-linux-amd64.tar.gz" \
&& curl -Lo /tmp/glide.tgz "https://github.com/Masterminds/glide/releases/download/v${GLIDE_VERSION}/glide-v${GLIDE_VERSION}-linux-amd64.tar.gz" \
&& tar -C /usr/bin -xzf /tmp/glide.tgz --strip=1 linux-amd64/glide \
&& curl --fail -Lso consul.zip "https://releases.hashicorp.com/consul/0.7.5/consul_0.7.5_linux_amd64.zip" \
&& curl --fail -Lso consul.zip "https://releases.hashicorp.com/consul/${CONSUL_VERSION}/consul_${CONSUL_VERSION}_linux_amd64.zip" \
&& unzip consul.zip -d /usr/bin

ENV CGO_ENABLED 0
Expand Down
185 changes: 94 additions & 91 deletions discovery/consul_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

consul "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/testutil"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -71,113 +70,117 @@ func TestCheckForChanges(t *testing.T) {
assert.True(t, didChange, "value for 'didChange' after t3")
}

/*
The TestWithConsul suite of tests uses Hashicorp's own testutil for managing
a Consul server for testing. The 'consul' binary must be in the $PATH
ref https://github.com/hashicorp/consul/tree/master/testutil
*/

var testServer *testutil.TestServer

func TestWithConsul(t *testing.T) {
testServer, _ = testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) {
c.LogLevel = "err"
})
testServer, err := NewTestServer(8500)
if err != nil {
t.Fatal(err)
}
defer testServer.Stop()
t.Run("TestConsulTTLPass", testConsulTTLPass)
t.Run("TestConsulReregister", testConsulReregister)
t.Run("TestConsulCheckForChanges", testConsulCheckForChanges)
t.Run("TestConsulEnableTagOverride", testConsulEnableTagOverride)

testServer.WaitForAPI()

t.Run("TestConsulTTLPass", testConsulTTLPass(testServer))
t.Run("TestConsulReregister", testConsulReregister(testServer))
t.Run("TestConsulCheckForChanges", testConsulCheckForChanges(testServer))
t.Run("TestConsulEnableTagOverride", testConsulEnableTagOverride(testServer))
}

func testConsulTTLPass(t *testing.T) {
consul, _ := NewConsul(testServer.HTTPAddr)
name := fmt.Sprintf("TestConsulTTLPass")
service := generateServiceDefinition(name, consul)
checkID := fmt.Sprintf("service:%s", service.ID)

service.SendHeartbeat() // force registration and 1st heartbeat
checks, _ := consul.Agent().Checks()
check := checks[checkID]
if check.Status != "passing" {
t.Fatalf("status of check %s should be 'passing' but is %s", checkID, check.Status)
func testConsulTTLPass(testServer *TestServer) func(*testing.T) {
return func(t *testing.T) {
consul, _ := NewConsul(testServer.HTTPAddr)
name := fmt.Sprintf("TestConsulTTLPass")
service := generateServiceDefinition(name, consul)
checkID := fmt.Sprintf("service:%s", service.ID)

service.SendHeartbeat() // force registration and 1st heartbeat
checks, _ := consul.Agent().Checks()
check := checks[checkID]
if check.Status != "passing" {
t.Fatalf("status of check %s should be 'passing' but is %s", checkID, check.Status)
}
}
}

func testConsulReregister(t *testing.T) {
consul, _ := NewConsul(testServer.HTTPAddr)
name := fmt.Sprintf("TestConsulReregister")
service := generateServiceDefinition(name, consul)
id := service.ID

service.SendHeartbeat() // force registration and 1st heartbeat
services, _ := consul.Agent().Services()
svc := services[id]
if svc.Address != "192.168.1.1" {
t.Fatalf("service address should be '192.168.1.1' but is %s", svc.Address)
}
func testConsulReregister(testServer *TestServer) func(*testing.T) {
return func(t *testing.T) {
consul, _ := NewConsul(testServer.HTTPAddr)
name := fmt.Sprintf("TestConsulReregister")
service := generateServiceDefinition(name, consul)
id := service.ID

service.SendHeartbeat() // force registration and 1st heartbeat
services, _ := consul.Agent().Services()
svc := services[id]
if svc.Address != "192.168.1.1" {
t.Fatalf("service address should be '192.168.1.1' but is %s", svc.Address)
}

// new Consul client (as though we've restarted)
consul, _ = NewConsul(testServer.HTTPAddr)
service = generateServiceDefinition(name, consul)
service.IPAddress = "192.168.1.2"
service.SendHeartbeat() // force re-registration and 1st heartbeat
// new Consul client (as though we've restarted)
consul, _ = NewConsul(testServer.HTTPAddr)
service = generateServiceDefinition(name, consul)
service.IPAddress = "192.168.1.2"
service.SendHeartbeat() // force re-registration and 1st heartbeat

services, _ = consul.Agent().Services()
svc = services[id]
if svc.Address != "192.168.1.2" {
t.Fatalf("service address should be '192.168.1.2' but is %s", svc.Address)
services, _ = consul.Agent().Services()
svc = services[id]
if svc.Address != "192.168.1.2" {
t.Fatalf("service address should be '192.168.1.2' but is %s", svc.Address)
}
}
}

func testConsulCheckForChanges(t *testing.T) {
backend := fmt.Sprintf("TestConsulCheckForChanges")
consul, _ := NewConsul(testServer.HTTPAddr)
service := generateServiceDefinition(backend, consul)
id := service.ID
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Fatalf("First read of %s should show `false` for change", id)
}
service.SendHeartbeat() // force registration and 1st heartbeat
func testConsulCheckForChanges(testServer *TestServer) func(*testing.T) {
return func(t *testing.T) {
backend := fmt.Sprintf("TestConsulCheckForChanges")
consul, _ := NewConsul(testServer.HTTPAddr)
service := generateServiceDefinition(backend, consul)
id := service.ID
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Fatalf("First read of %s should show `false` for change", id)
}
service.SendHeartbeat() // force registration and 1st heartbeat

if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); !changed {
t.Errorf("%v should have changed after first health check TTL", id)
}
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Errorf("%v should not have changed without TTL expiring", id)
}
check := fmt.Sprintf("service:TestConsulCheckForChanges")
consul.Agent().UpdateTTL(check, "expired", "critical")
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); !changed {
t.Errorf("%v should have changed after TTL expired.", id)
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); !changed {
t.Errorf("%v should have changed after first health check TTL", id)
}
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Errorf("%v should not have changed without TTL expiring", id)
}
check := fmt.Sprintf("service:TestConsulCheckForChanges")
consul.Agent().UpdateTTL(check, "expired", "critical")
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); !changed {
t.Errorf("%v should have changed after TTL expired.", id)
}
}
}

func testConsulEnableTagOverride(t *testing.T) {
backend := fmt.Sprintf("TestConsulEnableTagOverride")
consul, _ := NewConsul(testServer.HTTPAddr)
service := &ServiceDefinition{
ID: backend,
Name: backend,
IPAddress: "192.168.1.1",
TTL: 1,
Port: 9000,
EnableTagOverride: true,
Consul: consul,
}
id := service.ID
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Fatalf("First read of %s should show `false` for change", id)
}
service.SendHeartbeat() // force registration
catalogService, _, err := consul.Catalog().Service(id, "", nil)
if err != nil {
t.Fatalf("error finding service: %v", err)
}
func testConsulEnableTagOverride(testServer *TestServer) func(*testing.T) {
return func(t *testing.T) {
backend := fmt.Sprintf("TestConsulEnableTagOverride")
consul, _ := NewConsul(testServer.HTTPAddr)
service := &ServiceDefinition{
ID: backend,
Name: backend,
IPAddress: "192.168.1.1",
TTL: 1,
Port: 9000,
EnableTagOverride: true,
Consul: consul,
}
id := service.ID
if changed, _ := consul.CheckForUpstreamChanges(backend, "", ""); changed {
t.Fatalf("First read of %s should show `false` for change", id)
}
service.SendHeartbeat() // force registration
catalogService, _, err := consul.Catalog().Service(id, "", nil)
if err != nil {
t.Fatalf("error finding service: %v", err)
}

for _, service := range catalogService {
if service.ServiceEnableTagOverride != true {
t.Errorf("%v should have had EnableTagOverride set to true", id)
for _, service := range catalogService {
if service.ServiceEnableTagOverride != true {
t.Errorf("%v should have had EnableTagOverride set to true", id)
}
}
}
}
Expand Down
90 changes: 90 additions & 0 deletions discovery/test_server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package discovery

import (
"errors"
"fmt"
"io"
"net/http"
"os"
"os/exec"
"strconv"

"github.com/hashicorp/consul/testutil/retry"
cleanhttp "github.com/hashicorp/go-cleanhttp"
)

// TestServer represents a Consul server we can run our tests against. Depends
// on a local `consul` binary installed into our environ's PATH.
type TestServer struct {
HTTPAddr string
cmd *exec.Cmd
client *http.Client
}

// NewTestServer constructs a new TestServer by including the httpPort as well.
func NewTestServer(httpPort int) (*TestServer, error) {
path, err := exec.LookPath("consul")
if err != nil || path == "" {
return nil, fmt.Errorf("consul not found on $PATH - download and install " +
"consul or skip this test")
}

args := []string{"agent", "-dev", "-http-port", strconv.Itoa(httpPort)}
cmd := exec.Command("consul", args...)
cmd.Stdout = io.Writer(os.Stdout)
cmd.Stderr = io.Writer(os.Stderr)
if err := cmd.Start(); err != nil {
return nil, errors.New("failed starting command")
}

httpAddr := fmt.Sprintf("127.0.0.1:%d", httpPort)

client := cleanhttp.DefaultClient()

return &TestServer{httpAddr, cmd, client}, nil
}

// Stop stops a TestServer
func (s *TestServer) Stop() error {
if s.cmd == nil {
return nil
}

if s.cmd.Process != nil {
if err := s.cmd.Process.Signal(os.Interrupt); err != nil {
return errors.New("failed to kill consul server")
}
}

return s.cmd.Wait()
}

// failer implements the retry.Failer interface
type failer struct {
failed bool
}

func (f *failer) Log(args ...interface{}) { fmt.Println(args) }
func (f *failer) FailNow() { f.failed = true }

// WaitForAPI waits for only the agent HTTP endpoint to start responding. This
// is an indication that the agent has started, but will likely return before a
// leader is elected.
func (s *TestServer) WaitForAPI() error {
f := &failer{}
retry.Run(f, func(r *retry.R) {
resp, err := s.client.Get(s.HTTPAddr + "/v1/agent/self")
if err != nil {
r.Fatal(err)
}
defer resp.Body.Close()

if resp.StatusCode != 200 {
r.Fatalf("bad status code %d", resp.StatusCode)
}
})
if f.failed {
return errors.New("failed waiting for API")
}
return nil
}
27 changes: 13 additions & 14 deletions glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4f9845a

Please sign in to comment.