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 cronjob metadata by default #30637

Merged
merged 19 commits into from Mar 10, 2022
Merged

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Mar 2, 2022

What does this PR do?

  1. Makes metadata from cronjob objects available by default
  2. Adds cronjob.name for Pods handled by a Job but are actually created because of a CronJob: https://github.com/elastic/beats/pull/30637/files#diff-e127eb943d1a3edae0a9f12cc7c3849874a256ae557242bf1473a1a28654fe64R101

Why is it important?

In order to make metadata addition for cronjob objects by default.

Related issues

cc: @MichaelKatsoulis

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark added backport-v8.1.0 Automated backport with mergify Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team labels Mar 2, 2022
@ChrsMark ChrsMark self-assigned this Mar 2, 2022
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Mar 2, 2022
@botelastic
Copy link

botelastic bot commented Mar 2, 2022

This pull request doesn't have a Team:<team> label.

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 2, 2022

💚 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](https://ci-stats.elastic.co/app/apm/services/beats-ci/transactions/view?rangeFrom=2022-03-09T20:06:58.934Z&rangeTo=2022-03-09T20:26:58.934Z&transactionName=BUILD Beats/beats/PR-{number}&transactionType=job&latencyAggregationType=avg&traceId=84498d316bc7fafbd42bc2369465ce4c&transactionId=cc5e4aa2841fc490)

Expand to view the summary

Build stats

  • Start Time: 2022-03-09T20:16:58.934+0000

  • Duration: 121 min 12 sec

Test stats 🧪

Test Results
Failed 0
Passed 42437
Skipped 3708
Total 46145

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

for versions of k8s >= v1.21.
This metricset adds metadata by default only for versions of k8s >= v1.21.
For older versions the APIs are not compatible and one need to configure the
metricset with `add_metadata: false` and remove the proper `apiGroup` in the `ClusterRole`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should be a bit more specific here of what they need to remove. Why not add an example with the specific part of the cluster role they need to remove?

Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Copy link
Contributor

@MichaelKatsoulis MichaelKatsoulis left a comment

Choose a reason for hiding this comment

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

LGTM

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>
@ChrsMark ChrsMark added backport-v8.2.0 Automated backport with mergify and removed backport-v8.1.0 Automated backport with mergify labels Mar 4, 2022
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 4, 2022

Trying to isolate the CI failures at #30685. Something is really weird here.

@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 8, 2022

Waiting for #30732 to see if an upgrade fixes things.

@ChrsMark ChrsMark requested a review from a team as a code owner March 8, 2022 11:35
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 8, 2022

Something is odd with the way we test on the CI as described at #30733. For now we can disable metadata addition for state_cronjob tests. I will file a separate issue for this so as to keep it tracked. For reference, the metricset is indirectly tested about this functionality under integrations repo so we can claim that it is safe to skip metadata here for now.

@MichaelKatsoulis let me know what you think about this.

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@MichaelKatsoulis
Copy link
Contributor

@MichaelKatsoulis let me know what you think about this.

Although it is a dirty workaround, it does not seem to be another way.

So +1

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>
@ChrsMark ChrsMark requested a review from cmacknz March 9, 2022 19:03
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.2.0 Automated backport with mergify Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit how metadata are added for jobs/cronjobs
4 participants