ErrMapIncompatible should contain additional information

[ti-mo]

In cilium/cilium#22693, Cilium will start relying exclusively on the library to tell it when a map is incompatible with an existing pinned map, but right now ErrMapIncompatible is a little vague. Callers need to open the old map explicitly, inspect its info, and work out the differences with the MapSpec.

---

Updated to echo #1093 (comment).

I think there are 2 issues with the existing code:

• No clear communication (towards the user) of what 'expected' and 'got' means in (expected type %v, got %v). We should settle on language that clarifies which side is the existing Map and which is the incoming Spec.
• Only one error is surfaced at a time, being the first-found one.

As a bonus, it would be nice if this would return a new ebpf.IncompatibleMapError{OldType, NewType ebpf.MapType, ...} that .Unwrap()s to ErrMapIncompatible so it can be used like:

var merr *ebpf.IncompatibleMapError
if errors.As(err, &merr) {
  if merr.OldType == ebpf.ArrayOfMaps && merr.NewType == ebpf.HashOfMaps {
    // Convert the old map from ArrayOfMaps to HashOfMaps here..
  }
}

This would be really useful to trigger up/downgrade logic.

Also, .Error() should only return a diff of the map's properties, not a dump of all old/new properties. That's just something I hacked up in Cilium to move ahead with the changes there.

[swastik959]

@ti-mo hi I am new to this org and want to contribute to this issue can you please guide me through it

[ti-mo]

@swastik959 There's a short document at https://github.com/cilium/ebpf/blob/master/CONTRIBUTING.md. Do you have any specific questions?

[sharathsivakumar]

Hello @ti-mo, I would like to work on this if no one is actively working on this.

[ti-mo]

@sharathsivakumar Sure thing! Assigned the issue to you.

[sharathsivakumar]

Thanks!

[lmb]

Closing this for now, since it's actually quite tricky to implement. This should happen if / when we use the info from the cilium side.