Skip to content
This repository has been archived by the owner on Sep 19, 2018. It is now read-only.

1.7.6 from 1.3.0 #61

Open
wants to merge 16 commits into
base: 1.x
Choose a base branch
from
Open

1.7.6 from 1.3.0 #61

wants to merge 16 commits into from

Conversation

ebrigand
Copy link

  • plugin build process is coming from the tag elasticsearch-cloud-kubernetes-2.4.0_01 (to be able to use the Dockerfile and to have the last building process coming with the last maven pom file)
  • Dockefile install ES 1.7.6
  • Java class are coming from the tag elasticsearch-cloud-kubernetes-1.3.0

Copy link
Contributor

@jimmidyson jimmidyson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sending this in. Can you review the readme & make sure it is consistent with the 1.7 configuration?

LICENSE.txt Outdated
@@ -1,3 +1,18 @@
====
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not have changed.

NOTICE.txt Outdated
@@ -0,0 +1,16 @@
====
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file isn't actually required.

README.md Outdated
@@ -23,10 +28,11 @@ Here is a simple sample configuration:

```yaml
cloud:
k8s:
servicedns: ${SERVICE_DNS}
kubernetes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These options look like from later version? Can you update please to reflect the 1.3.x config?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes right

README.md Outdated
discovery:
type: io.fabric8.elasticsearch.discovery.k8s.K8sDiscoveryModule
type: kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Author

@ebrigand ebrigand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review done and i fixed some small glitch like I was using our docker id so i put back fabric8.
And I decided to change the Dockerfile to put a FROM using the official image instead of using a full copy past of their Dockerfile, same for the entry-point.sh we don't need it since it s the same as the official one

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants