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 Injections Happen asyc w/ Component Deployment #425

Merged
merged 25 commits into from
Mar 31, 2022

Conversation

YrrepNoj
Copy link
Contributor

@YrrepNoj YrrepNoj commented Mar 30, 2022

Description

Data injections used to operate after all the components have been deployed. This is an issue when apps are expecting data before they report themselves as healthy. When this happens the pod is never able to report healthy because their init container is waiting for the data which isn't getting injected because the pods are not reporting healthy yet. A un-fun loop.

This PR moves the data-injection operation at the same time as the component deployment and has the injection happen as a go routine that way once the pods come up the data will be injected in the background.

Related Issue

Fixes #423

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist before merging

  • Tests have been added/updated as necessary (add the needs-tests label)
  • Documentation has been updated as necessary (add the needs-docs label)

RothAndrew and others added 4 commits March 29, 2022 13:41
Update data-injection demo such that it uses an init container,
which is closer to what a realistic scenario would use.
…ta-injection-example' into fix/data-injection-race-condition
jeff-mccoy
jeff-mccoy previously approved these changes Mar 30, 2022
@jeff-mccoy jeff-mccoy added bug 🐞 Something isn't working packager labels Mar 30, 2022
@RothAndrew
Copy link
Contributor

update the schema file?

@YrrepNoj
Copy link
Contributor Author

@jeff-mccoy I thought this update would mean that we wouldn't need the loop when checking the data-injection results but that doesn't seem to be the case.

jeff-mccoy
jeff-mccoy previously approved these changes Mar 30, 2022
RothAndrew
RothAndrew previously approved these changes Mar 30, 2022
RothAndrew
RothAndrew previously approved these changes Mar 30, 2022
@YrrepNoj YrrepNoj force-pushed the fix/data-injection-race-condition branch from 1c2b72a to 09e0882 Compare March 30, 2022 19:39
@YrrepNoj
Copy link
Contributor Author

For some reason, the k8s api must be doing some weird cashing things. The exec is not showing files in the volume that should be there because:

  1. we have the init container waiting for the file to exist before the pod starts
  2. we have a ReadinessProbe waiting for the files to exist and Zarf is waiting for all pods to start being continuing with the test
  3. doing a manual go execkubectl exec POD -- ls /test actually shows the files there.

Screen Shot 2022-03-30 at 10 28 52 PM

Because of that I'm going to add the retry loop to the test. 👎

Copy link
Contributor

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

Freaking finally LGTM

@jeff-mccoy jeff-mccoy merged commit 039eec7 into master Mar 31, 2022
@jeff-mccoy jeff-mccoy deleted the fix/data-injection-race-condition branch March 31, 2022 04:57
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
- Breaking Change: move "data" to components -> "dataInjections"
- Update data-injection example to use initContainer & pod variants for E2E
- Cleanup E2E flow for data injection tests
- Fix issues with race conditions involving initContainers data injections & Helm wait
- Way too much work on making tests not cry with async ops for data injection

Co-authored-by: Andy Roth <[email protected]>
Co-authored-by: Jeff McCoy <[email protected]>
Co-authored-by: Jon Perry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support injecting data to pods with init containers
3 participants