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

WIP NO-JIRA: experimental adaptation of signer rotation test as fuzz target #1709

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

vrutkovs
Copy link
Member

@vrutkovs vrutkovs commented Apr 5, 2024

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 5, 2024
@openshift-ci-robot
Copy link

@vrutkovs: This pull request explicitly references no jira issue.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

@vrutkovs vrutkovs changed the title NO-JIRA: experimental adaptation of signer rotation test as fuzz target WIP NO-JIRA: experimental adaptation of signer rotation test as fuzz target Apr 5, 2024
@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 Apr 5, 2024
@openshift-ci openshift-ci bot requested review from hexfusion and stlaz April 5, 2024 10:39
Copy link
Contributor

openshift-ci bot commented Apr 5, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vrutkovs
Once this PR has been reviewed and has the lgtm label, please assign stlaz for approval. For more information see the Kubernetes Code Review Process.

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

@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch from f7264bc to b920f86 Compare April 5, 2024 11:08
@vrutkovs
Copy link
Member Author

vrutkovs commented Apr 5, 2024

/cc @p0lyn0mial @benluddy

@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch 2 times, most recently from a84ce43 to b4f87c7 Compare April 5, 2024 12:50
@benluddy
Copy link
Contributor

benluddy commented Apr 5, 2024

Thanks for running with this. There were a few things that I wanted to look into but did not have time:

  1. The fuzz input is just a PRNG seed, so it doesn't really have an correlation with the outcome of the test. In its current state, I don't think we're benefiting from fuzzing vs. simply iterating over int64s sequentially. What might work is changing the fuzz input to represent the sequence of choices that the dispatcher makes. For example, if you have fewer than 256 instrumented actors, the fuzz input could be []byte and the dispatcher would do for _, n := range choices { dispatch(actors[n%len(actors)]) }.
  2. The dispatcher relies on knowing how many instrumented actors there are (and when they terminate) so that it can wait until every actor is blocked before it dispatches one of them. It's easy to use it wrong in a test and deadlock. I would like to see some kind of dispatcher builder that tests can use to register an actor and get back a lightweight handle for blocking and unregistering (and after unregistering, the handle only returns errors). Or any better design that makes it harder to misuse in tests.
  3. The sleep at the beginning of the fuzz target badly limits the fuzzer throughput.
  4. Instead of, or in addition to, having actors block on string action names, it could be nice to save the call stack.

@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch from 84c4a14 to 37c4516 Compare April 5, 2024 14:34
@vrutkovs
Copy link
Member Author

vrutkovs commented Apr 5, 2024

Implemented a list of choices (which replaces rng and loops over number of workers). Also added fuzzing over number of workers. However its odd that although UseSecretUpdateOnly is fuzzed too I don't see failures :/

@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch from 73729b2 to 50517d1 Compare April 8, 2024 14:45
@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch from 50517d1 to d09f6a9 Compare April 8, 2024 15:08
@vrutkovs
Copy link
Member Author

vrutkovs commented Apr 8, 2024

Addressed only 1 from #1709 (comment) so far

@vrutkovs vrutkovs force-pushed the experiment-fuzz-signer-rotation-devel branch from 3587a53 to e47b6c1 Compare April 9, 2024 11:39
Copy link
Contributor

openshift-ci bot commented Apr 9, 2024

@vrutkovs: all tests passed!

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.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 9, 2024
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2024
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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-sigs/prow repository.

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants