New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add rancher-compose sub-generator #5159

Merged
merged 22 commits into from Apr 4, 2017

Conversation

Projects
None yet
7 participants
@stevehouel
Copy link
Contributor

stevehouel commented Feb 9, 2017

implement new rancher-compose sub-generator who will create docker-compose and rancher-compose specific files depending on their configuration. Il also provide a way to add the default rancher loadbalancer for front part (gateway or monolith app).

Fix #5138

HOUEL Steve
add rancher-compose sub-generator
implement new rancher-compose sub-generator who will create docker-compose and rancher-compose specific files depending on their configuration. Il also provide a way to add the default rancher loadbalancer for front part (gateway or monolith app).

Fix #5138
@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Feb 9, 2017

Thanks for the contribution @stevehouel !

We will have to test it with many configurations before merging. Also we should mark it as BETA.
I'm a bit worried about code duplication. Especially as we are already changing the docker-compose configuration for example by removing things like "container_name".

From the docs, I thought that Rancher only supported the docker-compose v1 syntax. But if it can support v2 and beyond maybe we could share part of the code with the docker subgen.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 9, 2017

@PierreBesson You'r Welcome :)
For your information Rancher is based on libcompose lib (https://github.com/docker/libcompose) who is supporting docker-compose v2 syntax since April. It does not support v3 yet that why I did some duplicate code due to issue #5135, I did not want to create some dependencies between those two (docker-compose v3 have some really nice features as service restart etc). But if you want to merge both I can do it.

Sorry for the BETA I just add it on the sayHello text. Could you tell me where to add it ? For my test I check on the last rancher version. I stay in touch when you will check those configs on your Rancher.

@@ -0,0 +1,382 @@
/*
* Copyright 2016 Google Inc. All Rights Reserved.

This comment has been minimized.

@jdubois

jdubois Feb 9, 2017

Member

why do you have a Google copyright here?

This comment has been minimized.

@stevehouel

stevehouel Feb 10, 2017

Author Contributor

My bad. I have used k8n subgen as ref so I did not check it. I will remove it

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Feb 15, 2017

Yes this code duplication is annoying, but as they are not exactly the same things, it's going to be hard to avoid... @stevehouel can you check if you could have less duplication, and re-use the existing Docker/Kubernetes features? I'm not sure if that's possible, so don't hesitate to send feedback on this.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 15, 2017

Ok @jdubois I will try to do some refactor with existing Docker-Compose feature. I did not use any code from k8n part. Just to be clear, do I keep in mind that docker-compose subgen and mine will have different lifecycle ?

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Feb 17, 2017

Yes you will have different lifecycles, then if you have a lot of common code it's really not good to copy/paste everything

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 17, 2017

@deepu105 deepu105 referenced this pull request Feb 17, 2017

Merged

Rancher logo #394

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Feb 17, 2017

@stevehouel I have started to work on some changes that I want to do on your branch. I will do a PR on your fork next week. It will include the following changes :

  • remove container names and external links
  • remove prometheus monitoring as it doesn't work because it cannot load it's config
  • fix consul
@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 20, 2017

Ok @PierreBesson, I am waiting your PR to merge everything and update that one.

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Feb 20, 2017

I am still working on this. This is really interesting... I'm working on some sidekick containers to load the config as I need to load a config file for consul to work properly.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 24, 2017

@PierreBesson Any news ? Need some help for what you are doing ?

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Feb 24, 2017

@stevehouel I did some progress but I was busy at work this week. I have found that there is still quite a bit of work to do on this subgen, especially for microservices support (setup registries with config, etc...). So there are definitely many things to polish here...

I will do my PR when I have the time. In the meantime if you want to do some work on this you can prepare the documentation page on the jhipster/jhipster.github.io repo. This is very important and the team doesn't have a lot of time to do it so I'm trusting you on this. Thank you.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Feb 27, 2017

Yes @PierreBesson I already start documentation on this subgen. If you need some helps for config I am here :) .

@cbornet

This comment has been minimized.

Copy link
Member

cbornet commented Mar 8, 2017

Since Rancher has it's own service discovery, could it be used as an alternative to eureka/consul ? The interest for me is that it would allow to add non-JHipster/Java services easily (Consul can be used but you still need a Consul-aware LB)

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Mar 8, 2017

@cbornet I will check it and try to bind JHipster to it. But I think this shoud be an other update to my PR. I keep you all in touch about that

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Mar 8, 2017

We need to have a service discovery that is compatible with Spring Cloud -> we use the same Spring Cloud API for Eureka and Consul. For example, this is needed for scaling Hazelcast.
So for me the JHipster Registry should be a better option, and we should have it on the Rancher store.

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Mar 8, 2017

@cbornet In any case if we support other service discovery systems it should be supported by the Spring Cloud abstraction so that we don't have to do too much work to integrate with it.

Kubernetes would be a good candidate as some people have already worked on this (see https://github.com/fabric8io/spring-cloud-kubernetes ). An alternative is to use no service discovery at all and rely on Ribbon serverList, (See my stackoverflow post).

However I agree with @jdubois, we should really push people to use the Registry as it works everywhere (also lots of cool features are on their way... 😄 ).

@stevehouel I'm finishing my PR for the rancher-compose subgen as soon as I can. Have you made progress on the docs ?

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Mar 8, 2017

@PierreBesson Yes my PR is ready for the doc, just waiting your update to finalize it with configuration part.

@cbornet

This comment has been minimized.

Copy link
Member

cbornet commented Mar 8, 2017

@PierreBesson @jdubois agreed that we should use only Spring-Cloud compatible service discoveries. Now the problem I have with JHipster Registry/Eureka/Ribbon is that you're bound to JVM languages or you need dedicated client code in other languages. At least Consul makes it more technology transparent.

@deepu105

This comment has been minimized.

Copy link
Member

deepu105 commented Mar 14, 2017

Guys I'm sorry but this sub generator would need some updates to be compatible with the new lint rules and yeoman 1.x . can you plz rebase @stevehouel

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Mar 14, 2017

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Mar 27, 2017

FYI I'm including a slide on this for my Devoxx France talk :-)

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Mar 27, 2017

Great !! Thx @jdubois

@DupontC

This comment has been minimized.

Copy link

DupontC commented on 6f2304a Mar 29, 2017

Good ;-)

@deepu105

This comment has been minimized.

Copy link
Member

deepu105 commented Apr 2, 2017

@jdubois @stevehouel @PierreBesson why is this on hold?

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 2, 2017

@pascalgrimaud

This comment has been minimized.

Copy link
Member

pascalgrimaud commented Apr 2, 2017

@stevehouel : don't put to much effort on Cassandra. In the Kubernetes sub generator (still in Beta), we didn't support Cassandra yet, and I'm not sure we'll do it in the future.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 2, 2017

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Apr 2, 2017

Yes, don't do Cassandra: the only real use case for Cassandra is if you have 10+ nodes, and in that case it requires ops procedures which are far beyond what we should do.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 2, 2017

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Apr 2, 2017

@stevehouel, no Cassandra is not going to work with Rancher out of the box since it relies on a local volume to launch CQL scripts so you would need a special sidekick to do it. I had thought of doing it for k8s then I realised that it's not worth it.

I will do some testing next week and then merge your PR before wednesday when we will do a release.

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 2, 2017

@PierreBesson PierreBesson self-requested a review Apr 4, 2017

@PierreBesson

This comment has been minimized.

Copy link
Contributor

PierreBesson commented Apr 4, 2017

Nice work @stevehouel !

@PierreBesson PierreBesson merged commit 6eee289 into jhipster:master Apr 4, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 4, 2017

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 4, 2017

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Apr 4, 2017

@stevehouel you now need to add the documentation, next to https://jhipster.github.io/kubernetes/ - just copy/paste the file and do your documentation, don't worry too much about the menu, I'll do it.

Remember the Zen koan: If a tree falls in a forest and no one is around to hear it, does it make a sound?

@stevehouel

This comment has been minimized.

Copy link
Contributor Author

stevehouel commented Apr 4, 2017

@jdubois

This comment has been minimized.

Copy link
Member

jdubois commented Apr 4, 2017

OK then @stevehouel just do the PR on the website, I'll review & and merge :-)

@jdubois jdubois modified the milestone: 4.2.0 Apr 4, 2017

@stevehouel stevehouel deleted the stevehouel:5138 branch Apr 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment