Skip to content
This repository has been archived by the owner on Sep 17, 2024. It is now read-only.

[7.x] Backport #537 #558

Merged
merged 25 commits into from
Jan 12, 2021
Merged

[7.x] Backport #537 #558

merged 25 commits into from
Jan 12, 2021

Conversation

mdelapenya
Copy link
Contributor

What does this PR do?

It cherry-picks all commits in that PR, manually resolving conflicts.

Why is it important?

Keep 7.x branch up-to-date

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have run the Unit tests for the CLI, and they are passing locally
  • I have run the End-2-End tests for the suite I'm working on, and they are passing locally
  • I have noticed new Go dependencies (run make notice in the proper directory)

Author's Checklist

  • [ ]

Related issues

Follow-ups

Once merged using squash, try to backport that resulting commit to 7.10.x too

@mdelapenya mdelapenya mentioned this pull request Jan 5, 2021
8 tasks
.ci/Jenkinsfile Outdated
@@ -35,6 +35,7 @@ pipeline {
string(name: 'SLACK_CHANNEL', defaultValue: 'observablt-bots', description: 'The Slack channel(s) where errors will be posted. For multiple channels, use a comma-separated list of channels')
string(name: 'ELASTIC_AGENT_DOWNLOAD_URL', defaultValue: '', description: 'If present, it will override the download URL for the Elastic agent artifact. (I.e. https://snapshots.elastic.co/7.12.0-069dfaa4/downloads/beats/elastic-agent/elastic-agent-7.12.0-SNAPSHOT-linux-x86_64.tar.gz')
string(name: 'ELASTIC_AGENT_VERSION', defaultValue: '7.x-SNAPSHOT', description: 'SemVer version of the stand-alone elastic-agent to be used for Fleet tests. You can use here the tag of your PR to test your changes')
string(name: 'ELASTIC_AGENT_STALE_VERSION', defaultValue: '7.10.0', description: 'SemVer version of the stale stand-alone elastic-agent to be used for Fleet upgrade tests.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michalpristas what version should we use as stale in 7.x? Same question will come up for the backport to 7.10.x

Copy link
Contributor

Choose a reason for hiding this comment

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

7.x i would keep previous stable
for 7.10.x we could do 7.10.(x-1) or 7.10.0? what do you think?

Copy link
Contributor Author

@mdelapenya mdelapenya Jan 7, 2021

Choose a reason for hiding this comment

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

IIUC, the stale version is a previous, valid version, right? So:

  • For 7.x branch: current=7.x-SNAPSHOT and stale=?
  • For 7.10.x branch: current=7.10-SNAPSHOT and stale=?

Because we are using the alias, which holds a reference to the current moving target, how do we get that (-1)? There is a method to retrieve the current version: GetElasticArtifactVersion (see 4c1cea5#diff-04a1accd1c12dafffe8d3ecc43a623cd0bf22d03d3561f7e04e296e64962048eR121). What would be the algorithm to rest one to that version? Do you foresee any corner case?

Copy link
Contributor Author

@mdelapenya mdelapenya Jan 11, 2021

Choose a reason for hiding this comment

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

As discussed in today's conversation, @EricDavisX and I decided we are not going to backport the upgrade tests to 7.10.x
Update: I understood it wrong and we do need to backport this to 7.10.x

Copy link
Contributor Author

@mdelapenya mdelapenya Jan 11, 2021

Choose a reason for hiding this comment

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

BTW, @michalpristas @EricDavisX I'd see that in master branch, the value of the stale agent it should be 7.x-SNAPSHOT, which is ((master-1) == (8.0.0-SNAPSHOT - 1)), because master is always running in the edge. Wdyt? Or maybe I'm wrong and it needs the latest released artifact.

@mdelapenya mdelapenya requested a review from a team January 5, 2021 14:45
@mdelapenya mdelapenya self-assigned this Jan 5, 2021
@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 5, 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 #558 updated

  • Start Time: 2021-01-12T09:44:56.217+0000

  • Duration: 23 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 80
Skipped 12
Total 92

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 80
Skipped 12
Total 92

@mdelapenya
Copy link
Contributor Author

@michalpristas this PR backports your PR. I'll squash the commits and do same backport to 7.10.x. Do you think that branch needs to receive the backport?

@EricDavisX
Copy link
Contributor

I agree with 7.x using current minor version minus one as the stale version.

  • Which means 7.x = 7.12 (currently) so the stale is 7.11 latest snapshot agent.

The exceptions are 7.10 and 8.0.
7.10 can use the .patch version minus 1.

  • so in this case it is 7.10.x = 7.10.2 and the stale version can be hardcoded to 7.10.1 (only because this is the first minor version where upgrade support exists)

8.0 is 'master' and it can use the 7.x latest major for the 'stale' agent. If it used the latest minus one (if easier) that is ok too, to avoid too much logic here - what ever is quickest is ok by me.

.ci/Jenkinsfile Outdated Show resolved Hide resolved
@mdelapenya mdelapenya merged commit c460506 into elastic:7.x Jan 12, 2021
@mdelapenya mdelapenya deleted the backport-537 branch January 12, 2021 15:44
@mdelapenya mdelapenya mentioned this pull request Jan 12, 2021
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants