api: fix OCI hook ownership tracking.

Fix hook ownership tracking and add a proper test case for it.

Ownership tracking for OCI hooks is supposed to be accumulative.
The implementation tried to accumulate owners, but it was buggy.
It tried to delete existing ownership by unclaiming which marks
ownershipdeleted by the plugin instead of clearing it.

This went unnoticed since we lacked any kind of proper test for
hook ownership accumulation. It was only noticed thanks to containerd#263
which switches much of the ownership tracking code to generated
from hand-written.

Signed-off-by: Krisztian Litkey <[email protected]>

Author: klihub

pkg/api/owners.go

@@ -475,14 +475,7 @@ func (f *FieldOwners) ClaimMount(destination, plugin string) error {
 }
 
 func (f *FieldOwners) ClaimHooks(plugin string) error {
-	plugins := plugin
-
-	if current, ok := f.simpleOwner(Field_OciHooks.Key()); ok {
-		f.clearSimple(Field_OciHooks.Key(), plugin)
-		plugins = current + "," + plugin
-	}
-
-	f.claimSimple(Field_OciHooks.Key(), plugins)
+	f.accumulateSimple(Field_OciHooks.Key(), plugin)
 	return nil
 }
 
@@ -678,6 +671,14 @@ func (f *FieldOwners) ClearRdt(plugin string) {
 	f.clearSimple(Field_RdtEnableMonitoring.Key(), plugin)
 }
 
+func (f *FieldOwners) accumulateSimple(field int32, plugin string) {
+	old, ok := f.simpleOwner(field)
+	if ok {
+		plugin = old + "," + plugin
+	}
+	f.Simple[field] = plugin
+}
+
 func (f *FieldOwners) Conflict(field int32, plugin, other string, qualifiers ...string) error {
 	return fmt.Errorf("plugins %q and %q both tried to set %s",
 		plugin, other, qualify(field, qualifiers...))

pkg/api/owners_test.go

@@ -113,3 +113,24 @@ func TestCompoundClaims(t *testing.T) {
 
 	require.Equal(t, api.Field_Annotations.String(), "Annotations", "annotation field name")
 }
+
+func TestAccumulatingOwnership(t *testing.T) {
+	o := api.NewOwningPlugins()
+
+	// claim OCI hooks of ctr0
+	err := o.ClaimHooks("ctr0", "plugin0")
+	require.NoError(t, err, "ctr0 OCI hooks by plugin0")
+
+	// claim OCI hooks of ctr0
+	err = o.ClaimHooks("ctr0", "plugin1")
+	require.NoError(t, err, "ctr0 OCI hooks by plugin1")
+
+	// claim OCI hooks of ctr0
+	err = o.ClaimHooks("ctr0", "plugin2")
+	require.NoError(t, err, "ctr0 OCI hooks by plugin2")
+
+	owners, ok := o.HooksOwner("ctr0")
+	require.True(t, ok, "ctr0 has hooks owners")
+	require.Equal(t, "plugin0,plugin1,plugin2", owners, "ctr0 hooks owners")
+
+}