Upgrade k8s.io/client-go and k8s keystore tests #18817
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: chrismark <chrismarkou92@gmail.com>
ChrsMark
added
in progress
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
botelastic
bot
added
needs_team
❕ Build Aborted
Expand to view the summary
Build stats
Log outputExpand to view the last 100 lines of log output
|
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
exekias
suggested changes
May 29, 2020
dev-tools/generate_notice.py
Outdated
| @@ -434,6 +434,7 @@ def detect_license_summary(content): | |||
| "MPL-2.0", | |||
| "UPL-1.0", | |||
| "ISC", | |||
| "UNKNOWN", | |||
There was a problem hiding this comment.
I don't think we can accept these, did you dig in what license is this project using?
There was a problem hiding this comment.
I did check and json-patch has a BSD3 license (which is accepted), but detection is failing, we need to adjust the script to detect it
There was a problem hiding this comment.
@exekias I'm afraid that their licence is not correct:
In https://github.com/evanphx/json-patch/blob/17efcbe3533fdf65efac5054bddbee809b4b4848/LICENSE#L9 there is a comma missing after notice.
Cross checked with https://opensource.org/licenses/BSD-3-Clause.
Tried with a sample repo of mine and Github seems to give a proper template for this (with the comma included) hence they might missed it somehow. Shall we mention it to them and/or a PR to fix it?
There was a problem hiding this comment.
Why not, this looks like a minor typo, a PR sounds like a good idea.
In order to avoid blocking this one I wonder if we should update our script to accept that or wait a little bit to see it fixed
There was a problem hiding this comment.
PR opened. I prefer to have it fixed there properly instead of changing our script :)
jsoriano
reviewed
May 29, 2020
There was a problem hiding this comment.
This is looking good!
I have added some suggestions about the use of contexts in this PR, in general:
context.Background()should be used, quoting docs: "by the main function, initialization, and tests, and as the top-level Context for incoming requests."context.TODO()should be used "when it's unclear which Context to use or it is not yet available (because the surrounding function has not yet been extended to accept a Context parameter)."- Functions having multiple calls that require a context should use the same context (or contexts based on the same one), so it is easier to refactor in the future when a context is provided.
libbeat/common/kubernetes/k8skeystore/kubernetes_keystore_test.go
Outdated
Show resolved
Hide resolved
Signed-off-by: chrismark <chrismarkou92@gmail.com>
|
Thanks for the suggestions @jsoriano I was thoughtful about this context thing too and you shed light :). Regarding #18817 (comment), do you see a way of leveraging this in the scope of this PR? Also tested in general(metricsets, autodiscover, metadata) k8s functionality manually and didn't notice anything suspicious 🤞 . |
|
Thanks for addressing all the comments!
No, better to do this in future PRs. Do you plan to wait for evanphx/json-patch#105 before merging this? I think it would be better to add another pattern to the script so this license is identified. |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
👍
Well that's true, I hadn't predicted all this versioning overhead 😁 . Pushed a temp workaround so as to cover this case in our extractor script :). |
ChrsMark
added
needs_backport
jsoriano
approved these changes
May 29, 2020
ChrsMark
added a commit
to ChrsMark/beats
that referenced
this pull request
Jun 1, 2020
(cherry picked from commit 5bfcc9c)
ChrsMark
removed
the
needs_backport
ChrsMark
added a commit
that referenced
this pull request
Jun 1, 2020
v1v
added a commit
to v1v/beats
that referenced
this pull request
Jun 2, 2020
…-stage-level * upstream/master: (30 commits) Add a GRPC listener service for Agent (elastic#18827) Disable host.* fields by default for iptables module (elastic#18756) [WIP] Clarify capabilities of the Filebeat auditd module (elastic#17068) fix: rename file and remove extra separator (elastic#18881) ci: enable JJBB (elastic#18812) Disable host.* fields by default for Checkpoint module (elastic#18754) Disable host.* fields by default for Cisco module (elastic#18753) Update latest.yml testing env to 7.7.0 (elastic#18535) Upgrade k8s.io/client-go and k8s keystore tests (elastic#18817) Add missing Jenkins stages for Auditbeat (elastic#18835) [Elastic Log Driver] Create a config shim between libbeat and the user (elastic#18605) Use indexers and matchers in config when defaults are enabled (elastic#18818) Fix panic on `metricbeat test modules` (elastic#18797) [CI] Fix permissions in MacOSX agents (elastic#18847) [Ingest Manager] When not port are specified and the https is used fallback to 443 (elastic#18844) [Ingest Manager] Fix install service script for windows (elastic#18814) [Metricbeat] Fix getting compute instance metadata with partial zone/region config (elastic#18757) Improve error messages in s3 input (elastic#18824) Add memory metrics into compute googlecloud (elastic#18802) include bucket name when logging error (elastic#18679) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR is a follow up for #18096.
Aims to add tests for k8s keystore internal functionality using a fake client for retrieving k8s secrets. An upgrade of
k8s.io/client-gois needed in order to make use of thefakeclient (https://github.com/kubernetes/client-go/releases/tag/v0.18.3, https://pkg.go.dev/k8s.io/client-go@v0.18.3/kubernetes?tab=doc).Upgrade procedure:
Changes:
k8s.io/client-gokube-state-metricsin order to avoid flakinessWhy is it important?
In order to cover k8s keystore
Retrievefunctionality.