Fix Catalog corruption issue when running CLI in parallel

[anujc25]

What this PR does / why we need it

• Fix the Catalog corruption issue when running CLI in parallel
• Implements NewContextCatalog (for the reading) and NewContextCatalogUpdater (for reading/writing) as separate APIs.
• Uses a new lockedfile API to lock the file. Inspired from: https://go.googlesource.com/proposal/+/master/design/33974-add-public-lockedfile-pkg.md

Which issue(s) this PR fixes

Fixes #471

Describe testing done for PR

• Wrote a bash script to manually verify the fix

#!/bin/bash

TANZU=tanzu
$TANZU plugin install --group vmware-tkg/default
for i in {1..10}
do
    for x in {1..50}
    do
        $TANZU version &
        echo "running: $x $!"        
    done
    echo "waiting"
    wait
    echo "done"    
done

$TANZU plugin list

• Also added E2E tests to run tanzu commands in parallel when 2 telemetry plugins are present

• Also added E2E tests to install multiple Plugins in parallel

• User can verify the E2E test by running tests with latest tanzu binary with the fix and the old `tanzu binary without the fix as below:

cd test/e2e/

export TANZU_CLI_E2E_TEST_BINARY_PATH=tanzu-old ## Without the fix
make e2e-catalog-tests. ## This should fail


export TANZU_CLI_E2E_TEST_BINARY_PATH=tanzu-new ## With the fix
make e2e-catalog-tests ## This should be successful

Release note

Fix the Catalog corruption issue (missing installed plugins) when running CLI in parallel

Additional information

Special notes for your reviewer

[marckhouzam]

This look very nice to me.
There are, I think, some important comments to address, but this is a very clean solution.
Thanks @anujc25 !

[vuil]

Do we have any e2e or integ tests (or doing multiple concurrent catalog reads/writes) that can help uncover the issue we are trying to fix here? Having them earlier would have help catch the issue, having them would have helped giving confidence that the issue is sufficiently addressed. Let's make sure we at least file a TODO to provide them.

[anujc25]

Do we have any e2e or integ tests (or doing multiple concurrent catalog reads/writes) that can help uncover the issue we are trying to fix here? Having them earlier would have help catch the issue, having them would have helped giving confidence that the issue is sufficiently addressed. Let's make sure we at least file a TODO to provide them.

I am planning to add e2e tests for this. I wanted to get high-level feedback on the approach before I deep dive into writing tests for this PR and hence a draft PR. If the approach looks good, I will add some e2e tests for this change.

[vuil]

Do we have any e2e or integ tests (or doing multiple concurrent catalog reads/writes) that can help uncover the issue we are trying to fix here? Having them earlier would have help catch the issue, having them would have helped giving confidence that the issue is sufficiently addressed. Let's make sure we at least file a TODO to provide them.

I am planning to add e2e tests for this. I wanted to get high-level feedback on the approach before I deep dive into writing tests for this PR and hence a draft PR. If the approach looks good, I will add some e2e tests for this change.

sgtm. The not-so-great part is an expectation that any valid unlock function returned has to be called by the caller, but the approach is a reasonable compromise.

[marckhouzam]

Nice effort. This is complicated stuff.
I haven't looked at the E2E tests yet because more concurrency questions came to my mind.

I'm wondering, in saveCatalogCache() do we need to use the lockedFile file descriptor to write to the file? I think doing that would make unit tests proper fail if Unlock() is called too early (which we had before but you corrected in your latest changes), because a call to Upsert() after the Unlock() would not be able to write to the closed file.

[marckhouzam]

LGTM
Thanks for this complicated and important change!

[vuil]

Thanks for the updates and improvements after the reviews.
There is a nit a typo in the comments, but changes lgtm.