Separate and generalize object-to-number allocator

In pillar we now have 3 number allocators with persistence
built on top of pubsub publication:
- appnumallocator.go
- bridgenumallocator.go
- appnumonunet.go

Most of the code of these allocators is in zedrouter, but some underlying
functions are separated to pillar/uuidtonum and pillar/uuidpairtonum and
partially also used from other microservices.

The main logic of these allocators is the same, only target objects
for which the numbers are allocated differ.

The main problems with the current implementation are:
- hard to understand, not easy to read, type names like UUIDPairAndIfIdxToNum
  are quite clumsy
- same logic replicated 3 times
- difficult/impossible to re-use elsewhere (a concept of number
  allocator could be useful outside of zedrouter)
- some hard-coded limits (e.g. allocation limited to 256 numbers max)
- difficult/impossible to customize or extend
- most of the code is tightly-coupled with the zedrouter

In this commit we generalize all these allocators into one
implementation, separated from the zedrouter and put into pillar/objtonum
package.
The allocator itself was split into 3 types:
- object-to-number map: allows to store number assignments (e.g. using pubsub
  for persistence)
- number allocator: allows to allocate number from some integer set
  (could be a range of numbers <0-255> or something else)
- object-to-number allocator: allocates number for an object and stores
  the allocation. Uses an object-to-number map and a number allocator
  (of the caller choice).

All changes were done in a backward-compatible way - nothing changed in how
these number allocations are persisted (neither content nor topic names changed,
but note that Go alias was used, learn about it here: https://yourbasic.org/golang/type-alias/),
This ensures that EVE upgrade will not be broken.

This commit is first in a series of the zedrouter refactoring.

Signed-off-by: Milan Lenco <milan@zededa.com>

pkg/pillar/.golangci.yml

@@ -30,6 +30,8 @@ linters:
     - nestif            # Too opinionated on existing code
     - exhaustruct       # Too opinionated on existing code
     - nosnakecase       # Uppercase with underscores is in some cases used for enum values
+    - nonamedreturns    # Named return is not a bad practice
+
 
 issues:
   max-issues-per-linter: 0

pkg/pillar/base/logobjecttypes.go

@@ -158,8 +158,8 @@ const (
 	MemoryNotificationType = "memory_notification"
 	// DiskNotificationType
 	DiskNotificationType = "disk_notification"
-	// UUIDPairToNumAndIfIdxLogType:
-	UUIDPairToNumAndIfIdxLogType LogObjectType = "uuid_pair_and_if_idx_to_num"
+	// AppInterfaceToNumLogType
+	AppInterfaceToNumLogType LogObjectType = "app_interface_to_num"
 	// EncryptedVaultKeyFromDeviceLogType:
 	EncryptedVaultKeyFromDeviceLogType LogObjectType = "encrypted_vault_key_from_device"
 	// EncryptedVaultKeyFromControllerLogType:

pkg/pillar/cmd/upgradeconverter/UUIDPairToNumConvert.go

@@ -42,17 +42,17 @@ func convertUUIDPairToNum(ctxPtr *ucContext) error {
 	}
 	defer pubUUIDPairToNum.Close()
 
-	pubUUIDPairAndIfIdxToNum, err := ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
+	pubAppInterfaceToNum, err := ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
 		AgentName:  "zedrouter",
 		Persistent: true,
-		TopicType:  types.UUIDPairAndIfIdxToNum{},
+		TopicType:  types.AppInterfaceToNum{},
 	})
 
 	if err != nil {
 		log.Error(err)
 		return err
 	}
-	defer pubUUIDPairAndIfIdxToNum.Close()
+	defer pubAppInterfaceToNum.Close()
 
 	items := pubUUIDPairToNum.GetAll()
 	if len(items) == 0 {
@@ -62,29 +62,32 @@ func convertUUIDPairToNum(ctxPtr *ucContext) error {
 
 	log.Tracef("UUIDPairToNum found %d", len(items))
 
-	// if we have UUIDPairToNum assume that we should remove old items and recreate them
-	olditems := pubUUIDPairAndIfIdxToNum.GetAll()
+	// if we have some AppInterfaceToNum publications assume that we should remove
+	// old items and recreate them.
+	olditems := pubAppInterfaceToNum.GetAll()
 	for key := range olditems {
-		err = pubUUIDPairAndIfIdxToNum.Unpublish(key)
+		err = pubAppInterfaceToNum.Unpublish(key)
 		if err != nil {
 			log.Error(err)
 		}
 	}
 
 	for _, item := range items {
 		uptn := item.(UUIDPairToNum)
-		upitn := types.UUIDPairAndIfIdxToNum{
-			BaseID:      uptn.BaseID,
-			AppID:       uptn.AppID,
+		appifnum := types.AppInterfaceToNum{
+			AppInterfaceKey: types.AppInterfaceKey{
+				NetInstID: uptn.BaseID,
+				AppID:     uptn.AppID,
+				IfIdx:     0,
+			},
 			Number:      uptn.Number,
 			NumType:     uptn.NumType,
 			CreateTime:  uptn.CreateTime,
 			LastUseTime: uptn.LastUseTime,
 			InUse:       uptn.InUse,
-			IfIdx:       0,
 		}
 		// publish new allocator
-		err = pubUUIDPairAndIfIdxToNum.Publish(upitn.Key(), upitn)
+		err = pubAppInterfaceToNum.Publish(appifnum.Key(), appifnum)
 		if err != nil {
 			log.Error(err)
 			continue

pkg/pillar/cmd/upgradeconverter/upgradeconverter.go

@@ -45,7 +45,7 @@ var preVaultconversionHandlers = []ConversionHandler{
 		handlerFunc: applyDefaultConfigItem,
 	},
 	{
-		description: "Move UUIDPairToNumKey to UUIDPairAndIfIdxToNumKey",
+		description: "Move UUIDPairToNum to AppInterfaceToNum",
 		handlerFunc: convertUUIDPairToNum,
 	},
 }

pkg/pillar/cmd/upgradeconverter/upgradeconverter_test.go

@@ -404,7 +404,8 @@ func Test_ApplyDefaultConfigItem(t *testing.T) {
 	}
 }
 
-// Test_ConvertUUIDPairToNum verifies conversion from UUIDPairToNum to UUIDPairAndIfIdxToNum
+// Test_ConvertUUIDPairToNum verifies conversion from UUIDPairToNum to AppInterfaceToNum
+// (previously called UUIDPairAndIfIdxToNum)
 func Test_ConvertUUIDPairToNum(t *testing.T) {
 	logrus.SetLevel(logrus.DebugLevel)
 	log = base.NewSourceLogObject(logrus.StandardLogger(), "test", 1234)
@@ -417,10 +418,10 @@ func Test_ConvertUUIDPairToNum(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	pubUUIDPairAndIfIdxToNum, err := ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
+	pubAppInterfaceToNum, err := ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
 		AgentName:  "zedrouter",
 		Persistent: true,
-		TopicType:  types.UUIDPairAndIfIdxToNum{},
+		TopicType:  types.AppInterfaceToNum{},
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -433,12 +434,13 @@ func Test_ConvertUUIDPairToNum(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	numType := "num-type1"
 	//it should be converted
 	uptn := UUIDPairToNum{
 		BaseID:      baseID,
 		AppID:       appID,
 		Number:      0,
-		NumType:     "",
+		NumType:     numType,
 		CreateTime:  time.Now(),
 		LastUseTime: time.Now(),
 		InUse:       false,
@@ -448,17 +450,19 @@ func Test_ConvertUUIDPairToNum(t *testing.T) {
 		t.Fatal(err)
 	}
 	// it should be removed
-	uptin := types.UUIDPairAndIfIdxToNum{
-		BaseID:      uuid.UUID{},
-		AppID:       uuid.UUID{},
+	appifnum := types.AppInterfaceToNum{
+		AppInterfaceKey: types.AppInterfaceKey{
+			NetInstID: uuid.UUID{},
+			AppID:     uuid.UUID{},
+			IfIdx:     0,
+		},
 		Number:      0,
 		NumType:     "",
 		CreateTime:  time.Now().Add(time.Hour),
 		LastUseTime: time.Now().Add(time.Hour),
 		InUse:       false,
-		IfIdx:       0,
 	}
-	err = pubUUIDPairAndIfIdxToNum.Publish(uptin.Key(), uptin)
+	err = pubAppInterfaceToNum.Publish(appifnum.Key(), appifnum)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -468,7 +472,7 @@ func Test_ConvertUUIDPairToNum(t *testing.T) {
 		t.Fatal(err)
 	}
 	// we are done with persist publishing, close publisher
-	err = pubUUIDPairAndIfIdxToNum.Close()
+	err = pubAppInterfaceToNum.Close()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -489,30 +493,31 @@ func Test_ConvertUUIDPairToNum(t *testing.T) {
 	}
 	// Verify that a second publish gets the data
 	// filled in.
-	pubUUIDPairAndIfIdxToNum, err = ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
+	pubAppInterfaceToNum, err = ctxPtr.ps.NewPublication(pubsub.PublicationOptions{
 		AgentName:  "zedrouter",
 		Persistent: true,
-		TopicType:  types.UUIDPairAndIfIdxToNum{},
+		TopicType:  types.AppInterfaceToNum{},
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	oldUUIDs := pubUUIDPairToNum.GetAll()
-	if len(oldUUIDs) > 0 {
-		t.Fatalf("unexpected UUIDPairToNum count (expected 0): %d", len(oldUUIDs))
+	oldMappings := pubUUIDPairToNum.GetAll()
+	if len(oldMappings) > 0 {
+		t.Fatalf("unexpected UUIDPairToNum count (expected 0): %d", len(oldMappings))
 	}
-	newUUIDs := pubUUIDPairAndIfIdxToNum.GetAll()
-	if len(newUUIDs) != 1 {
-		t.Fatalf("unexpected UUIDPairAndIfIdxToNum count (expected 1): %d", len(newUUIDs))
+	newMappings := pubAppInterfaceToNum.GetAll()
+	if len(newMappings) != 1 {
+		t.Fatalf("unexpected AppInterfaceToNum count (expected 1): %d", len(newMappings))
 	}
-	for _, v := range newUUIDs {
-		val, ok := v.(types.UUIDPairAndIfIdxToNum)
+	for _, v := range newMappings {
+		val, ok := v.(types.AppInterfaceToNum)
 		if !ok {
-			t.Fatal("cannot cast interface to UUIDPairAndIfIdxToNum")
+			t.Fatal("cannot cast interface to AppInterfaceToNum")
 		}
 		assert.Equal(t, val.AppID, appID)
-		assert.Equal(t, val.BaseID, baseID)
+		assert.Equal(t, val.NetInstID, baseID)
 		assert.Equal(t, val.IfIdx, uint32(0))
+		assert.Equal(t, val.NumType, numType)
 		if !val.CreateTime.Equal(uptn.CreateTime) {
 			t.Fatalf("CreateTime mismatch: %s vs %s", val.CreateTime, uptn.CreateTime)
 		}

pkg/pillar/cmd/zedagent/parseconfig.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	zconfig "github.com/lf-edge/eve/api/go/config"
+	"github.com/lf-edge/eve/pkg/pillar/objtonum"
 	"github.com/lf-edge/eve/pkg/pillar/sriov"
 	"github.com/lf-edge/eve/pkg/pillar/types"
 	fileutils "github.com/lf-edge/eve/pkg/pillar/utils/file"
@@ -2081,10 +2082,14 @@ func parseIpspec(ipspec *zconfig.Ipspec,
 			config.Gateway.String())
 	}
 	addressesInRange := config.DhcpRange.Size()
-	// we cannot use more than types.BitMapMax IPs for dynamic allocation
-	// and should keep some place in the end for static IPs assignment
-	if addressesInRange > types.BitMapMax {
-		config.DhcpRange.End = types.AddToIP(config.DhcpRange.Start, types.BitMapMax)
+	// Currently, we cannot use more than ByteAllocatorMaxNum IPs for dynamic allocation
+	// (i.e. 256 at maximum) and should keep some place in the end for static IPs
+	// assignment.
+	// XXX Later we could implement NumberAllocator with larger capacity (larger than
+	// what ByteAllocator can provide) and use it for network instances
+	// that require bigger subnets.
+	if addressesInRange > objtonum.ByteAllocatorMaxNum {
+		config.DhcpRange.End = types.AddToIP(config.DhcpRange.Start, objtonum.ByteAllocatorMaxNum)
 	}
 	return nil
 }

pkg/pillar/cmd/zedmanager/updatestatus.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/lf-edge/eve/pkg/pillar/types"
-	"github.com/lf-edge/eve/pkg/pillar/uuidtonum"
 	"github.com/satori/go.uuid"
 )
 
@@ -764,10 +763,13 @@ func purgeCmdDone(ctx *zedmanagerContext, config types.AppInstanceConfig,
 		config.Key(), len(status.VolumeRefStatusList), len(newVrs))
 	status.VolumeRefStatusList = newVrs
 	// Update persistent counter
-	uuidtonum.UuidToNumAllocate(log, ctx.pubUuidToNum,
-		status.UUIDandVersion.UUID,
-		int(config.PurgeCmd.Counter+config.LocalPurgeCmd.Counter),
-		false, "purgeCmdCounter")
+	mapKey := types.UuidToNumKey{UUID: config.UUIDandVersion.UUID}
+	purgeCounter := int(config.PurgeCmd.Counter + config.LocalPurgeCmd.Counter)
+	err := ctx.appToPurgeCounterMap.Assign(mapKey, purgeCounter, false)
+	if err != nil {
+		log.Errorf("Failed to update persisted purge counter for app %s-%s: %v",
+			config.DisplayName, config.UUIDandVersion.UUID, err)
+	}
 	return changed
 }
 

pkg/pillar/cmd/zedmanager/zedmanager.go

@@ -17,11 +17,11 @@ import (
 	"github.com/lf-edge/eve/pkg/pillar/agentlog"
 	"github.com/lf-edge/eve/pkg/pillar/base"
 	"github.com/lf-edge/eve/pkg/pillar/flextimer"
+	"github.com/lf-edge/eve/pkg/pillar/objtonum"
 	"github.com/lf-edge/eve/pkg/pillar/pidfile"
 	"github.com/lf-edge/eve/pkg/pillar/pubsub"
 	"github.com/lf-edge/eve/pkg/pillar/types"
 	"github.com/lf-edge/eve/pkg/pillar/utils"
-	"github.com/lf-edge/eve/pkg/pillar/uuidtonum"
 	"github.com/sirupsen/logrus"
 )
 
@@ -52,7 +52,7 @@ type zedmanagerContext struct {
 	subHostMemory         pubsub.Subscription
 	subZedAgentStatus     pubsub.Subscription
 	globalConfig          *types.ConfigItemValueMap
-	pubUuidToNum          pubsub.Publication
+	appToPurgeCounterMap  objtonum.Map
 	GCInitialized         bool
 	checkFreedResources   bool // Set when app instance has !Activated
 	currentProfile        string
@@ -152,16 +152,14 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar
 	ctx.pubDomainConfig = pubDomainConfig
 	pubDomainConfig.ClearRestarted()
 
-	pubUuidToNum, err := ps.NewPublication(pubsub.PublicationOptions{
-		AgentName:  agentName,
-		Persistent: true,
-		TopicType:  types.UuidToNum{},
-	})
+	// Persist purge counter for each application.
+	mapPublisher, err := objtonum.NewObjNumPublisher(
+		ps, agentName, true, &types.UuidToNum{})
 	if err != nil {
 		log.Fatal(err)
 	}
-	ctx.pubUuidToNum = pubUuidToNum
-	pubUuidToNum.ClearRestarted()
+	ctx.appToPurgeCounterMap = objtonum.NewPublishedMap(
+		log, mapPublisher, "purgeCmdCounter", objtonum.AllKeys)
 
 	// Look for global config such as log levels
 	subGlobalConfig, err := ps.NewSubscription(pubsub.SubscriptionOptions{
@@ -597,8 +595,8 @@ func handleCreate(ctxArg interface{}, key string,
 
 	// Do we have a PurgeCmd counter from before the reboot?
 	// Note that purgeCmdCounter is a sum of the remote and the local purge counter.
-	persistedCounter, err := uuidtonum.UuidToNumGet(log, ctx.pubUuidToNum,
-		config.UUIDandVersion.UUID, "purgeCmdCounter")
+	mapKey := types.UuidToNumKey{UUID: config.UUIDandVersion.UUID}
+	persistedCounter, _, err := ctx.appToPurgeCounterMap.Get(mapKey)
 	configCounter := int(config.PurgeCmd.Counter + config.LocalPurgeCmd.Counter)
 	if err == nil {
 		if persistedCounter == configCounter {
@@ -617,9 +615,11 @@ func handleCreate(ctxArg interface{}, key string,
 		// Save this PurgeCmd.Counter as the baseline
 		log.Functionf("handleCreate(%v) for %s saving purge counter %d",
 			config.UUIDandVersion, config.DisplayName, configCounter)
-		uuidtonum.UuidToNumAllocate(log, ctx.pubUuidToNum,
-			config.UUIDandVersion.UUID, configCounter,
-			true, "purgeCmdCounter")
+		err = ctx.appToPurgeCounterMap.Assign(mapKey, configCounter, true)
+		if err != nil {
+			log.Errorf("Failed to persist purge counter for app %s-%s: %v",
+				config.DisplayName, config.UUIDandVersion.UUID, err)
+		}
 	}
 
 	status.VolumeRefStatusList = make([]types.VolumeRefStatus,
@@ -784,7 +784,12 @@ func handleDelete(ctx *zedmanagerContext, key string,
 
 	removeAIStatus(ctx, status)
 	// Remove the recorded PurgeCmd Counter
-	uuidtonum.UuidToNumDelete(log, ctx.pubUuidToNum, status.UUIDandVersion.UUID)
+	mapKey := types.UuidToNumKey{UUID: status.UUIDandVersion.UUID}
+	err := ctx.appToPurgeCounterMap.Delete(mapKey, false)
+	if err != nil {
+		log.Warnf("Failed to delete persisted purge counter for app %s-%s: %v",
+			status.DisplayName, status.UUIDandVersion.UUID, err)
+	}
 	log.Functionf("handleDelete done for %s", status.DisplayName)
 }