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

[NGINX] After 8.6.0, kubernetes pod labels used for condition matching are not dedoted #5099

Conversation

MichaelKatsoulis
Copy link
Contributor

What does this PR do?

After 8.6.0, kubernetes pod labels used for condition matching are not dedoted.
PR that introduced this is elastic/elastic-agent#1398

So the nginx_ingress_controller integration needs to be updated with the correct defaults for conditions used to match the ingress-nginx pod.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

  1. Set up an elastic-stack of 8.6.0 version
  2. Set up kubernetes cluster and ingress-nginx following https://kind.sigs.k8s.io/docs/user/ingress/
  3. Install 1.7.0 versions of nginx_ingress_controller package
  4. Run agent with the correct policy
  5. Check logs being collected

@MichaelKatsoulis MichaelKatsoulis requested a review from a team as a code owner January 25, 2023 09:52
@elasticmachine
Copy link

elasticmachine commented Jan 25, 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-01-25T10:02:31.793+0000

  • Duration: 13 min 30 sec

Test stats 🧪

Test Results
Failed 0
Passed 28
Skipped 0
Total 28

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (2/2) 💚
Classes 100.0% (2/2) 💚
Methods 92.0% (23/25) 👎 -3.868
Lines 94.428% (322/341) 👎 -1.339
Conditionals 100.0% (0/0) 💚

@andrewkroh andrewkroh changed the title After 8.6.0, kubernetes pod labels used for condition matching are not dedoted [NGINX] After 8.6.0, kubernetes pod labels used for condition matching are not dedoted Jan 26, 2023
@MichaelKatsoulis MichaelKatsoulis added Integration:nginx_ingress_controller Nginx Ingress Controller Logs and removed Integration:nginx Nginx labels Jan 26, 2023
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.

Change looks good to me. Do you also plan to enhance the provider's docs regarding the dedoting concept?

@MichaelKatsoulis
Copy link
Contributor Author

Change looks good to me. Do you also plan to enhance the provider's docs regarding the dedoting concept?

Yes until now there is no documentation pointing that for autodiscover templates , dedoting was needed for labels.
This information is blur and needs to be clear.

@MichaelKatsoulis MichaelKatsoulis merged commit 5ae892f into elastic:main Jan 26, 2023
@elasticmachine
Copy link

Package nginx_ingress_controller - 1.7.0 containing this change is available at https://epr.elastic.co/search?package=nginx_ingress_controller

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:nginx_ingress_controller Nginx Ingress Controller Logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants