Paolo Abeni <pab...@redhat.com> writes:
> On Wed, 2023-12-06 at 13:32 +0100, Petr Machata wrote:
>> Paolo Abeni <pab...@redhat.com> writes:
>>
>> > Side note for a possible follow-up: if you maintain $ns_list as global
>> > variable, and remove from such list the ns deleted by cleanup_ns, you
>> > could remove the cleanup trap from the individual test with something
>> > alike:
>> >
>> > final_cleanup_ns()
>> > {
>> > cleanup_ns $ns_list
>> > }
>> >
>> > trap final_cleanup_ns EXIT
>> >
>> > No respin needed for the above, could be a follow-up if agreed upon.
>>
>> If you propose this for the library then I'm against it. The exit trap
>> is a global resource that the client scripts sometimes need to use as
>> well, to do topology teardowns or just general cleanups.
>> So either the library would have to provide APIs for cleanup management, or
>> the trap
>> is for exclusive use by clients. The latter is IMHO simpler.
>
> Even the former would not be very complex:
>
> TRAPS=""
> do_at_exit() {
> TRAPS="${TRAPS}$@;"
>
> trap "${TRAPS}" EXIT
> }
>
> And then use "do_at_exit <whatever>" instead of "trap <whatever> EXIT"
Yep. I mentioned this during v2 review:
https://github.com/pmachata/stuff/blob/master/ptp-test/lib.sh#L13
Not much code at all, though you need to convert all EXIT trap users to
this contraption. Again, a mechanical process, just needs to be done.
>> It also puts the cleanups at the same place where the acquisition is
>> prompted: the client allocates the NS, the client should prompt its
>> cleanup.
>
> I guess I could argue that the the script is asking the library to
> allocate the namespaces, and the library could take care of disposing
> them.
It could also be said that since the script asked for NS creation, the
script should ask for NS disposal :)
But what I object against is that the library uses trap without having a
way for user scripts to schedule at-exit work, because that's used
literally everywhere in forwarding tests. If people are willing to do
the conversion, I'm OK with that.
> But I'm not pushing the proposed option, if there is no agreement no
> need for additional work ;)
>
> Cheers,
>
> Paolo
