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

ElasticSearch 5 support #1607

Merged
merged 3 commits into from
Jun 23, 2017
Merged

Conversation

AlmogBaku
Copy link
Contributor

@AlmogBaku AlmogBaku commented Apr 24, 2017

Solves #1410

This PR finally add the support for ElasticSearch 5 (with backward compatibility for ES2)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 24, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@AlmogBaku
Copy link
Contributor Author

/assign @AlmogBaku @andyxning @huangyuqi

@AlmogBaku AlmogBaku changed the title ElasticSearch 5 support ElasticSearch 5 support [WIP] Apr 24, 2017
@AlmogBaku
Copy link
Contributor Author

@rikatz can you help us out with reviewing this?

@rikatz
Copy link
Contributor

rikatz commented Apr 27, 2017

@AlmogBaku sure. Will review tomorrow :)

@AlmogBaku
Copy link
Contributor Author

I added a support for Ignest pipeline.

@rikatz
Copy link
Contributor

rikatz commented May 1, 2017

Sorry @AlmogBaku I've been pretty busy in work. Have seen that this worked now. Will try to test the code tomorrow and give you a feedback :)

@AlmogBaku
Copy link
Contributor Author

@rikatz any update?

@rikatz
Copy link
Contributor

rikatz commented May 10, 2017

@AlmogBaku sorry. I'll be testing this today and will return you in the end of this day!

@rikatz
Copy link
Contributor

rikatz commented May 10, 2017

@AlmogBaku while running heapster with elasticsearch 5, it gaves me the following error:

E0510 15:30:33.101740 12777 factory.go:84] Failed to create sink: Failed to create ElasticSearch client: Failed to an ElasticSearch Client: no Elasticsearch node available

I'm using the following directive: --sink="elasticsearch:?nodes=http://ipaddress:9200&index=testEvent&ver=5"

The Elasticsearch is available and answering:

curl http://ipaddress:9200
{
  "name" : "Pz3qaoM",
  "cluster_name" : "elasticsearch",
  "cluster_uuid" : "BUd5OBEGTpCIE6Xak5qglA",
  "version" : {
    "number" : "5.4.0",
    "build_hash" : "780f8c4",
    "build_date" : "2017-04-28T17:43:27.229Z",
    "build_snapshot" : false,
    "lucene_version" : "6.5.0"
  },
  "tagline" : "You Know, for Search"
}

Am I missing something?

@rikatz
Copy link
Contributor

rikatz commented May 11, 2017

@AlmogBaku some more info:

I'm running a standalone Elasticsearch, and started it now with 'sniff=false' (as in docs). Running with ver, disabling health check, all have the same result.

The following is a dump from communication between Heapster and Elastic:

HEAD / HTTP/1.1
Host: 10.X.X.X:9200
User-Agent: Go-http-client/1.1

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 327

GET /_nodes/http HTTP/1.1
Host: 10.X.X.X:9200
User-Agent: elastic/5.0.36 (linux-amd64)
Accept: application/json
Content-Type: application/json
Accept-Encoding: gzip

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-encoding: gzip
transfer-encoding: chunked

@AlmogBaku
Copy link
Contributor Author

AlmogBaku commented May 14, 2017

@rikatz

  1. please double check your config
  2. please send me logs with verbose enabled (as described here https://github.com/kubernetes/heapster/blob/master/docs/debugging.md )
  3. do you use usr/pwd?

@AlmogBaku
Copy link
Contributor Author

@rikatz ?

@chancez
Copy link
Contributor

chancez commented May 21, 2017

@AlmogBaku I for what it's worth, I got this working just fine. I had to make a small addition to the PR to add an option to disable SSL verification on the http.Client since I'm using a self signed TLS cert for ES (using the elasticsearch-operator) and didn't bother with getting the certs correctly mounted in the eventer container, but otherwise it works great.

Here's the manifest I'm using: https://gist.github.com/e725dea50300bafc781382546bfe6f2b

The additional code is just this:

httpClient := &http.Client{}
if len(opts["verifySSL"]) > 0 {
	verifySSL, err := strconv.ParseBool(opts["verifySSL"][0])
	if err != nil {
		return nil, errors.New("Failed to parse URL's verifySSL value into a bool")
	}
	httpClient.Transport = &http.Transport{
		TLSClientConfig: &tls.Config{
			InsecureSkipVerify: !verifySSL,
		},
	}
}
startupFnsV2 = append(startupFnsV2, elastic2.SetHttpClient(httpClient))
startupFnsV5 = append(startupFnsV5, elastic5.SetHttpClient(httpClient))

@AlmogBaku
Copy link
Contributor Author

@chancez I guess that's belong to a different PR..

@chancez
Copy link
Contributor

chancez commented May 21, 2017

@AlmogBaku Yeah, I'm waiting for this one to get merged and I'll open a PR for it. Mostly wanted to say this is working great perfectly for me.

@rikatz
Copy link
Contributor

rikatz commented May 21, 2017 via email

@rikatz
Copy link
Contributor

rikatz commented May 22, 2017

@chancez Can you please quickly write how you started ES5 and Heapster?

Thanks!

@chancez
Copy link
Contributor

chancez commented May 22, 2017

@rikatz I'm using https://github.com/upmc-enterprises/elasticsearch-operator and the manifest I'm using is in the gist above. Just know I've made a minor modification to this code to work with my elasticsearch, as mentioned, so I can disable SSL verification.

@rikatz
Copy link
Contributor

rikatz commented May 25, 2017

OK. I'm testing this again now and will let you know

@rikatz
Copy link
Contributor

rikatz commented May 26, 2017

@AlmogBaku tested here in ES5 and it worked really fine :)

LGTM!

Thanks and sorry about delaying this so much :(

@rikatz
Copy link
Contributor

rikatz commented May 26, 2017

I was just wondering here if it wouldn't be better to have a 'Generic Timestamp' (like a @timestamp) for each metric, as this breaks each Timestamp into CpuMetricsTimestamp, MemoryMetricsTimestamp and this, in kibana makes a lot difficult to create dashboards (as each metric have it's own timestamp and not a generic one).

Also, after pushing some metrics I've faced some 'Failed to push data to sink: ElasticSearch Sink' but don't know why. Heapster is pushing metrics anyway, so probably is something specific to some object inside my cluster.

@rikatz
Copy link
Contributor

rikatz commented May 26, 2017

@AlmogBaku other 'review'. While creating a dashboard in Grafana, I was trying to aggregate metrics per namespace and per pod-name.

When using MetricsTags.pod_id it worked fine, but when using MetricsTags.pod_name I've faced the error related to fielddata

Don't know if this is fine for now, but probably we need a better field mapping in this Sink, as those searches/aggregations could be used :)

Anything I can help with?

@SupreethKadalur
Copy link

Hello @AlmogBaku , @rikatz - Please let us know if ES5 PR is merged and ready for use. I have the same problem and unable to send data from heapster v 1.3.0 to ES5

@rikatz - I totally agree that we should have @timestamp for each metrics :-) this will solve many problems I have while creating dashboards in kibana.

@DirectXMan12
Copy link
Contributor

Alright, squashed into three commits (Godeps, ES5, ingest). Should be good to merge once it passes the tests.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

This commit adds in support for ES5 to the ElasticSearch sink.
The ES5 support lands side-by-side with the ES2 support, since
the APIs are slightly different.
This commit updates the Godeps for ES5 support in the ElasticSearch
sink.
This commit adds support for ElasticSearch ingest pipelines.  This
only works on ES5+.
@DirectXMan12
Copy link
Contributor

/retest

@DirectXMan12
Copy link
Contributor

looks like a flake unrelated to this PR

/retest

@AlmogBaku
Copy link
Contributor Author

/retest

@DirectXMan12
Copy link
Contributor

looks good, merging

@DirectXMan12 DirectXMan12 merged commit b7884da into kubernetes-retired:master Jun 23, 2017
@rikatz
Copy link
Contributor

rikatz commented Jun 23, 2017

@AlmogBaku thanks!! :)

@DirectXMan12
Copy link
Contributor

Thanks everyone for all their hard work on this :-)

@AlmogBaku
Copy link
Contributor Author

AlmogBaku commented Jun 24, 2017 via email

@SupreethKadalur
Copy link

@DirectXMan12 , @AlmogBaku - I see that ES5 fix is merged and Heapster v1.4.0-beta.0 is created. I'm now using docker image "gcr.io/google_containers/heapster-amd64:v1.4.0-beta.0" but still see the same problem I had with heapster-amd64:v1.3.0 image. Am I missing something here? Please suggest. Thanks

@chancez
Copy link
Contributor

chancez commented Jun 27, 2017

@SupreethKadalur This got merged after that pre-relase, you'll have to wait for another pre-release or build it yourself.

@piosz piosz changed the title ElasticSearch 5 support [WIP] ElasticSearch 5 support Jun 28, 2017
@SupreethKadalur
Copy link

Thank you all for the support and fix. Now ES5 issue is fixed with heapster v1.4.0

@piosz
Copy link
Contributor

piosz commented Jun 30, 2017

Yes, this is included in 1.4.0 as described in https://github.com/kubernetes/heapster/releases/tag/v1.4.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sink/elasticsearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.