bobtfish / AWSnycast

'Anycast' in AWS
Apache License 2.0
229 stars 25 forks source link

Use SSL to do TCP Healthchecks #5

Closed tsheasha closed 8 years ago

tsheasha commented 8 years ago

This attempts to solve #3.

tsheasha commented 8 years ago

@bobtfish I have fixed the tests and added loads more. The coverage fails for some reason i am oblivious to. However, I have managed to do a 100% coverage of heatlhcheck/. Running tests locally was non trivial for me to do frequently so I *abused travis for that by doing commits and testing using it.

I plan to make the testing locally more automated and trivial :)

bobtfish commented 8 years ago

This looks good, except for the fact that I think the SSL settings need to be per healthcheck, not an overall flag.

For example at Yelp we have apt package repositories (not SSL) and puppet (SSL) both being healthchecked by the same instance of AWSnycast - so it needs to be configurable in the config file on a per check basis.

tsheasha commented 8 years ago

You are right, I pushed https://github.com/tsheasha/AWSnycast/commit/73b447b4808f075b5c382b69fcb4c8f8ac35ff5e to make the ssl option per healthcheck and not global

tsheasha commented 8 years ago

@bobtfish that is true, since as per https://github.com/bobtfish/AWSnycast/blob/96e268338de8ced9032adf5771fc6750797fccd2/healthcheck/healthcheck.go#L41 and https://github.com/bobtfish/AWSnycast/blob/96e268338de8ced9032adf5771fc6750797fccd2/config/config.go#L15-L26 we unmarshal the yaml into the healthcheck struct which in turn unmarshals the config as a map[string]string. So basically it reads all the yaml content in config as a string, so yes the yaml boolean becomes a golang String of "true" | "false".

I have created #6 in order to follow up on that. This should help in the cleanup you were saying.