Skip to content

Test each integration with the oldest supported version - #1541

Merged
jsoriano merged 29 commits into
elastic:masterfrom
jsoriano:integrations-stack-version
Aug 26, 2021
Merged

Test each integration with the oldest supported version#1541
jsoriano merged 29 commits into
elastic:masterfrom
jsoriano:integrations-stack-version

Conversation

@jsoriano

@jsoriano jsoriano commented Aug 18, 2021

Copy link
Copy Markdown
Contributor

What does this PR do?

Test each package with the oldest supported version of the stack. This will help making the builds a bit more deterministic, while ensuring that no changes are introduced that break the condition of supported version.

If the version doesn't exist yet, it will try to use the closer snapshot, for example if 7.14.1 doesn't exist, it will try to use 7.14.1-SNAPSHOT or 7.x-SNAPSHOT, this will allow to develop integrations with unreleased features.

Some packages using 7.14.0 have been modified to require at least 7.14.1 as they were affected by elastic/beats#27299.

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.
  • If I'm introducing a new feature, I have modified the Kibana version constraint in my package's manifest.yml file to point to the latest Elastic stack release (e.g. ^7.13.0).

Author's Checklist

How to test this PR locally

Related issues

Screenshots

@jsoriano
jsoriano force-pushed the integrations-stack-version branch 2 times, most recently from f708e3c to 1b79f82 Compare August 18, 2021 12:15
@jsoriano
jsoriano force-pushed the integrations-stack-version branch from 1b79f82 to 10afe79 Compare August 18, 2021 12:16
@elasticmachine

elasticmachine commented Aug 18, 2021

Copy link
Copy Markdown

💚 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: 2021-08-26T10:35:16.425+0000

  • Duration: 70 min 37 sec

  • Commit: e1722f5

Test stats 🧪

Test Results
Failed 0
Passed 2896
Skipped 3
Total 2899

Trends 🧪

Image of Build Times

Image of Tests

@jsoriano
jsoriano force-pushed the integrations-stack-version branch 4 times, most recently from ce1d34c to beaf3e1 Compare August 18, 2021 15:58
@jsoriano
jsoriano force-pushed the integrations-stack-version branch from beaf3e1 to aa483ef Compare August 18, 2021 16:00
@jsoriano
jsoriano marked this pull request as ready for review August 23, 2021 17:00
@elasticmachine

Copy link
Copy Markdown

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine

Copy link
Copy Markdown

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@jsoriano
jsoriano requested review from andrewkroh and ruflin August 23, 2021 17:00
@jsoriano
jsoriano force-pushed the integrations-stack-version branch from 0229fcd to 76570a2 Compare August 23, 2021 17:14

@ruflin ruflin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really like the idea that we test with the oldest version supported.

Later on, I wonder if we could also do a test run with the newest version supported. This might be unstable and maybe only run ones every day / week to show where we are standing.

type: integration
conditions:
kibana.version: "^7.14.0"
kibana.version: "^7.14.1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this change by itention? Same for the other packaegs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is intentional, these packages fail to install in 7.14.0, probably due to elastic/beats#27299.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it. Sounds like we should start working on elastic_agent.version: ... as this one is not Kibana related.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think we are going to need conditions for elastic agent soon, but this will imply more changes.

@jsoriano

Copy link
Copy Markdown
Contributor Author

Later on, I wonder if we could also do a test run with the newest version supported. This might be unstable and maybe only run ones every day / week to show where we are standing.

Yep, agree, I was thinking on adding a jenkins parameter so we can launch all the tests with an specific version, this would allow for example to run the tests against a build candidate. This could be run with master/7.x on a daily basis.

Comment thread .ci/Jenkinsfile Outdated
Comment thread .ci/Jenkinsfile Outdated
}

def findStackVersion(String versionCondition) {
def version = versionCondition.substring(1)

@endorama endorama Aug 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this line supposed to remove the initial v from the version string? I would add a note about it's behaviour.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree, this is a bit obscure. Victor is moving this function to the shared library and we are being more explicit about this there (explicitly checking that first character is ^, documenting that we only support this operator here and testing this expectation). You can find it here: elastic/apm-pipeline-library#1248

Comment thread .ci/Jenkinsfile Outdated
Comment on lines +254 to +255
// Old minors may not be available in artifacts-api, if it is older
// than the others in the same major, return the version as is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is not clear to me what is happening here and why returning the version as is is the correct choice.
Does artifacts-api delete all minors apart from latest? (so for example it keeps 7.x and not 7.x-1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, older versions are not available in this API, you can see it here: https://artifacts-api.elastic.co/v1/versions. For example 7.13.0 is not there now.

In these cases I am assuming that these older versions existed, and their docker images are available.

Comment thread .ci/Jenkinsfile Outdated
Comment thread .ci/Jenkinsfile Outdated
jsoriano and others added 4 commits August 24, 2021 17:29
Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
Comment thread .ci/Jenkinsfile
if (manifest != null && manifest["conditions"] != null) {
def kibanaVersionCondition = manifest["conditions"]["kibana.version"]
if (kibanaVersionCondition != null) {
def version = findOldestSupportedVersion(versionCondition: kibanaVersionCondition)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I cannot find findOldestSupportedVersion, where is it defined?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We use a shared library for jenkinsfiles, this function has been moved there: elastic/apm-pipeline-library#1248

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@francescayeye

francescayeye commented Aug 26, 2021

Copy link
Copy Markdown

prepareStack() LGTM, cannot say about conditions: kibana.version bump, I assume it is correct as well

@jsoriano

Copy link
Copy Markdown
Contributor Author

cannot say about conditions: kibana.version bump, I assume it is correct as well

This is needed for packages that fail to install in 7.14.0 (see this comment #1541 (comment)).

@jsoriano
jsoriano merged commit f50eb7c into elastic:master Aug 26, 2021
@jsoriano
jsoriano deleted the integrations-stack-version branch August 26, 2021 13:18
orestisfl pushed a commit to orestisfl/integrations that referenced this pull request May 15, 2026
Test each package with the oldest supported version of the stack. This
will help making the builds a bit more deterministic, while ensuring that
no changes are introduced that break the condition of supported version.

If the version doesn't exist yet, it will try to use the closer snapshot, for
example if 7.14.1 doesn't exist, it will try to use 7.14.1-SNAPSHOT or
7.x-SNAPSHOT, this will allow to develop integrations with unreleased
features.

Some packages using 7.14.0 have been modified to require at least 7.14.1
as they were affected by elastic/beats#27299.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Integrations Label for the Integrations team

6 participants