Closed omus closed 3 years ago
Appears that the manager could not connect to one or more of the workers in the "test-multi-addprocs" test. Possibly this is related to the port changes and using multiple nodes.
https://github.com/beacon-biosignals/K8sClusterManagers.jl/runs/2440135312
Re-running with the corrected debugging tooling did not result in the same error occurring :/
It's hard to be sure of what the problem was but I'll switch to using a fixed port number to avoid any random errors that could be occurring from accidentally binding to a port that is in use.
Looks like I may to close/open to use the changes in #45
Merging #44 (b0ed6be) into main (533a92b) will increase coverage by
0.04%
. The diff coverage is12.50%
.:exclamation: Current head b0ed6be differs from pull request most recent head 759761a. Consider uploading reports for the commit 759761a to get more accurate results
@@ Coverage Diff @@
## main #44 +/- ##
==========================================
+ Coverage 52.81% 52.85% +0.04%
==========================================
Files 4 4
Lines 142 140 -2
==========================================
- Hits 75 74 -1
+ Misses 67 66 -1
Impacted Files | Coverage Δ | |
---|---|---|
src/native_driver.jl | 4.47% <0.00%> (+0.06%) |
:arrow_up: |
src/pod.jl | 96.22% <100.00%> (-0.07%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 533a92b...759761a. Read the comment docs.
Fixes #39 by using unique names for each worker pod. It was also noticed that logic for using unique ports wasn't to avoid port conflicts but rather an attempt to make the pod names unique. Because of this the use of ports has mostly been refactored away.
Bonus change: Originally the cluster tests on the CI needed to use 1 node per pod to ensure there was enough CPU resources. As we can request partial CPU resources the use of multiple nodes on the CI is probably no longer necessary but I'll leave this in place for now.
Was originally part of: #41