balena-engine icon indicating copy to clipboard operation
balena-engine copied to clipboard

Published 20 hours ago • verified publisher icon balena-os

Update netlink

Open lmbarros opened this issue 2 years ago • 1 comments

This new version of netlink includes a number of bugfixes, including a fix to #292.

This PR needs testing (it actually uses a beta version of netlink). An alternative would be to cherry peek the commits we are most interested in.

lmbarros avatar Apr 11 '22 21:04 lmbarros

Your landr site preview has been successfully deployed to https://landr-balena-os-repo-balena-engine-preview-294.netlify.app

Deployed with Landr 9.4.14

ghost avatar Apr 11 '22 21:04 ghost

@lmbarros could we cherry-pick https://github.com/vishvananda/netlink/pull/665 for the time being?

alexgg avatar Dec 13 '22 10:12 alexgg

@alexgg I see I mentioned cherry picking myself, but that's not easily done. netlink is a dependency, not code we own. (We do keep a vendored copy of netlink within our repo -- changing it directly could technically work, but would be really bad practice and asking for trouble in the future.)

lmbarros avatar Dec 13 '22 18:12 lmbarros

We would need to fork netlink, cherry pick into our fork and point the dependency to the fork. It's doable, but I don't know... doesn't appear to be much simpler than pointing to the beta netlink and going through a round of Engine tests.

lmbarros avatar Dec 13 '22 18:12 lmbarros

hey @lmbarros using a beta version of netlink feels like looking for trouble - if it was production ready it wouldn't be beta. I understand forking a dependency is cumbersome, but once netlink releases a stable version we can just drop it. And having the engine crash when using primary cellular connectivity is something we need to address. Do you want to take it to an arch call maybe to discuss pros/cons?

alexgg avatar Dec 14 '22 09:12 alexgg

@alexgg , not saying this is the right approach, but in their next-release branch Moby is already using the beta netlink for some time.

lmbarros avatar Dec 14 '22 11:12 lmbarros

@alexgg , not saying this is the right approach, but in their next-release branch Moby is already using the beta netlink for some time.

@lmbarros but we also wouldn't use Moby's next release in production, would we?

alexgg avatar Dec 14 '22 13:12 alexgg

Hey @alexgg ! This is the netlib update (with the fix cherry-picked into a temporary netlib fork, as we agreed).

I slipped in a second commit fixing an integration test failure introduced by the migration to Flowzone.

lmbarros avatar Dec 20 '22 21:12 lmbarros