Skip to content
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

[6.8] Check license expiry date zero value (#14591) #14961

Merged
merged 7 commits into from
Dec 23, 2019

Conversation

ycombinator
Copy link
Contributor

Backports the following commits to 6.8:

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

module/elasticsearch/ccr/ccr.go:93:5: m.Logger undefined (type *MetricSet has no field or method Logger) CI is not happy here?

@ycombinator
Copy link
Contributor Author

@chrisronline was testing this PR and pinged me offline. He's getting the following error when he runs this PR against an ES 6.8.0 node:

panic: value method time.Time.MarshalText called using nil *Time pointer

goroutine 58 [running]:
time.(*Time).MarshalText(0x0, 0x57274e0, 0x0, 0x81a5990, 0x0, 0x1)
        <autogenerated>:1 +0x93
github.com/elastic/beats/libbeat/common.normalizeValue(0x57274e0, 0x0, 0xc420fbbc10, 0x2, 0xa, 0x56762a0, 0xc42104ade0, 0x0, 0x0, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/common/event.go:161 +0x180c
github.com/elastic/beats/libbeat/common.normalizeMap(0xc4202af7a0, 0xc420fbbc10, 0x1, 0xa, 0x0, 0x0, 0x0, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/common/event.go:64 +0x1a7
github.com/elastic/beats/libbeat/common.normalizeValue(0x56762a0, 0xc4202af7a0, 0xc420fbbc10, 0x1, 0xa, 0x56762a0, 0xc42104a5d0, 0x0, 0x0, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/common/event.go:181 +0x1354
github.com/elastic/beats/libbeat/common.normalizeMap(0xc4202af980, 0xc420fbbc10, 0x0, 0xa, 0xbf7680c721b40ae7, 0x3ae1a82, 0x664ef60, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/common/event.go:64 +0x1a7
github.com/elastic/beats/libbeat/common.ConvertToGenericEvent(0xc4202af980, 0xc4204a2420)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/common/event.go:48 +0x84
github.com/elastic/beats/libbeat/publisher/pipeline.glob..func2(0xc4204d06c0, 0x38, 0x55d0220, 0xbf7680c721b40ae7)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/processor.go:207 +0x51
github.com/elastic/beats/libbeat/publisher/pipeline.(*processorFn).Run(0xc420176e80, 0xc4204d06c0, 0x0, 0xc4204b4c00, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/processor.go:198 +0x34
github.com/elastic/beats/libbeat/publisher/pipeline.(*program).Run(0xc4202a5dd0, 0xc4204d06c0, 0xc420fbbde0, 0x4058520, 0xc4204f9680)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/processor.go:168 +0xad
github.com/elastic/beats/libbeat/publisher/pipeline.(*client).publish(0xc42029eea0, 0xbf7680c721b41804, 0x3ae279f, 0x664ef60, 0xc4202cee10, 0xc4202af980, 0x0, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/client.go:84 +0x474
github.com/elastic/beats/libbeat/publisher/pipeline.(*client).Publish(0xc42029eea0, 0xbf7680c721b41804, 0x3ae279f, 0x664ef60, 0xc4202cee10, 0xc4202af980, 0x0, 0x0)
        /Users/shaunak/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/client.go:63 +0x95
github.com/elastic/beats/metricbeat/mb/module.PublishChannels.func1(0xc42029ef60)
        /Users/shaunak/go/src/github.com/elastic/beats/metricbeat/mb/module/publish.go:41 +0x125
created by github.com/elastic/beats/metricbeat/mb/module.PublishChannels
        /Users/shaunak/go/src/github.com/elastic/beats/metricbeat/mb/module/publish.go:48 +0xda

I am able to reproduce this error locally. Investigating.

@ycombinator
Copy link
Contributor Author

I did some more testing. This appears to be an issue with Metricbeat 6.8.x. That is, even if I build Metricbeat from the 6.8 branch (without this PR's changes in the mix), I am able to get the same error as above. I tested with different versions of ES and that didn't seem to matter. So it's something in the Metricbeat/libbbeat 6.8.x code. Will investigate.

@ycombinator
Copy link
Contributor Author

I traced it down to a change introduced in #7999, which exists in master and 7.x branches, but not in the 6.8 branch.

So I've setup the backport PR (targeting 6.8) for this change: #15175.

Indeed, if I check out the backport PR and build Metricbeat from it, the above error goes away. So we need #15175 to be merged before this PR here can be re-tested and merged.

Nice catch, @chrisronline!

* Check license expiry date zero value

* Adding check to system test

* Refactoring: moving fix to better location in code

* Adding CHANGELOG entry

* Fixing typo

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Change CCR log message to debug

* Start basic license
@ycombinator
Copy link
Contributor Author

ycombinator commented Dec 18, 2019

@chrisronline Thanks, again, for finding that issue. Turned out it was happening on 6.8 branch (i.e. without the changes in this PR) because of a missed backport. I've merged that backport now and rebased this PR on 6.8. Would you mind taking it for a spin again?

@ycombinator
Copy link
Contributor Author

CI test failure is related to this PR. Investigating.

@chrisronline
Copy link
Contributor

@ycombinator Should I wait to re-review until after the CI issues are sorted?

@ycombinator
Copy link
Contributor Author

@chrisronline Yeah, better to wait. I'll re-ping you on the PR when it's ready for your 👀 again.

@ycombinator
Copy link
Contributor Author

@chrisronline I updated the failing test code so it's now passing. Can you re-review this now please?

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM!

@ycombinator
Copy link
Contributor Author

Travis CI is green. Jenkins CI failure is unrelated to this PR. Merging.

@ycombinator ycombinator merged commit fa94b18 into elastic:6.8 Dec 23, 2019
@ycombinator ycombinator deleted the backport/6.8/pr-14591 branch December 23, 2019 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants