Skip to content

Conversation

@eggfoobar
Copy link
Contributor

Signed-off-by: ehila ehila@redhat.com

Signed-off-by: ehila <ehila@redhat.com>
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2022
@openshift-ci openshift-ci bot requested review from bbguimaraes and dgoodwin June 8, 2022 04:25
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 8, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eggfoobar
To complete the pull request process, please assign petr-muller after the PR has been reviewed.
You can assign the PR to them by writing /assign @petr-muller in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eggfoobar
Copy link
Contributor Author

/cc @DennisPeriquet @neisw

@openshift-ci openshift-ci bot requested review from DennisPeriquet and neisw June 8, 2022 12:33

## Summary

Disruption tests are a set of tests that measure an endpoints resilience or an allowed alert duration. As the `e2e tests` are run the results are matched against historical data to determine if the disruption intervals are within expectations.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/endpont/endpoint's/

we are measuring disruption which is defined as the amount time an endpoint is not responding to requests (e.g., during an upgrade). The disruption testing is done independent of alerts checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention that the doc in this PR is specifically for documenting/explaining the part about disruption testing that relates to how disruption data is stored and later used to determine pass/fail criteria for disruption tests that are run later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just saw your link on origin/pkg/synthetictests/allowedalerts/types.go and wonder if this is part of disruption testing -- I don't know the answer to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, the intent here was to try and cover both, from my understanding the way that the disruption and alert numbers are gathered is the same. Even the matcher is the same, the only thing that is different is the historical data set which we check against. Maybe we say this is documentation on how we determine allowed tolerations?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think describing both is a good idea but I think since they are quite similar, I recommend just mentioning the difference as you just explained.

@neisw
Copy link
Contributor

neisw commented Jun 8, 2022

@eggfoobar, we are discussing how best to coordinate the separate efforts for building out the disruption documentation. Also have #259 going. Want to promote one of these as a template / skeleton where we can then fill in the pieces individually. Do you have a preference for promoting incomplete doc to master or choosing one of these forks as the 'master' and merging updates into it?

…ference easier

Signed-off-by: ehila <ehila@redhat.com>
@eggfoobar
Copy link
Contributor Author

@eggfoobar, we are discussing how best to coordinate the separate efforts for building out the disruption documentation. Also have #259 going. Want to promote one of these as a template / skeleton where we can then fill in the pieces individually. Do you have a preference for promoting incomplete doc to master or choosing one of these forks as the 'master' and merging updates into it?

@neisw I'm okay with changing it up any way you guys see fit. If it'll make things easier I don't mind forking off of that PR so we can publish it all at the same time when we feel we've covered it well.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 8, 2022

@eggfoobar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/links cb800a2 link true /test links

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@neisw
Copy link
Contributor

neisw commented Jun 8, 2022

@neisw I'm okay with changing it up any way you guys see fit. If it'll make things easier I don't mind forking off of that PR so we can publish it all at the same time when we feel we've covered it well.

@eggfoobar, if it isn't too much trouble let's do that. Feel free to supersede anything I have and / or add modify the sections. Appreciate the help on this.

@eggfoobar
Copy link
Contributor Author

@neisw If we're good with that, I created the PR with this information, closing it for now neisw#2

@eggfoobar eggfoobar closed this Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants