Skip to content

Conversation

@seivan
Copy link
Contributor

@seivan seivan commented Dec 12, 2017

I just tested it, and it works! But so far not satisfied with naming and general documentation so I would love some feedback and discussion here.

Also pretty sure the Kubernetes based strategies could work together in sharing code as only the fetching nodes (actual discovery) seem to differ. But could be part of a larger refactoring

config/config.exs

  config :libcluster,
  topologies: [
    k8s_example: [
      strategy: Cluster.Strategy.Kubernetes.DNS,
      config: [
        service: "web-headless",
        application_name: "myapp",
        polling_interval: 1000
        ]]]

Service.yml

apiVersion: v1
kind: Service
metadata:
  labels:
    deployment: web
  name: web-headless
  namespace: default
spec:
  clusterIP: None 
  ports:
    - port: 80
  selector:
    deployment: web
#on node1
iex --name myapp@172.17.0.6 --cookie EDABUQMNEWFJWXGCAZMV -S mix

#on node2
iex --name myapp@172.17.0.7 --cookie EDABUQMNEWFJWXGCAZMV -S mix
iex(myapp@172.17.0.7)1> Node.list
[:"myapp@172.17.0.6"]

It could use some input on documentation and maybe look into naming of things. 
I just tested it, and it works great. But I am not even 50% sure on the naming as well as the general documentation. I would love some feedback and discussion here. 



```elixir
  config :libcluster,
  topologies: [
    k8s_example: [
      strategy: Cluster.Strategy.Kubernetes.DNS,
      config: [
        service: "web-headless",
        application_name: "myapp",
        polling_interval: 1000
        ]]]
```
```
apiVersion: v1
kind: Service
metadata:
  labels:
    deployment: web
  name: web-headless
  namespace: default
spec:
  clusterIP: None 
  ports:
    - port: 80
  selector:
    deployment: web
```
@bitwalker
Copy link
Owner

We should document this in the README which covers the built-in strategies, otherwise this looks good to go! Sorry for the holdup!

@seivan
Copy link
Contributor Author

seivan commented Jan 31, 2018

@bitwalker How do you feel about the naming of the config keys? We're in an akward spot where we have to balance Erlang naming conventions together with what Kubernetes names its stuff.
e.g Node

@bitwalker
Copy link
Owner

I'm fine with them as is - you've done a good job of explaining them in the strategy documentation, and some degree of conflict in naming conventions is expected, especially between Erlang and an orchestration platform like k8s. If there appears to be confusion in the future, we can address that with better documentation if needed.

@bitwalker
Copy link
Owner

You mentioned you weren't satisfied with the quality here, I haven't taken the time to do a deep review of the code, but are there specific areas you are concerned with (beyond naming)? I will make some time to do a better review

@bitwalker
Copy link
Owner

Other than the comments I've left, this looks good to me!

@seivan
Copy link
Contributor Author

seivan commented Jan 31, 2018

Alright, I'll go through and fix these. Appreciate the comments!
I might need help with the "blocking" stuff you mentioned earlier, but overall it's straightforward. 👍

@seivan
Copy link
Contributor Author

seivan commented Mar 16, 2018

@bitwalker Sorry for the delay, other stuff came up.
I'll take a look at this this weekend.

seivan added 2 commits April 19, 2018 12:10
* Changing `init/1` to be a blocking operation.
* Extracting polling_interval value into `polling_interval/1`
@seivan
Copy link
Contributor Author

seivan commented Apr 19, 2018

@bitwalker I took care of your comments. I let the formatter deal with the spaces, extracting polling_interval/1 changed the pipe(!) and made load/1 a blocking operation.

@seivan seivan changed the title PoC for DNS based service discovery on Kubernetes DNS based service discovery on Kubernetes Apr 19, 2018
@bitwalker bitwalker merged commit 7d57a42 into bitwalker:master May 8, 2018
@bitwalker
Copy link
Owner

Thanks for all your work on this!

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.

2 participants