Skip to content
This repository has been archived by the owner on Jan 11, 2018. It is now read-only.

allowing for http api to be used for fleet provider #1

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
bin/*
terraform-provider-fleet

28 changes: 27 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,37 @@ A plugin for Terraform enabling it to manipulate

## Usage

This terraform plugin supports basic connections to the ETCD endpoint,
the HTTP API endpoint, and over SSH

There is minimal configuration currently supported so the ETCD and API clients
are attempting to connect directly without SSL

The configuration value 'driver' defaults to 'tunnel' but can be configured with:
* etcd
* api

EX:

```
provider "fleet" {
driver = "etcd"
endpoint = "http://192.168.0.1:4001"
}
```

```
provider "fleet" {
driver = "api"
endpoint = "http://192.168.0.1:4001"
Copy link
Contributor

Choose a reason for hiding this comment

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

driver = "api" means to use the fleet HTTP API, not frobbing etcd directly, right? If so, it's pretty unlikely the endpoint would be on port 4001 in this case.

Copy link
Author

Choose a reason for hiding this comment

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

It is, that's a bad example I didn't mean to push

}
```

There is only one resource: `fleet_unit`. Here is the first example from
[the Fleet introduction][3], transcribed to Terraform:

provider "fleet" {
tunnel_address = "IP_OR_HOSTNAME_OF_A_COREOS_HOST"
endpoint = "IP_OR_HOSTNAME_OF_A_COREOS_HOST"
}

resource "fleet_unit" "myapp" {
Expand Down
133 changes: 120 additions & 13 deletions provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,31 @@ import (
"net/url"
"strings"
"time"
"fmt"
"log"

"github.com/coreos/fleet/client"
"github.com/coreos/fleet/pkg"
"github.com/coreos/fleet/ssh"

"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"

"github.com/coreos/fleet/version"
"github.com/coreos/fleet/registry"

etcd "github.com/coreos/fleet/Godeps/_workspace/src/github.com/coreos/etcd/client"
)

const oldVersionWarning = `####################################################################
WARNING: fleetctl (%s) is older than the latest registered
version of fleet found in the cluster (%s). You are strongly
recommended to upgrade fleetctl to prevent incompatibility issues.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this message be clarified to make it more clear that it's not the fleetctl binary that needs to be updated, but the version of fleet that terraform-provider-fleet is linked against?

####################################################################
`

const defaultTimeout = time.Second*10

// retry wraps a function with retry logic. Only errors containing "timed out"
// will be retried. (authentication errors and other stuff should fail
// immediately)
Expand All @@ -31,14 +47,67 @@ func retry(f func() (interface{}, error), maxRetries int) (interface{}, error) {
return result, err
}

// getAPI returns an API to Fleet.
func getAPI(hostAddr string, maxRetries int) (client.API, error) {
if hostAddr == "" {
return nullAPI{}, nil
func checkVersion(cReg registry.ClusterRegistry) (string, bool) {
fv := version.SemVersion
lv, err := cReg.LatestDaemonVersion()
if err != nil {
log.Fatal("error attempting to check latest fleet version in Registry: %v", err)
} else if lv != nil && fv.LessThan(*lv) {
return fmt.Sprintf(oldVersionWarning, fv.String(), lv.String()), false
}
return "", true
}

func getHTTPClient(driverEndpoint string) (client.API, error) {
log.Printf("Using API connection for requests")

endpoint, err := url.Parse(driverEndpoint)

if err != nil {
return nil, err
}

trans := pkg.LoggingHTTPTransport{
Transport: http.Transport{},
}

httpClient := http.Client{
Transport: &trans,
}

return client.NewHTTPClient(&httpClient, *endpoint)
}

func getETCDClient(driverEndpoint string, etcdKeyPrefix string) (client.API, error) {
log.Printf("Using ETCD connection for requests")

trans := &http.Transport{}

eCfg := etcd.Config{
Endpoints: strings.Split(driverEndpoint, ","),
Transport: trans,
}

eClient, err := etcd.New(eCfg)
if err != nil {
return nil, err
}

kAPI := etcd.NewKeysAPI(eClient)
reg := registry.NewEtcdRegistry(kAPI, etcdKeyPrefix, defaultTimeout)

if msg, ok := checkVersion(reg); !ok {
log.Printf(msg)
}

return &client.RegistryClient{Registry: reg}, nil
}

func getTunnelClient(driverEndpoint string, maxRetries int) (client.API, error) {
log.Printf("Using Fleet Tunnel connection for requests")

getSSHClient := func() (interface{}, error) {
return ssh.NewSSHClient("core", hostAddr, nil, false, time.Second*10)
return ssh.NewSSHClient("core", driverEndpoint, nil, false, defaultTimeout)
}

result, err := retry(getSSHClient, maxRetries)
Expand All @@ -52,6 +121,15 @@ func getAPI(hostAddr string, maxRetries int) (client.API, error) {
return ssh.DialCommand(sshClient, cmd)
}

// This is needed to fake out the client - it isn't used
// since we're overloading the dial method on the transport
// but the client complains if it isn't set
fakeHttpEndpoint, err := url.Parse("http://domain-sock")

if err != nil {
return nil, err
}

trans := pkg.LoggingHTTPTransport{
Transport: http.Transport{
Dial: dial,
Expand All @@ -62,27 +140,54 @@ func getAPI(hostAddr string, maxRetries int) (client.API, error) {
Transport: &trans,
}

// since dial() ignores the endpoint, we just need something here that
// won't make the HTTP client complain.
endpoint, err := url.Parse("http://domain-sock")
return client.NewHTTPClient(&httpClient, *fakeHttpEndpoint)
}

return client.NewHTTPClient(&httpClient, *endpoint)

// getAPI returns an API to Fleet.
func getAPI(driver string, driverEndpoint string, maxRetries int, etcdKeyPrefix string) (client.API, error) {

switch strings.ToLower(driver) {
case "api":
return getHTTPClient(driverEndpoint)
case "etcd":
return getETCDClient(driverEndpoint, etcdKeyPrefix)
case "tunnel":
return getTunnelClient(driverEndpoint, maxRetries)
case "null":
fallthrough
default:
return nullAPI{}, nil
}
}

// Provider returns the ResourceProvider implemented by this package. Serve
// this with the Terraform plugin helper and you are golden.
func Provider() terraform.ResourceProvider {
return &schema.Provider{
Schema: map[string]*schema.Schema{
"tunnel_address": &schema.Schema{
"driver": &schema.Schema{
Type: schema.TypeString,
Required: true,
Optional: true,
Default: "tunnel",
Description: "Driver to use to connect to Fleet. Can be tunnel or api.",
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit at odds with the capabilities of fleetctl, where the --driver option can be "API" or "etcd".

--tunnel is a different option, and it's possible to combine with --endpoint, for instance if you want to tunnel to some host and then access the endpoint there at something other than the default unix:///var/run/fleet.sock. I think it would be good to provide a similar functionality here, and keep the configuration item names consistent with the fleetctl option names.

Copy link
Author

Choose a reason for hiding this comment

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

Gotcha, I saw the difference but wasn't sure about modifying your static setting of that tunnel socket location, I've updated that so it makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Modify all you want. I doubt many people are using this provider, so it's probably more important that we get it right than it is to maintain backwards compatibility.

I think we should more or less be able to use the code from https://github.com/coreos/fleet/blob/master/fleetctl/fleetctl.go with some modifications to get the configuration from Terraform instead of globalFlags. It's a shame it's not exported in a more easily reused way, so I guess some copy & paste could be fine. 😞

Copy link
Author

Choose a reason for hiding this comment

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

Ahh, yeah it would be good to mimic more completely how they build the client. Do you want that refactor in this PR or another one?

Copy link
Author

Choose a reason for hiding this comment

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

@bitglue hey Can I open a new PR for pulling in what the main fleetctl config is doing? I want to be sure I've got enough PR's for this: https://hacktoberfest.digitalocean.com/

Copy link
Contributor

Choose a reason for hiding this comment

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

Heh, I was trying to do hacktoberfest too. Open as many PRs as you want, as long as each one has a more or less coherent purpose.

},
"endpoint": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Description: "Endpoint for Fleet.",
},
"connection_retries": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Default: 12,
},
"etc_key_prefix": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be named etcd_key_prefix for consistency with fleetctl's options?

Type: schema.TypeString,
Optional: true,
Default: "/_coreos.com/fleet/",
Description: "EtcdKeyPrefix to use for fleet",
},
},
ResourcesMap: map[string]*schema.Resource{
"fleet_unit": resourceUnit(),
Expand All @@ -92,7 +197,9 @@ func Provider() terraform.ResourceProvider {
}

func providerConfigure(d *schema.ResourceData) (interface{}, error) {
addr := d.Get("tunnel_address").(string)
retries := d.Get("connection_retries").(int)
return getAPI(addr, retries)
driver := d.Get("driver").(string)
endpoint := d.Get("endpoint").(string)
etcKeyPrefix := d.Get("etc_key_prefix").(string)
return getAPI(driver, endpoint, retries, etcKeyPrefix)
}
2 changes: 1 addition & 1 deletion provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestGetAPI(test *testing.T) {
// when the address is an empty string, we get a nullAPI
var api client.API

api, err := getAPI("", 1)
api, err := getAPI("null", "endpoint", 1, "etcd_prefix")
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be testing "when the address is an empty string, we get a nullAPI", though maybe it doesn't matter anymore. It used to be that if the tunnel_address was something like ${aws_instance.coreos.public_ip}, then when terraform would expand that variable in the planning stage, before that instance existed, it would expand it to an empty string. When that happened, all we needed to do was to not fail, and then when Terraform would get to the apply stage it wouldn't initialize the fleet provider until after the instance was actually created, and by then we'd have the real IP address and could work normally.

However, this doesn't seem to work in more recent versions of Terraform, for reasons I haven't yet fully investigated. hashicorp/terraform#2430 has a bit more context.

Until that's sorted out, perhaps it's best to retain the old behavior.

Copy link
Author

Choose a reason for hiding this comment

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

One of the comments on that linked thread:
"""
The workaround for this problem is to explicitly split the problem into two steps: make one Terraform config that creates the EC2 instance and produces the instance IP address as an output, publish the state from that configuration somewhere, and then use the terraform_remote_state resource to reference that from a separate downstream config that sets up the Docker resources.
"""
So that's what I'm doing currently, but I'll add back in that expected functionality in the case of an empty string. I didn't understand why it was operating that way but it makes sense. I'll keep up on that ticket as well.


if err != nil {
test.Fatal(err)
Expand Down