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

[Standalone Agent] Disallow upgrade if upgrade is already in progress #3473

Merged

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Sep 25, 2023

What does this PR do?

This PR prevents users of standalone Agents from being able to upgrade the Agent if an upgrade is already in progress.

Why is it important?

Attempting to upgrade an Agent while an upgrade is already in progress could corrupt the Agent installation.

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 added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

Related issues

@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 25, 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-09-28T20:54:27.421+0000

  • Duration: 25 min 20 sec

Test stats 🧪

Test Results
Failed 0
Passed 6333
Skipped 59
Total 6392

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 26, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.78% (81/82) 👍
Files 66.102% (195/295) 👍
Classes 65.693% (360/548) 👍
Methods 52.766% (1135/2151) 👍 0.022
Lines 38.208% (12890/33736) 👍 0.041
Conditionals 100.0% (0/0) 💚

@ycombinator ycombinator added the Team:Elastic-Agent Label for the Agent team label Sep 26, 2023
@ycombinator ycombinator marked this pull request as ready for review September 26, 2023 11:13
@ycombinator ycombinator requested a review from a team as a code owner September 26, 2023 11:13
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@jlind23 jlind23 requested a review from pchila September 27, 2023 05:28
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@@ -156,6 +156,7 @@ type DiagnosticComponentResult struct {
}

// Client communicates to Elastic Agent through the control protocol.
// go:generate mockery --name Client
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@pchila pchila left a comment

Choose a reason for hiding this comment

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

Code looks good.
There is a clash between your changes for the upgrade integration test and the modifications proposed in #3477 so I guess we have to sync a bit to make sure we don't break/lose testcases (pinging @blakerouse so he can have a look at this)

For the code coverage, I guess we have to exclude mocks/*.go from analysis since most of the uncovered lines are actually the generated code.

@ycombinator
Copy link
Contributor Author

For the code coverage, I guess we have to exclude mocks/*.go from analysis since most of the uncovered lines are actually the generated code.

Done in 98764d8.

@ycombinator ycombinator force-pushed the standalone-prevent-quick-upgrades branch from 85507df to 2cffce6 Compare September 28, 2023 16:51
@mergify
Copy link
Contributor

mergify bot commented Sep 28, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b standalone-prevent-quick-upgrades upstream/standalone-prevent-quick-upgrades
git merge upstream/main
git push upstream standalone-prevent-quick-upgrades

@elastic-sonarqube
Copy link

@ycombinator ycombinator merged commit bc1982a into elastic:main Sep 29, 2023
24 checks passed
mergify bot pushed a commit that referenced this pull request Sep 29, 2023
…#3473)

* [Standalone] Disallow upgrade if upgrade is already in progress

* Adding CHANGELOG entry

* Add TODOs

* WIP: integration test

* Check for Watcher; move function to upgrade package

* Check Upgrade Watcher PIDs first

* Fix syntax error

* Adjust test to start from older version

* Fixing more errors

* Fix test

* Mock client

* Fix typo

(cherry picked from commit bc1982a)

# Conflicts:
#	sonar-project.properties
#	testing/integration/upgrade_test.go
ycombinator added a commit that referenced this pull request Oct 2, 2023
…#3473)

* [Standalone] Disallow upgrade if upgrade is already in progress

* Adding CHANGELOG entry

* Add TODOs

* WIP: integration test

* Check for Watcher; move function to upgrade package

* Check Upgrade Watcher PIDs first

* Fix syntax error

* Adjust test to start from older version

* Fixing more errors

* Fix test

* Mock client

* Fix typo

(cherry picked from commit bc1982a)

# Conflicts:
#	sonar-project.properties
#	testing/integration/upgrade_test.go
@ycombinator ycombinator deleted the standalone-prevent-quick-upgrades branch November 27, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants