You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by andreaturli <gi...@git.apache.org> on 2017/09/19 10:21:10 UTC
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
GitHub user andreaturli opened a pull request:
https://github.com/apache/brooklyn-server/pull/830
Add support for kubernetes helm
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andreaturli/brooklyn-server feature/container-k8s-helm
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/830.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #830
----
commit 209be648dc5660019f61596129bb59e003a90e54
Author: Andrea Turli <an...@gmail.com>
Date: 2017-08-21T13:21:52Z
Add support for kubernetes helm
----
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
would be great to have this merged! nearly there IMO.
re clocker, deploy _to_ i think of as Brooklyn, and this is squarely in that camp ... deploy _of_ {k8s,docker,etc} is more clocker.
would be nice to have a yaml example -- ideally a docs addition that references an example tested here (is there a brooklyn-docs PR ?). also nice to have another pair of eyes on this but given the time and value, and i've looked at the code and am happy with it, i'm happy for this to be merged.
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
@tbouron as per our review, I've add the creation of some `.helm` dirs required to run on a fresh installation: thanks for spotting the issue!
it basically mimic the same approach taken by the `microbeam-helm` project - https://github.com/microbean/microbean-helm/blob/ae51ed6571af204df9b764f0b2b59f2f8e2c2aa4/.travis.yml#L6
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
@andreaturli Might be completely off-base here but wouldn't it make more sense for this to be in the clocker project
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
@tbouron I remember I used minikube to test it. Any k8s cluster should work though
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158293470
--- Diff: locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java ---
@@ -347,8 +386,8 @@ public Boolean call() {
if (result.isEmpty()) {
return false;
}
- List<HasMetadata> check = client.resource(result.get(0)).inNamespace(result.get(0).getMetadata().getNamespace()).get();
- if (result.size() > 1 || check.size() != 1 || check.get(0).getMetadata() == null) {
+ HasMetadata check = client.resource(result.get(0)).inNamespace(result.get(0).getMetadata().getNamespace()).get();
--- End diff --
think so, but need to double-check as I don't recall the details
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
retest this please
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
retest this please
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158289882
--- Diff: locations/container/src/main/java/org/apache/brooklyn/container/location/kubernetes/KubernetesLocation.java ---
@@ -347,8 +386,8 @@ public Boolean call() {
if (result.isEmpty()) {
return false;
}
- List<HasMetadata> check = client.resource(result.get(0)).inNamespace(result.get(0).getMetadata().getNamespace()).get();
- if (result.size() > 1 || check.size() != 1 || check.get(0).getMetadata() == null) {
+ HasMetadata check = client.resource(result.get(0)).inNamespace(result.get(0).getMetadata().getNamespace()).get();
--- End diff --
Is that normal the signature changed from `List<HasMetadata>` to `HasMetadata` ?
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158288981
--- Diff: locations/container/pom.xml ---
@@ -32,10 +32,50 @@
</parent>
<properties>
- <kubernetes-client.version>1.4.27</kubernetes-client.version>
+ <kubernetes-client.version>2.5.6</kubernetes-client.version>
</properties>
<dependencies>
+ <dependency>
+ <groupId>org.microbean</groupId>
+ <artifactId>microbean-helm</artifactId>
+ <version>2.5.0.1-SNAPSHOT</version>
+ <exclusions>
+ <!--<exclusion>-->
+ <!--<groupId>io.grpc</groupId>-->
+ <!--<artifactId>grpc-netty</artifactId>-->
+ <!--</exclusion>-->
--- End diff --
Can be removed
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by drigodwin <gi...@git.apache.org>.
Github user drigodwin commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
retest this please
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158289019
--- Diff: locations/container/pom.xml ---
@@ -32,10 +32,50 @@
</parent>
<properties>
- <kubernetes-client.version>1.4.27</kubernetes-client.version>
+ <kubernetes-client.version>2.5.6</kubernetes-client.version>
</properties>
<dependencies>
+ <dependency>
+ <groupId>org.microbean</groupId>
+ <artifactId>microbean-helm</artifactId>
+ <version>2.5.0.1-SNAPSHOT</version>
+ <exclusions>
+ <!--<exclusion>-->
+ <!--<groupId>io.grpc</groupId>-->
+ <!--<artifactId>grpc-netty</artifactId>-->
+ <!--</exclusion>-->
+ <exclusion>
+ <groupId>com.google.api.grpc</groupId>
+ <artifactId>proto-google-common-protos</artifactId>
+ </exclusion>
+ <!--<exclusion>-->
+ <!--<groupId>com.google.instrumentation</groupId>-->
+ <!--<artifactId>instrumentation-api</artifactId>-->
+ <!--</exclusion>-->
--- End diff --
Same as above
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158293223
--- Diff: locations/container/pom.xml ---
@@ -32,10 +32,50 @@
</parent>
<properties>
- <kubernetes-client.version>1.4.27</kubernetes-client.version>
+ <kubernetes-client.version>2.5.6</kubernetes-client.version>
</properties>
<dependencies>
+ <dependency>
+ <groupId>org.microbean</groupId>
+ <artifactId>microbean-helm</artifactId>
+ <version>2.5.0.1-SNAPSHOT</version>
+ <exclusions>
+ <!--<exclusion>-->
+ <!--<groupId>io.grpc</groupId>-->
+ <!--<artifactId>grpc-netty</artifactId>-->
+ <!--</exclusion>-->
--- End diff --
thanks @tbouron
also now there is an official release http://search.maven.org/#search%7Cga%7C1%7Ca%3Amicrobean-helm which may be better and needs testing
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/830
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
Just tested, works like a charm, thanks @andreaturli !
Just need to make Jenkins happy again and I'll merge it 👍
---
[GitHub] brooklyn-server pull request #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/830#discussion_r158294318
--- Diff: locations/container/pom.xml ---
@@ -32,10 +32,50 @@
</parent>
<properties>
- <kubernetes-client.version>1.4.27</kubernetes-client.version>
+ <kubernetes-client.version>2.5.6</kubernetes-client.version>
</properties>
<dependencies>
+ <dependency>
+ <groupId>org.microbean</groupId>
+ <artifactId>microbean-helm</artifactId>
+ <version>2.5.0.1-SNAPSHOT</version>
+ <exclusions>
+ <!--<exclusion>-->
+ <!--<groupId>io.grpc</groupId>-->
+ <!--<artifactId>grpc-netty</artifactId>-->
+ <!--</exclusion>-->
--- End diff --
Ah ok, I'll let you decide which one is best then ;)
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
good question @tbouron -- I think Helm is a package manager for Kubernets and helps you deploy application to kubernetes. Seemed to me the `location/container` is a good fit, as it is where brooklyn implements the support to deployTo kubernetes. No strong feelings though
---
[GitHub] brooklyn-server issue #830: Add support for kubernetes helm
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/830
retest this please
---