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 k8s overview dashboard #2876

Merged
merged 5 commits into from
Mar 30, 2022
Merged

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Mar 23, 2022

What does this PR do?

Fixes k8s dashboard as part of #2159. Porting PR for elastic/beats#30913 (+elastic/beats#30986):

Screenshot 2022-03-23 at 10 24 20 AM
Screenshot 2022-03-23 at 10 24 02 AM
Screenshot 2022-03-23 at 10 23 48 AM

Closes #2906 too.

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team [elastic/obs-cloudnative-monitoring] label Mar 23, 2022
@ChrsMark ChrsMark requested a review from a team as a code owner March 23, 2022 10:31
@ChrsMark ChrsMark self-assigned this Mar 23, 2022
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@elasticmachine
Copy link

elasticmachine commented Mar 23, 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

Expand to view the summary

Build stats

  • Start Time: 2022-03-30T10:10:09.988+0000

  • Duration: 29 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 116
Skipped 0
Total 116

🤖 GitHub comments

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

  • /test : Re-trigger the build.

@ChrsMark
Copy link
Member Author

@mtojek
Copy link
Contributor

mtojek commented Mar 23, 2022

Did you check Kibana version in the dashboard/visualizations JSON files?

EDIT:

I suspect that you used/dumped using newer Kibana and now testing with 7.16.0.

@ChrsMark
Copy link
Member Author

Did you check Kibana version in the dashboard/visualizations JSON files?

EDIT:

I suspect that you used/dumped using newer Kibana and now testing with 7.16.0.

Yes this is intentional. The dashboards I build are leveraging 8.2 Kibana's features. Is this supported in testing or I have to wait?

@mtojek
Copy link
Contributor

mtojek commented Mar 23, 2022

Yes this is intentional. The dashboards I build are leveraging 8.2 Kibana's features. Is this supported in testing or I have to wait?

I guess that it means that the package won't be supported by older Kibanas and you need to update the Kibana version constraint. Dashboards are part of the package.

@ChrsMark
Copy link
Member Author

Isn't https://github.com/elastic/integrations/pull/2876/files#diff-d20d8086b58468701d8a944fc3ead31ddcc098054affdcc335b69464b4c16c54R13 enough? Or I should completely remove the 7.16.0 part, this would lead to making the package backwards incompatible with 7.x from now on?

@mtojek
Copy link
Contributor

mtojek commented Mar 23, 2022

Yes, it's a breaking change for dashboards anyway. In this case you need to drop the 7.x part at all.

@ChrsMark
Copy link
Member Author

@elastic/obs-cloudnative-monitoring are we ok with such change, removing the backwards compatibility with 7.x?

@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 24, 2022

@mtojek is there any reference about such changes' sanity? I'm wondering how safe it is to just remove the 7.16 compatibility at this point but on the other hand we cannot just stick not doing the new changes. Do we have a specific plan for such cases so as to be covered in case we need a fix for 7.x?

@mtojek
Copy link
Contributor

mtojek commented Mar 24, 2022

There are teams which already decided to support only the 8.x packages and, in case there is a necessity to release a bugfix, they push modified packages directly to Package Storage. Fortunately, these are really rare cases. The idea is to prevent overusing backporting and just design features to be backward compatible.

There are open discussions around those issues and due to low priority, we don't look into them actively. I can give you references to those if you want to take a look and comment.

@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 24, 2022

There are teams which already decided to support only the 8.x packages and, in case there is a necessity to release a bugfix, they push modified packages directly to Package Storage. Fortunately, these are really rare cases. The idea is to prevent overusing backporting and just design features to be backward compatible.

There are open discussions around those issues and due to low priority, we don't look into them actively. I can give you references to those if you want to take a look and comment.

I see, thanks for the extra info.

@ChrsMark
Copy link
Member Author

As discussed internally with @elastic/obs-cloudnative-monitoring we move forward with releasing the package for version ^8.2 and if we need a bugfix for previous version of stack we will use direct branching and ad-hoc patch release of package.

For example at the moment we are on 1.17.3 version of package which is compatible with kibana.version: "^7.16.0 || ^8.0.0". So after this PR we will ship 1.18.0 versions of package with kibana.version: "^8.2.0" and if we need a bugfix compatible with previous versions we would branch out manually from 1.17.3 and ship 1.17.4 directly.

@mtojek
Copy link
Contributor

mtojek commented Mar 29, 2022

Zrzut ekranu 2022-03-29 o 16 21 18

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

@tetianakravchenko @MichaelKatsoulis this PR is ready for review :).

Copy link
Contributor

@tetianakravchenko tetianakravchenko 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: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

CI is failing because of #2906. I added the missing field here so this PR close #2906 too.

@ChrsMark ChrsMark merged commit 1137a2e into elastic:main Mar 30, 2022
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team [elastic/obs-cloudnative-monitoring]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[kubernetes] undocumented field detected
5 participants