Rancher strategy #27
Rancher strategy #27
Conversation
zorbash
force-pushed
the
rancher-strategy
branch
from
897c3a5
to
313b1bf
Compare
There was a problem hiding this comment.
Just a few comments, but looks good! I'm generally not a huge fan of rolling strategies into the mainline libcluster repo, as it means I have to maintain them, and if I'm not using them myself, or likely to, then it can mean they fall out of date. That said, this is pretty basic, so it should be easy to keep up to date. Thanks for the PR!
lib/strategy/rancher.ex
Outdated
| new_nodelist = MapSet.new(get_nodes(state)) | ||
| added = MapSet.difference(new_nodelist, state.meta) | ||
| removed = MapSet.difference(state.meta, new_nodelist) | ||
| new_nodelist = case Cluster.Strategy.disconnect_nodes(topology, disconnect, list_nodes, MapSet.to_list(removed)) do |
There was a problem hiding this comment.
Small nitpick:
new_nodelist =
case ...
rather than the case and assignment on one line
There was a problem hiding this comment.
I updated this part, let me know what you think.
lib/strategy/rancher.ex
Outdated
| defp get_nodes(%State{topology: topology, config: config}) do | ||
| app_name = Keyword.fetch!(config, :node_basename) | ||
|
|
||
| cond do |
There was a problem hiding this comment.
I think I'd prefer:
case Keyword.fetch!(config, :node_basename) do
app_name when is_binary(app_name) and app_name != "" ->
<happy case>
app_name ->
warn topology, "rancher strategy is selected, but :node_basename is invalid, got: #{inspect app_name}"
[]
endThere was a problem hiding this comment.
Changed according to your preference, but I also allow app_name to be configured as an Atom. Is this OK?
There was a problem hiding this comment.
Please ignore the above comment, I changed it back to be exactly as you suggested, since it's a binary in the example config of the documentation and also the Kubernetes strategy only allows binary for the equivalent config option.
zorbash
force-pushed
the
rancher-strategy
branch
from
313b1bf
to
b5febd7
Compare
This introduces a strategy for the Rancher container management platform (see: https://github.com/rancher/rancher). It leverages the metadata HTTP API to get the IPs of all the containers of a service to form a cluster.
zorbash
force-pushed
the
rancher-strategy
branch
from
b5febd7
to
99ec2ad
Compare
|
Should I also update the CHANGELOG and README with info about this change, in this PR? |
|
Sure! That would be greatly appreciated :) |
|
README and CHANGELOG files have been updated. |
|
Thanks for all the great work! |
This introduces a strategy for the Rancher container management platform.
see: https://github.com/rancher/rancher
It leverages the metadata HTTP API to get the IPs of all the containers of a service to form a cluster.