feat: support falling back to docker inspect data for gleaning mgmt bridge gw address #1482
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
- Coverage 21.83% 21.77% -0.07%
==========================================
Files 65 65
Lines 6809 6830 +21
==========================================
Hits 1487 1487
- Misses 5191 5212 +21
Partials 131 131
|
hellt
reviewed
Jul 25, 2023
| @@ -256,7 +256,33 @@ func (d *DockerRuntime) CreateNet(ctx context.Context) (err error) { | |||
| // this was added to allow mgmt network gw ip to be available in a startup config templation step (ceos) | |||
| var v4, v6 string | |||
| if v4, v6, err = utils.FirstLinkIPs(bridgeName); err != nil { | |||
There was a problem hiding this comment.
@carlmontanari was it painful to look at that err handling line? :P
There was a problem hiding this comment.
lol yes! im too dumb to see it all on the one line!!!!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
title!
similar issue reported in discord here a while back.
tl;dr -- that commit we snag the bridge address so ceos (or presumably others) can set gateway, buuuuuut if you are docker in docker (like me and my little weird launcher tool so i can run this on my mac) then the bridge is not mounted in the "launcher" container so trying to get that bridges address fails.
we already have the output of docker inspect network from earlier on in this func, so this just falls back to attempting to iterate over the ipam section to set v4/v6 addresses. if we still dont get any we just return the original error.
tested this in my dind setup (basically mount docker sock to debian container and launch clab from there but all in docker desktop on Mac) and it worked nicely whereas before it was bombing out due to the error returned from FirstLinkIPs.
lemme know if this makes sense or any changes are needed!