Separate and generalize object-to-number allocator

pkg/pillar/.golangci.yml

@@ -30,6 +30,9 @@ 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
+    - cyclop            # Raises warnings even for relatively simple functions
+    - ireturn           # Returning interfaces is a common practise for constructors in defensive programming
 
 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(
+		log, 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)
 }