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

Make data injection idempotent #1085

Merged
merged 15 commits into from
Dec 6, 2022
Merged

Make data injection idempotent #1085

merged 15 commits into from
Dec 6, 2022

Conversation

jeff-mccoy
Copy link
Contributor

@jeff-mccoy jeff-mccoy commented Dec 5, 2022

This PR addresses some race conditions that can exist when running data injections or performing data injections. Key changes:

  • Replace WaitForPodsAndContainers() second argument, waitForAllPods with a new filter function and simply return all pods found leaving it to the caller to determine any further action (only data injection needed more than one pod)
  • Properly deal with matching the correct init containers based on the dataInjectionMarker so that we don't match other failed installs
  • Properly validate after injection that at least one pod with the correct dataInjectionMarker is now ready
  • No longer just warn and keep going on failure but actually retry the injection again
  • Add 3 x data injection tests to ensure idempotence is working as expected

@jeff-mccoy jeff-mccoy changed the title make data injection more idempotent Make data injection idempotent Dec 5, 2022
@jeff-mccoy jeff-mccoy marked this pull request as ready for review December 5, 2022 10:14
src/internal/cluster/data.go Outdated Show resolved Hide resolved
src/test/e2e/23_data_injection_test.go Outdated Show resolved Hide resolved
@jeff-mccoy
Copy link
Contributor Author

updated as requested

Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jeff-mccoy jeff-mccoy merged commit 5384789 into main Dec 6, 2022
@jeff-mccoy jeff-mccoy deleted the data-injection-things branch December 6, 2022 17:00
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
This PR addresses some race conditions that can exist when running data
injections or performing data injections. Key changes:

- Replace `WaitForPodsAndContainers()` second argument, `waitForAllPods`
with a new filter function and return all pods found leaving it
to the caller to determine any further action (only data injection
needed more than one pod)
- Properly deal with matching the correct init containers based on the
`dataInjectionMarker` so that we don't match other failed installs
- Properly validate after injection that at least one pod with the
correct `dataInjectionMarker` is now ready
- No longer warn and keep going on failure but retry the
injection again
- Add 3 x data injection tests to ensure idempotence is working as
expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants