Add complete k8s metadata through composable provider#27691
Add complete k8s metadata through composable provider#27691ChrsMark merged 17 commits intoelastic:masterfrom
Conversation
Signed-off-by: chrismark <chrismarkou92@gmail.com>
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
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>
|
/test |
|
Pinging @elastic/integrations (Team:Integrations) |
|
I'm opening this for an early review. I will need to add/tune tests as well as do extensive manual testing, however an early review would be more than welcome so as to spot any possible foundational issues early on. |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
|
Heads-up on this: Unit tests were tuned accordingly and I'm running several manual testing scenarios to verify nothing is broken. I'm planning to work on adding e2e tests right after this one is merged (issue: elastic/e2e-testing#1090) I have particularly tried to "port" the logic from the old autodiscovery feature to this one and try to cover cases like short-living jobs etc. Review parts:
Maybe in the same release we can tune the metadata part (in follow-ups) in order to be aligned with the outcomes of #13911 and more specifically #16483 and #14875 |
| ExcludeLabels []string `config:"exclude_labels"` | ||
|
|
||
| LabelsDedot bool `config:"labels.dedot"` | ||
| AnnotationsDedot bool `config:"annotations.dedot"` |
There was a problem hiding this comment.
Can this just be the default? Do we need this configurable now?
| AnnotationsDedot bool `config:"annotations.dedot"` | ||
|
|
||
| // Undocumented settings, to be deprecated in favor of `drop_fields` processor: | ||
| IncludeCreatorMetadata bool `config:"include_creator_metadata"` |
There was a problem hiding this comment.
If undocumented and deprecated, why add it? Being this is all new to Elastic Agent, it could be the time to remove it.
There was a problem hiding this comment.
You are right this one is not exposed so it should be removed.
There was a problem hiding this comment.
Actually this touches code that is used by beats too. I will remove it in follow up PR target 8.0
|
|
||
| AddResourceMetadata *metadata.AddResourceMetadataConfig `config:"add_resource_metadata"` | ||
| IncludeLabels []string `config:"include_labels"` | ||
| ExcludeLabels []string `config:"exclude_labels"` |
There was a problem hiding this comment.
Where is IncludeLabels and ExcludeLabels used? I was not able to find them in the diff.
There was a problem hiding this comment.
These are passed and used deeper in meta Generators, which is already existent codebase.
jsoriano
left a comment
There was a problem hiding this comment.
@jsoriano it would be super helpful if you could review the discovery lifecycle start/stop events , proper termination etc since you had worked on this recently :)
This part looks good (waiting for e2e tests for confirmation 🙂), added only some thoughts, nothing really blocking.
| k8s "k8s.io/client-go/kubernetes" | ||
|
|
||
| "github.com/elastic/beats/v7/libbeat/common" | ||
|
|
| "github.com/elastic/beats/v7/libbeat/common/kubernetes/metadata" | ||
| "github.com/elastic/beats/v7/libbeat/common/safemapstr" |
There was a problem hiding this comment.
Nit. Move beats imports to the last group of imports.
x-pack/elastic-agent/pkg/composable/providers/kubernetes/kubernetes.go
Outdated
Show resolved
Hide resolved
| for _, c := range pod.Spec.EphemeralContainers { | ||
| c := kubernetes.Container(c.EphemeralContainerCommon) | ||
| containers = append(containers, &containerInPod{spec: c}) | ||
| } |
There was a problem hiding this comment.
There are two places now where we collect all the containers in the pod and their statuses (the other here), this can be error-prone in future refactors. I wonder if this could be unified.
There was a problem hiding this comment.
Fair point. I will tune it! :)
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>
|
@jsoriano @blakerouse I tested this extensively (manually) and I think is good to go. Would you mind giving it another review? Next steps will be to add 2e2 tests (in this iteration/release) and verify that we are aligned with #13911 (cc @MichaelKatsoulis ). |
|
/test |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
blakerouse
left a comment
There was a problem hiding this comment.
Overall this looks good to me.
I would prefer to see the dedot feature just set to default and the options to turn if off removed. But I will leave that up to you to determine.
Signed-off-by: chrismark <chrismarkou92@gmail.com>
(cherry picked from commit 46d17b4)
* upstream/master: (658 commits) Add complete k8s metadata through composable provider (elastic#27691) Revert "Fix issue where --insecure didn't propogate to Fleet Server ES connection (elastic#27969)" (elastic#27997) Remove deprecated kafka fields (elastic#27938) [Filebeat] Add Base64 encoded HMAC & UUID template functions to httpjson input (elastic#27873) Improve httpjson template function join (elastic#27996) Remove kubernetes.container.image alias (elastic#27898) [Elastic Agent] Golden files for program tests (elastic#27862) [Elastic Agent] Disable modules.d in metricbeat (elastic#27860) libbeat/common/seccomp: provide default policy for linux arm64 (elastic#27955) Fix logger statement in aws-s3 input (elastic#27982) Fix wrong merge (elastic#27976) Fix issue where --insecure didn't propogate to Fleet Server ES connection (elastic#27969) Forward-port 7.14.2 changelog to master (elastic#27975) [Filebeat] Removing duplicate modules (aliases) Observability (elastic#27919) Fix path in vagrant windows script (elastic#27966) [Filebeat] Removing duplicate modules (aliases) and Cyberark (elastic#27915) No changelog for 8.0.0-alpha2 (elastic#27961) Add write access to 'url.value' from 'request.transforms'. (elastic#27937) Docker: remove deprecated fields (elastic#27933) Filebeat: Make all filesets disabled in default configuration (elastic#27762) ...
What does this PR do?
Add all k8s metadata via the provider.
Why is it important?
To support full metadata access like in Beats.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.TODOs
How to test this PR locally
./elastic-agent -c ./elastic-agent.yml inspect output -o defaultUsing annotations (Note: annotations are not exposed to meta fields by default but are exposed in variable resolution mechanism)
Specify a condition based on an annotation like
condition: ${kubernetes.annotations.level} == 'production' AND ${kubernetes.container.port_name} == 'web'and verify that input is created again. (note: tune Redis Pod manifest accordingly)Short-living pods
Define a log input with condition based on container's name:
Deploy elastic-agent and verify that it collects logs for the following short-living pod:
Short living Init containers are covered
Ensure logs are captured even if Pod is marked for deletion
Related issues
Notes for the reviewer
Most of the functionality is ported from
libbeat/autodiscover/providers/kubernetesandlibbeat/common/kubernetes/metadata.