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.

Sign up for GitHub

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

Jump to bottom

Cancel agent command #279

Merged
merged 4 commits into from
Aug 18, 2023
Merged

Cancel agent command #279

merged 4 commits into from
Aug 18, 2023

Conversation

pablochacin
Copy link
Collaborator

@pablochacin pablochacin commented Aug 4, 2023 •
edited
Loading

Description

Fixes #82

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make test) and all tests pass.
  • I have run relevant e2e test locally (make e2e-xxx for agent, disruptors, kubernetes or cluster related changes)
  • Any dependent changes have been merged and published in downstream modules

@pablochacin pablochacin force-pushed the cancel-agent-command branch 7 times, most recently from 7610595 to 00bc3d2 Compare August 16, 2023 17:54
@pablochacin pablochacin mentioned this pull request Aug 17, 2023
Closed
6 tasks
@pablochacin pablochacin marked this pull request as ready for review August 17, 2023 15:09
@pablochacin pablochacin changed the title WIP Cancel agent command Cancel agent command Aug 17, 2023
roobre
roobre reviewed Aug 17, 2023
View reviewed changes
Copy link
Member

@roobre roobre left a comment

Choose a reason for hiding this comment

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

I overall agree with the approach, left some comments regarding implementation details.

cmd/agent/commands/cleanup.go Show resolved Hide resolved
pkg/agent/agent.go Outdated Show resolved Hide resolved
cmd/agent/commands/cleanup.go Outdated Show resolved Hide resolved
pkg/disruptors/controller.go Outdated Show resolved Hide resolved
pkg/disruptors/controller.go Outdated Show resolved Hide resolved
pkg/disruptors/controller.go Show resolved Hide resolved
pkg/disruptors/pod_test.go Show resolved Hide resolved
pkg/runtime/lock.go Outdated Show resolved Hide resolved
pkg/runtime/lock.go Outdated Show resolved Hide resolved
@pablochacin pablochacin merged commit 1cfe9cf into main Aug 18, 2023
6 checks passed
@pablochacin pablochacin deleted the cancel-agent-command branch August 18, 2023 10:52
roobre
roobre approved these changes Aug 18, 2023
View reviewed changes
Copy link
Member

@roobre roobre left a comment

Choose a reason for hiding this comment

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

Looking great and working wonderfully according to some manual testing. Great work!

@pablochacin pablochacin mentioned this pull request Aug 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roobre roobre roobre approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

xk6-disruptor-agent does not terminate if test is cancelled
2 participants
@pablochacin @roobre