NinesStack / sidecar

Gossip-based service discovery. Docker native, but supports non-container discovery, too.
MIT License
69 stars 7 forks source link

Enable golangci lint #45

Closed mihaitodor closed 5 years ago

mihaitodor commented 5 years ago

Hey @relistan, this is a big-ish PR with quite a few cleanup changes that I'd like to get merged before I proceed with implementing #44. It was a good way for me to re-explore the code a bit and see what's in here. I'm happy to do adjust these changes in any way or not do them at all, but the main goal was to remove all warnings reported by golangci-lint and enable it for Travis builds.

Changes to review individually:

mihaitodor commented 5 years ago

@relistan Regarding the "Change the state in StateChangedEvent to be a pointer type" change, I did some testing in our dev environment with the following setup on a test host:

At this point, both haproxy-api containers have the expected config written in /etc/haproxy.cfg.

As a first experiment, I bounced a service and noticed that both haproxy-api containers have received the update and started proxying the new service instance.

Then, as a second experiment, I also took down Sidecar on one of the existing nodes in the cluster and observed updates coming into both haproxy-api containers, which seemed consistent.

I didn't observe any crashes or unexpected messages, so I believe 870816f is safe. Please let me know if you wish me to do any other experiments.