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

Fix race condition for Docker local tests #1384

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Fix race condition for Docker local tests #1384

merged 3 commits into from
Nov 14, 2024

Conversation

achantavy
Copy link
Contributor

@achantavy achantavy commented Nov 14, 2024

Summary

Describe your changes.

Before:

  1. Pull down a fresh cartography from github
    Screenshot 2024-11-14 at 2 16 51 PM

  2. Build the container with docker build -t cartography-cncf/cartography-dev -f dev.Dockerfile ./ (I forgot to screenshot this step)

  3. Running the docker-compose linter works on the first try because the initialization step takes a while:
    Screenshot 2024-11-14 at 2 17 23 PM

  4. [Failure] But subsequent linter runs fail with "is this a git repository?"
    Screenshot 2024-11-14 at 2 17 33 PM

After:

  1. Pull down fresh cartography from github and checkout this fix branch (fixdcwait)
    image

  2. Build container
    image

  3. Run linter on the first try
    image

  4. [SUCCESS] Subsequent linter runs work because we've added a waiting mechanism
    Screenshot 2024-11-14 at 2 25 59 PM

Related issues or links

Include links to relevant issues or other pages.

Checklist

Provide proof that this works (this makes reviews move faster). Please perform one or more of the following:

  • Update/add unit or integration tests.
  • Include a screenshot showing what the graph looked like before and after your changes.
  • Include console log trace showing what happened before and after your changes.

Signed-off-by: Alex Chantavy <[email protected]>
Signed-off-by: Alex Chantavy <[email protected]>
@achantavy achantavy enabled auto-merge (squash) November 14, 2024 22:34
@achantavy achantavy merged commit c86e388 into master Nov 14, 2024
7 checks passed
@achantavy achantavy deleted the fixdcwait branch November 14, 2024 22:57
chandanchowdhury pushed a commit to chandanchowdhury/cartography that referenced this pull request Nov 27, 2024
## Summary
> Describe your changes.

### Before:

1. Pull down a fresh cartography from github
![Screenshot 2024-11-14 at 2 16
51 PM](https://github.com/user-attachments/assets/ad7a5706-c670-45a2-894d-44329eed28a4)

2. Build the container with `docker build -t
cartography-cncf/cartography-dev -f dev.Dockerfile ./` (I forgot to
screenshot this step)

3. Running the docker-compose linter works on the first try because the
initialization step takes a while:
![Screenshot 2024-11-14 at 2 17
23 PM](https://github.com/user-attachments/assets/148c5efa-799a-425d-b52c-0fca33f28d5a)

4. [Failure] But subsequent linter runs fail with "is this a git
repository?"
![Screenshot 2024-11-14 at 2 17
33 PM](https://github.com/user-attachments/assets/2447c0aa-26ac-447a-85bb-ade408db9d21)

### After:

1. Pull down fresh cartography from github and checkout this fix branch
(fixdcwait)

![image](https://github.com/user-attachments/assets/1fbafca6-6b6e-4e9f-a43c-7df2efbfbc03)

2. Build container

![image](https://github.com/user-attachments/assets/60b93567-85bc-4b24-97c9-879adab49ee2)

3. Run linter on the first try

![image](https://github.com/user-attachments/assets/7d8d9a52-9f3c-4501-82a0-60abfb3f29d7)

4. [SUCCESS] Subsequent linter runs work because we've added a waiting
mechanism
![Screenshot 2024-11-14 at 2 25
59 PM](https://github.com/user-attachments/assets/a8dfe235-95e4-4a40-83ec-c9c758928e8b)

### Related issues or links
> Include links to relevant issues or other pages.

- https://github.com/lyft/cartography/issues/...

### Checklist

Provide proof that this works (this makes reviews move faster). Please
perform one or more of the following:
- [ ] Update/add unit or integration tests.
- [x] Include a screenshot showing what the graph looked like before and
after your changes.
- [x] Include console log trace showing what happened before and after
your changes.

---------

Signed-off-by: Alex Chantavy <[email protected]>
Signed-off-by: chandanchowdhury <[email protected]>
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