hep-gc / shoal

A squid cache publishing and advertising tool designed to work in fast changing environments
Apache License 2.0
4 stars 8 forks source link

Client default proxy always being added on successful communication with server. #127

Closed colsond closed 7 years ago

colsond commented 8 years ago

This line: https://github.com/hep-gc/shoal/blob/master/shoal-client/shoal-client#L147

Shows that even after getting the nearest proxies from the shoal server (closest_http_proxy) it also adds default_http_proxy (defined as default_http_proxy = config.default_squid_proxy) to the cvmfs_http_proxy.

I feel like this isn't the behaviour we are looking for and could result in a lot of nodes using the config default. I'm going to look at fixing this along with #125 probably with a control flow as follows:

New VM env variables imported shoal client runs

attempts to contact shoal-server, on success set retrieved proxy and exit on failure check if the env variable contains a valid default and if so exit if no env var, set it using the default from the config file (if it is valid)

Colin and I discussed the merit of doing a basic test on the default proxys just for sanity checking in the logs. He suggested simply checking if the port was open.

rptaylor commented 8 years ago

It doesn't do any harm to add default proxies at the end of the list. It would only result in nodes using the default if the shoal-provided ones aren't available, and that would be a good thing. The more proxies in the list, the more reliable CVMFS can be generally speaking.

I think the concept of using the default proxies as "something that should work and be available to use as a fallback even if Shoal totally breaks" is important. So for Shoal to check and potentially exclude those default proxies defeats the purpose completely: if there is a problem with Shoal (i.e. the test of the default proxies doesn't work correctly) then we will be left without any proxies at all. That's exactly what the default proxy list is supposed to avoid.

We should not be quite so worried about having bad proxies in the CVMFS_HTTP_PROXY list; the CVMFS client is meant to handle that and fail over. In this case I think we should be more worried about false negatives (good squids being accidentally excluded) than false positives (bad squids being accidentally included). The recent problem was a result of not having enough proxies in the list, caused by incorrect syntax, and testing if ports were open would not have addressed that.

colsond commented 8 years ago

That makes sense to me @rptaylor. Does this mean that from a reliability standpoint we should always be concatenating the defaults to the end of the list? Leave all of the proxy rejection to the CVFMS client and just have shoal provide as much wiggle room (ie more proxies in the list) as possible.

rptaylor commented 8 years ago

Yes, I think https://github.com/hep-gc/shoal/blob/master/shoal-client/shoal-client#L147 should remain as closest + default.

That being said, functional testing of the proxies advertised by Shoal agent is an important functionality of course. Shoal server should not publish proxies known to be bad.
But I'd say the default proxies should be used as is, as a fail-safe.

colsond commented 7 years ago

Pull request #129 now reflects these changes. I've tested the client in both centos6 and centos7 environments.