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

fix system test for kubernetes integration for k8s v1.27.0 #6310

Merged
merged 4 commits into from
May 26, 2023

Conversation

gsantoro
Copy link
Contributor

What does this PR do?

Fix failing system tests for the integration kubernetes when testing with kubernetes > v1.27.0.

It fails with the following error

[error] failed to initialize Lua VM in /etc/nginx/nginx.conf:47

I tried to just upgrade the version of container image k8s.gcr.io/nginx-slim:0.8 to a newer tag but since I couldn't easily list the tags available (like I would do with docker hub) I couldn't really figure out the Dockerfile related to that image or what was wrong with that image. Questions on stackoverflow about this error seems to suggest that the official image of nginx doesn't come with Lua at all.

Also because of announcement I tried to replace the container registry from k8s.gcr.io to registry.k8s.io but I had a similar problem trying to figure out what was the latest version of nginx image at that container registry.

So the solution was to use an official nginx image from dockerhub. That was enough to fix the issue and make all system tests pass.

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

  • all tests pass on Jenkins

How to test this PR locally

  • replicate the Jenkins pipeline locally

Related issues

Screenshots

@gsantoro gsantoro added the bug Something isn't working, use only for issues label May 24, 2023
@gsantoro gsantoro requested a review from a team as a code owner May 24, 2023 15:23
@gsantoro gsantoro self-assigned this May 24, 2023
@gsantoro gsantoro requested a review from a team as a code owner May 24, 2023 15:23
@gsantoro gsantoro requested review from ChrsMark and constanca-m May 24, 2023 15:23
@constanca-m
Copy link
Contributor

Hey @jsoriano, does merging this commit overwrite the beta versions in Kubernetes package? Beta versions are in 1.40.0 versions as well.

@elasticmachine
Copy link

elasticmachine commented May 24, 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-05-26T08:42:47.614+0000

  • Duration: 30 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 92
Skipped 0
Total 92

🤖 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

elasticmachine commented May 24, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (0/0) 💚
Files 100.0% (0/0) 💚
Classes 100.0% (0/0) 💚
Methods 96.154% (75/78)
Lines 100.0% (0/0) 💚
Conditionals 100.0% (0/0) 💚

@jsoriano
Copy link
Member

jsoriano commented May 26, 2023

Hey @jsoriano, does merging this commit overwrite the beta versions in Kubernetes package? Beta versions are in 1.40.0 versions as well.

The 1.40.0 will be published and beta versions will remain published too. Search requests will start including 1.40.0.

@@ -25,6 +25,7 @@ streams:
title: Dataset name
description: >
Set the name for your dataset. Changing the dataset will send the data to a different index. For more info look at [data_stream field](https://www.elastic.co/guide/en/ecs/master/ecs-data_stream.html).

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this line added? Same for the next file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's the output of elastic-package format or elastic-package lint

Copy link
Contributor

Choose a reason for hiding this comment

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

strange. Because there is not empty line under all fields. Personally I wouldn't leave this line but it's your choice.

Copy link
Contributor

@constanca-m constanca-m left a comment

Choose a reason for hiding this comment

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

You will have to update the K8s package version again since 1.41 exists now.

@gsantoro
Copy link
Contributor Author

You will have to update the K8s package version again since 1.41 exists now.

Do you mean this PR. It hasn't been merged yet and I haven't got a feedback yet on what's the right naming scheme for beta versions and what comes next. I left a comment at here.

@constanca-m
Copy link
Contributor

You will have to update the K8s package version again since 1.41 exists now.

Do you mean this PR. It hasn't been merged yet and I haven't got a feedback yet on what's the right naming scheme for beta versions and what comes next. I left a comment at here.

You're right, when I saw it was pushed I though it had been committed as well. Sorry, my mistake :)

@gsantoro gsantoro merged commit 14c5d1d into elastic:main May 26, 2023
@gsantoro gsantoro deleted the feature/k8s_1.27.0 branch May 26, 2023 12:11
@elasticmachine
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working, use only for issues Integration:kubernetes Kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants