Skip to content

libnetwork: replace BurntSushi/toml with pelletier/go-toml#42465

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:no_sushi_today
Jun 5, 2021
Merged

libnetwork: replace BurntSushi/toml with pelletier/go-toml#42465
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:no_sushi_today

Conversation

@thaJeztah
Copy link
Member

replaces moby/libnetwork#2627
relates to #42246

Dependency will go away once #42249 is merged

The BurntSushi project is no longer maintained, and the container ecosystem is moving to use the pelletier/go-toml project instead.

This patch moves libnetwork to use the pelletier/go-toml library, to reduce our dependency tree and use the same library in all places.

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Jun 4, 2021
Copy link
Member

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member Author

let's wait until #42249 is merged, otherwise vendor check on master will fail (when it's merged, I'll revendor so that the dependency will be removed)

@samuelkarp
Copy link
Member

#42249 is merged so @thaJeztah this is ready to be rebased

The BurntSushi project is no longer maintained, and the container ecosystem
is moving to use the pelletier/go-toml project instead.

This patch moves libnetwork to use the pelletier/go-toml library, to reduce
our dependency tree and use the same library in all places.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

Arf.. looks like BuildKit also still uses it, so looks like I need to open a PR there to replace it;

A bit odd though that we're importing from a cmd feels like some more refactoring there would be good.

@thaJeztah
Copy link
Member Author

Anyway; rebased; 🤞 hopefully not hitting a flaky test this time

@thaJeztah thaJeztah added this to the 21.xx milestone Jun 4, 2021
@thaJeztah
Copy link
Member Author

Arm64 failing on the flaky libnetwork test; #42459

=== RUN   TestNetworkDBIslands
networkdb_test.go:864: timeout hit after 2m0s: node5:Waiting for all nodes to cleanly leave, left: 2, failed nodes: 1

@thaJeztah
Copy link
Member Author

Windows now failing on the same test 😞. Kicking CI again

@thaJeztah
Copy link
Member Author

Failure (again) on arm64: TestConcurrencyNoWait tracked through #42458

=== Failed
 === FAIL: libnetwork/iptables TestConcurrencyNoWait (0.10s)
     iptables_test.go:217:  (iptables failed: iptables -t nat -A POSTROUTING -p tcp -s 172.17.0.1 -d 172.17.0.1 --dport 4321 -j MASQUERADE: Another app is currently holding the xtables lock. Perhaps you want to use the -w option?
          (exit status 4))

@thaJeztah
Copy link
Member Author

This is gonna be playing "whack-a-mole"; if other platforms pass, I'll merge, as we'd have had a green run for all jobs (in different combinations), and they should be unrelated anyway

@thaJeztah
Copy link
Member Author

Everything else is green; bringing this one in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactor PR's that refactor, or clean-up code status/4-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants