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

Implemented RetryJoin on AzureRM #2978

Closed
wants to merge 11 commits into from
Closed

Conversation

leowmjw
Copy link
Contributor

@leowmjw leowmjw commented Apr 28, 2017

Addresses #2667; please provide feedback, suggestions and guidance. @slackpad for your consideration.

DONE:

Binaries at: https://gist.github.com/leowmjw/fe8344b5b8e7c0d000f18335774e7ef3

Changesets:

  • Azure libs for network vendored in with go vendor; including reps
  • Implemented and test passed for config decoding of RetryJoinAzure
  • Added the RetryJoinAzure struct which has the mapstructure to config file; including top level
  • Implement the merge logic for all the fields in RetryJoinAzure struct
  • Implemented and test passed for discovering of Azure Hosts using tags; for now only Public Environment
  • Added 2 additional cli config for tag name + value; credentials need to be passed via config file
  • Cleaned up discover Azure Hosts to its bare minimum and try to be as robust in handling nil pointers
  • Refactored the retryJoin so standardised to detect which is enabled ec2, gee or azure and take action accordingly

leowmjw added 2 commits April 28, 2017 20:09
Changesets:
- Azure libs for network vendored in with go vendor; including reps
- Implemented and test passed for config decoding of RetryJoinAzure
- Added the RetryJoinAzure struct which has the mapstructure to config file; including top level
- Implement the merge logic for all the fields in RetryJoinAzure struct
- Implemented and test passed for discovering of Azure Hosts using tags; for now only Public Environment
- Added 2 additional cli config for tag name + value; credentials need to be passed via config file
- Cleaned up discover Azure Hosts to its bare minimum and try to be as robust in handling nil pointers
- Refactored the retryJoin so standardised to detect which is enabled ec2, gee or azure and take action accordingly
TODO: Review of PR and update the docs and with example config.
Changesets:
- Updated docs to have the Azure Retry Join using Tags/Value
- Added details for Retry Join in options + config
TODO: Find out the minimal IAM key needed to ListAll Network Interfaces
@leowmjw leowmjw changed the title [WIP] Implemented RetryJoin on AzureRM Implemented RetryJoin on AzureRM May 2, 2017
leowmjw added 7 commits May 2, 2017 15:06
Changesets:
- Azure libs for network vendored in with go vendor; including reps
- Implemented and test passed for config decoding of RetryJoinAzure
- Added the RetryJoinAzure struct which has the mapstructure to config file; including top level
- Implement the merge logic for all the fields in RetryJoinAzure struct
- Implemented and test passed for discovering of Azure Hosts using tags; for now only Public Environment
- Added 2 additional cli config for tag name + value; credentials need to be passed via config file
- Cleaned up discover Azure Hosts to its bare minimum and try to be as robust in handling nil pointers
- Refactored the retryJoin so standardised to detect which is enabled ec2, gee or azure and take action accordingly
TODO: Review of PR and update the docs and with example config.
Changesets:
- Updated docs to have the Azure Retry Join using Tags/Value
- Added details for Retry Join in options + config
TODO: Find out the minimal IAM key needed to ListAll Network Interfaces
# Conflicts:
#	command/agent/command_test.go
@magiconair
Copy link
Contributor

@leowmjw I'm going to take a look at this. Could you please rebase your branch, resolve any conflicts and force push the changes? I've recently moved some code around. The retry/join code now lives in separate files per provider. This will make the review easier.

@magiconair magiconair self-assigned this May 15, 2017
@leowmjw
Copy link
Contributor Author

leowmjw commented May 15, 2017

@magiconair thanks for the heads-up; yeah I noticed the code being refactored :P I'll fix it up later this week when I get some time and ping you for review.

@magiconair
Copy link
Contributor

@leowmjw ping

leowmjw added 2 commits May 22, 2017 16:32
 Changesets:
- Put back removed retry-join-azure-* params
- Refactor out the discoverAzureHosts method to its own file as per same in AWS, GCE
@leowmjw
Copy link
Contributor Author

leowmjw commented May 22, 2017

@magiconair Ready for your review. Thanks!

Simple usage via Terraform can be found at: https://github.com/leowmjw/nomad-box/blob/master/terraform/templates/base.tpl

# Extract the IP address from the determined interface
CONSUL_CLIENT_INTERFACE="eth0"
CONSUL_CLIENT_ADDRESS=$(ip -o -4 addr list $CONSUL_CLIENT_INTERFACE | head -n1 | awk '{print $4}' | cut -d/ -f1)
# Use that address to setup the HTTP endpoint so that it is reachable from within Docker container
cat > ./consul.d/config.json <<EOF
{
    "addresses": {
        "http": "$${CONSUL_CLIENT_ADDRESS}"
    }
}
EOF

cat > ./consul.d/retry.json <<EOF
{
    "retry_join_azure": {
                "tag_name": "type",
                "tag_value": "Foundation",
                "subscription_id": "${vars_subscription_id}",
                "tenant_id": "${vars_tenant_id}",
                "client_id": "${vars_client_id}",
                "secret_access_key": "${vars_secret_access_key}"
        }
    }
}
EOF

# Extract the IP address from the determined interface
CONSUL_BIND_INTERFACE="eth0"
CONSUL_BIND_ADDRESS=$(ip -o -4 addr list $CONSUL_BIND_INTERFACE | head -n1 | awk '{print $4}' | cut -d/ -f1)

# Start up the Consul agent
/opt/consul/consul agent -server -ui -bootstrap-expect=${vars_bootstrap_expected} -data-dir=/tmp/consul \
  -config-dir=./consul.d -bind=$${CONSUL_BIND_ADDRESS} &

@magiconair
Copy link
Contributor

Thx. I have a look on Tue or Wed.

@magiconair
Copy link
Contributor

@leowmjw I've rebased your change again and split out the vendored libs into separate commits. Could you have a look and test whether it works, please?

@leowmjw
Copy link
Contributor Author

leowmjw commented May 25, 2017

Latest test binaries in: https://gist.github.com/leowmjw/fe8344b5b8e7c0d000f18335774e7ef3/revisions

Next to test final in Nomad Box to ensure vendor lib did not break anything.

@leowmjw
Copy link
Contributor Author

leowmjw commented May 25, 2017

@magiconair Just to confirm; it works fine; auto join cluster as per below:

$ CONSUL_HTTP_ADDR=http://acme-nomad-dev-foundation-node-2:8500 ./consul members
Node                              Address        Status  Type    Build     Protocol  DC
acme-nomad-dev-foundation-node-1  10.0.1.4:8301  alive   server  0.8.2dev  2         dc1
acme-nomad-dev-foundation-node-2  10.0.2.4:8301  alive   server  0.8.2dev  2         dc1
acme-nomad-dev-foundation-node-3  10.0.3.4:8301  alive   server  0.8.2dev  2         dc1

@slackpad
Copy link
Contributor

Thank you @leowmjw - appreciate the final check!

@magiconair
Copy link
Contributor

Awesome @leowmjw . Thanks for the patch and for the testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants