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

Enable package compatibility checks with older Elastic stacks #740

Merged
merged 17 commits into from Mar 1, 2021

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Feb 23, 2021

What does this PR do?

This PR enables package compatibility checks with older Elastic stacks. A package will be installed (not tested) in Kibana.

Checklist

- [ ] I have reviewed [tips for building integrations](https://github.com/elastic/integrations/blob/master/docs/tips_for_building_integrations.md) 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.

How to test this PR locally

CI will verify it.

Related issues

@mtojek mtojek self-assigned this Feb 23, 2021
@mtojek
Copy link
Contributor Author

mtojek commented Feb 23, 2021

Blocked by: elastic/beats#24163

@elasticmachine
Copy link

elasticmachine commented Feb 23, 2021

💚 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #740 updated

  • Start Time: 2021-03-01T14:01:24.218+0000

  • Duration: 38 min 43 sec

  • Commit: ed26c02

Test stats 🧪

Test Results
Failed 0
Passed 1723
Skipped 3
Total 1726

Trends 🧪

Image of Build Times

Image of Tests

@mtojek
Copy link
Contributor Author

mtojek commented Feb 25, 2021

I found a weird behavior when used -SNAPSHOT for constraint checks:

../../build/elastic-package install -c kibana.version=7.11.0-SNAPSHOT -v
— Check compatibility with 7.11.0-SNAPSHOT
<1s
[2021-02-25T12:40:26.436Z] + ../../build/elastic-package install -c kibana.version=7.11.0-SNAPSHOT -v
[2021-02-25T12:40:26.436Z] 2021/02/25 12:40:26 DEBUG Enable verbose logging
[2021-02-25T12:40:26.436Z] Check conditions for package
[2021-02-25T12:40:26.436Z] 2021/02/25 12:40:26 DEBUG Verify Kibana version (constraint: ^7.9.0, requirement: 7.11.0-SNAPSHOT)
[2021-02-25T12:40:26.436Z] Error: checking conditions failed: Kibana constraint unsatisfied: [0] 7.11.0-SNAPSHOT is a prerelease version and the constraint is only looking for release versions

I will try with releases, e.g. 7.11.0, 7.10.0

@mtojek
Copy link
Contributor Author

mtojek commented Feb 25, 2021

Ok, without SNAPSHOT tags, the elastic-package properly considers the version as valid/compatible. It fails because of #745 (need to merge this one first).

@mtojek
Copy link
Contributor Author

mtojek commented Feb 25, 2021

I think I have to improve the elastic-package, as it supports only released version:

docker pull docker.elastic.co/elasticsearch/elasticsearch:7.11.0 - works

docker pull docker.elastic.co/elasticsearch/elasticsearch:7.12.0 - doesn't work, as 7.12.0 is not released

@mtojek
Copy link
Contributor Author

mtojek commented Feb 25, 2021

@ycombinator it's not ready for merge yet (-SNAPSHOT case), but feel free to comment if you want to.

EDIT:

I think I will fix the condition in a follow up PR. This one already polishes a couple packages in terms of compatibility.

@mtojek mtojek marked this pull request as ready for review February 25, 2021 18:02
try {
dir("${BASE_DIR}/packages/${integrationName}") {
sh(label: "Boot up the Elastic stack (${stackVersion})", script: "../../build/elastic-package stack up -d -v --version ${stackVersion}")
sh(label: "Install integration: ${integrationName}", script: '''
Copy link
Contributor

Choose a reason for hiding this comment

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

So it seems we are calling a package as compatible if we are able to install it. Is this sufficient or do we also want to run tests for the package against the previous stack versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I prefer to stick to installation as testing will elongate test execution much (+20minutes).

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. Does installing a package result in any testing? In other words, what are the reasons this step would fail in CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e.g. installation can check if all Kibana objects can be installed (correct migrationVersion field).

@@ -11,7 +11,7 @@ release: experimental
license: basic
type: integration
conditions:
kibana.version: '^7.9.0'
kibana.version: '^7.11.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't understand why we need these kibana.version bumps here and in other packages in this PR?

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 because of the fact that these packages are not compatible with older Kibana stacks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Is there a reason these changes are part of this PR? In other words, do we need these changes anyway even without this PR? Did the compatibility checks introduced in this PR catch that these packages needed kibana version bumps?

Copy link
Contributor Author

@mtojek mtojek Mar 1, 2021

Choose a reason for hiding this comment

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

Did the compatibility checks introduced in this PR catch that these packages needed kibana version bumps?

Yes, exactly this. Otherwise the CI for this PR will be red.

@mtojek mtojek requested a review from ycombinator March 1, 2021 13:56
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM. This is a nice addition!

@mtojek mtojek merged commit 06284f6 into elastic:master Mar 1, 2021
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
…c#740)

* Start stack two more times

* Check package compatibility with older stacks

* Correct versions

* nit

* Fix

* Fix: ignore prerelease

* Adjust versions

* Update release numbers

* Fix: shellinit

* Fix

* Fix

* Fix

* Fix compatibility with Kibana
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable backward compatibility tests
3 participants