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

Add filestream stream IDs to k8s manifests. #2788

Merged
merged 4 commits into from
Jun 13, 2023

Conversation

cmacknz
Copy link
Member

@cmacknz cmacknz commented Jun 6, 2023

Each filestream data stream also needs a unique ID, in addition to the input ID. The input ID is used by the agent itself, the stream level ID is what is used in the Filebeat registry.

Each filestream data stream also needs a unique ID, in addition to the
input ID. The input ID is used by the agent itself, the stream level ID
is what is used in the Filebeat registry.
@cmacknz cmacknz added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 6, 2023
@cmacknz cmacknz requested a review from a team as a code owner June 6, 2023 14:24
@cmacknz cmacknz self-assigned this Jun 6, 2023
@cmacknz cmacknz requested review from ChrsMark and constanca-m June 6, 2023 14:24
@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2023

This pull request does not have a backport label. Could you fix it @cmacknz? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip label Jun 6, 2023
@cmacknz cmacknz added skip-changelog backport-v8.8.0 Automated backport with mergify backport-v8.7.0 Automated backport with mergify labels Jun 6, 2023
@mergify mergify bot removed the backport-skip label Jun 6, 2023
@elasticmachine
Copy link
Contributor

elasticmachine commented Jun 6, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-06-07T18:08:10.968+0000

  • Duration: 24 min 21 sec

Test stats 🧪

Test Results
Failed 0
Passed 5983
Skipped 19
Total 6002

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented Jun 6, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.667% (74/75) 👍
Files 68.605% (177/258) 👍
Classes 67.423% (327/485) 👍
Methods 53.702% (1001/1864) 👍
Lines 39.597% (11462/28947) 👍 0.021
Conditionals 100.0% (0/0) 💚

@@ -340,7 +340,8 @@ data:
data_stream:
namespace: default
streams:
- data_stream:
- id: container-log-${kubernetes.pod.name}-${kubernetes.container.id}
Copy link
Member

Choose a reason for hiding this comment

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

So we need this ID in streams' level too? How is this different to the one that already exists at

- id: container-log-${kubernetes.pod.name}-${kubernetes.container.id}
?

cc: @gizas @MichaelKatsoulis

Copy link
Member Author

Choose a reason for hiding this comment

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

Explained here: #2573 (comment)

The input level ID is used by the agent to keep track of the input state. For filestream specifically, each data stream becomes an independent instance of the filestream input in Beats. We need the ID at the stream level to give each filestream input in Beats a unique ID.

There is a one to many mapping between a filestream input in the agent policy and the number of filestream inputs created in Filebeat.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although, for kubernetes use case it seems like an one to one mapping, as each agent policy filestream input can only contain one stream, as they both point to one pod-container pair.

Copy link
Member Author

Choose a reason for hiding this comment

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

The input itself uses variable substitution here, so the number of inputs would depend on the number of monitored containers wouldn't it?

Either way, the official examples should use a stream level ID as well so that someone modifying it will a correct example to start from.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cmacknz why we need both pod.name and container.id? Can we keep only container.id as it is unique afterall?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume because a pod can contain multiple containers, this is the same thing we use for the input level ID.

Copy link
Member

@ChrsMark ChrsMark Jun 7, 2023

Choose a reason for hiding this comment

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

So reading #2573 (comment) I understand that we need the id also on the streams level. Based on this the change looks good to me but could we add a reference to any docs or the used GH comment to avoid any confusion in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, we don't have a place in the official docs where we document how to configure each input type in the standalone agent. I just pasted a summary of how this works in the reference YAML and the k8s manifests. Hopefully it helps clarify things.

@cmacknz cmacknz requested a review from a team as a code owner June 7, 2023 18:07
@cmacknz cmacknz requested review from AndersonQ and blakerouse June 7, 2023 18:07
@cmacknz cmacknz force-pushed the add-filestream-stream-ids-k8s branch from f661a00 to 8381fa1 Compare June 7, 2023 18:08
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

LGTM

@cmacknz cmacknz requested a review from pchila June 8, 2023 17:16
@cmacknz cmacknz removed the backport-v8.7.0 Automated backport with mergify label Jun 13, 2023
@cmacknz cmacknz merged commit 6a6720e into elastic:main Jun 13, 2023
@cmacknz cmacknz deleted the add-filestream-stream-ids-k8s branch June 13, 2023 15:45
mergify bot pushed a commit that referenced this pull request Jun 13, 2023
* Add filestream stream IDs to k8s manifests.

Each filestream data stream also needs a unique ID, in addition to the
input ID. The input ID is used by the agent itself, the stream level ID
is what is used in the Filebeat registry.

* Add filestream docs to the reference configuration.

* Describe what IDs are used for in cfg files.

* Document IDs in the k8s configuration.

(cherry picked from commit 6a6720e)

# Conflicts:
#	_meta/config/common.p2.yml.tmpl
#	elastic-agent.yml
cmacknz added a commit that referenced this pull request Jun 13, 2023
)

* Add filestream stream IDs to k8s manifests. (#2788)

* Add filestream stream IDs to k8s manifests.

Each filestream data stream also needs a unique ID, in addition to the
input ID. The input ID is used by the agent itself, the stream level ID
is what is used in the Filebeat registry.

* Add filestream docs to the reference configuration.

* Describe what IDs are used for in cfg files.

* Document IDs in the k8s configuration.

(cherry picked from commit 6a6720e)

# Conflicts:
#	_meta/config/common.p2.yml.tmpl
#	elastic-agent.yml

* Resolve conflict.

---------

Co-authored-by: Craig MacKenzie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.8.0 Automated backport with mergify skip-changelog Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8.7.1: K8s Stand-alone Kubernetes Deployment - Filestream input with ID '' Already Exists
5 participants