Skip to content

fix(kubernetes): Coordinate Runtime's RunContainer/TailContainer via a channel#286

Closed
cognifloyd wants to merge 1 commit intogo-vela:masterfrom
cognifloyd:fix-k8s-stream-run-race
Closed

fix(kubernetes): Coordinate Runtime's RunContainer/TailContainer via a channel#286
cognifloyd wants to merge 1 commit intogo-vela:masterfrom
cognifloyd:fix-k8s-stream-run-race

Conversation

@cognifloyd
Copy link
Copy Markdown
Member

The Kubernetes runtime needs to start streaming logs before the container starts.
The Docker runtime needs to start the container before streaming longs.
With both running in goroutines, the Runtime can block one method until the other completes in whichever order makes sense for that Runtime.

Alternate to #277

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 12, 2022

Codecov Report

Merging #286 (a6417a6) into master (c1d4563) will decrease coverage by 0.77%.
The diff coverage is 97.01%.

❗ Current head a6417a6 differs from pull request most recent head b91cf19. Consider uploading reports for the commit b91cf19 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage   78.91%   78.13%   -0.78%     
==========================================
  Files          67       67              
  Lines        4870     4954      +84     
==========================================
+ Hits         3843     3871      +28     
- Misses        884      944      +60     
+ Partials      143      139       -4     
Impacted Files Coverage Δ
runtime/kubernetes/container.go 55.92% <42.85%> (-21.69%) ⬇️
executor/linux/secret.go 73.62% <100.00%> (+3.15%) ⬆️
executor/linux/service.go 64.14% <100.00%> (+1.60%) ⬆️
executor/linux/step.go 66.38% <100.00%> (+1.27%) ⬆️
executor/local/service.go 92.17% <100.00%> (+1.35%) ⬆️
executor/local/step.go 88.65% <100.00%> (+1.55%) ⬆️
runtime/docker/container.go 85.18% <100.00%> (+0.31%) ⬆️
... and 1 more

@cognifloyd cognifloyd force-pushed the fix-k8s-stream-run-race branch from 4bb4b83 to 0b94b85 Compare March 12, 2022 05:21
@cognifloyd cognifloyd changed the title Coordinate Runtime's RunContainer/TailContainer via a channel fix(kubernetes): Coordinate Runtime's RunContainer/TailContainer via a channel Mar 15, 2022
The Kubernetes runtime needs to start streaming logs before the container starts.
The Docker runtime needs to start the container before streaming longs.
With both running in goroutines, the Runtime can block one method until the other completes
in whichever order makes sense for that Runtime.
@cognifloyd
Copy link
Copy Markdown
Member Author

Closing in favor of #303

@cognifloyd cognifloyd closed this Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant