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

Add upgrade rule smoke tests #136048

Merged
merged 6 commits into from
Jul 20, 2022
Merged

Conversation

liza-mae
Copy link
Contributor

@liza-mae liza-mae commented Jul 8, 2022

Adding an upgrade test for issue #135740.

@liza-mae liza-mae added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.4.0 v8.3.3 labels Jul 8, 2022
@liza-mae liza-mae requested a review from kobelb July 8, 2022 21:32
@liza-mae liza-mae self-assigned this Jul 8, 2022
@liza-mae
Copy link
Contributor Author

@elasticmachine merge upstream

@liza-mae
Copy link
Contributor Author

@kobelb can I get a review for this please?

@kobelb
Copy link
Contributor

kobelb commented Jul 13, 2022

How is the "Upgrade Rule" being created? I don't see where we load the problematic alerting rule that would cause the rule to fail to run.

Also, this test is only checking the last response status. Depending on how the alerting rule is created, this might miss an upgrade failure if the alerting rule didn't have the chance to run yet.

@liza-mae
Copy link
Contributor Author

The loading of the rule is done in separate repository as part of the upgrade setup. It uses the rest API to create it, I will put up that PR and link it here so you can review. I was able to reproduce the bug with the rule I created, however I did not verify the last response in the setup, maybe it is needed there. I will make the updates and let you know.

@liza-mae
Copy link
Contributor Author

@elasticmachine merge upstream

@liza-mae
Copy link
Contributor Author

liza-mae commented Jul 14, 2022

@kobelb the rule setup is linked to this is PR if you would like to review again.

@kobelb
Copy link
Contributor

kobelb commented Jul 14, 2022

Thanks @liza-mae. Is the rule setup done on an old version of Kibana prior to upgrading to a new version and then the smoke tests are done?

If so, that relegates my concerns to the fact that this alerting rule might not have had a chance to run prior to the smoke test verifying that the last response status is "OK". This is potentially unlikely, and the only way I can think of detecting that is by comparing the last time it ran prior to the upgrade to post upgrade, or having the smoke test wait for the last response to update.

@liza-mae
Copy link
Contributor Author

@kobelb the setup is done an older version of kibana. I added a rest api call to create the rule and it appeared to work fine with the testing I did on 7.13.x, 7.17.x, 8.2.x, 8.3.x and they all had status Ok except for the bug originating from 8.2.x where it had Error and this test caught it. This was done in local testing, I will need to merge the PR to try it on all the paths in CI though, if there is a concern the rule does not run and I can adjust the setup. In what cases would the rule not run after it is created?

@liza-mae
Copy link
Contributor Author

@liza-mae
Copy link
Contributor Author

When I run the API setup, I get this in the UI:

Screenshot from 2022-07-14 13-51-59

I have not seen it be anything other than Ok in the setup when testing of the versions I listed above. It appears the rule is executed when created and then at each interval listed after that. We can have a quick meeting so I can get clarification if I am missing something.

@liza-mae
Copy link
Contributor Author

@elasticmachine merge upstream

@liza-mae
Copy link
Contributor Author

@elasticmachine merge upstream

@liza-mae
Copy link
Contributor Author

liza-mae commented Jul 20, 2022

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @liza-mae

@liza-mae liza-mae merged commit d862f6f into elastic:main Jul 20, 2022
@liza-mae liza-mae deleted the liza/add-rule-upgrade-test branch July 20, 2022 17:55
kibanamachine pushed a commit that referenced this pull request Jul 20, 2022
* Add upgrade rule smoke tests

* Add version check

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit d862f6f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Jul 20, 2022
* Add upgrade rule smoke tests

* Add version check

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit d862f6f)

Co-authored-by: liza-mae <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v8.3.3 v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants