VOL-4215 : Rearrange tech-profile delete to trigger ani config deletion by Gem deletion

Change-Id: I90c8e7812e59e2c617355208bf353d4a40468eb4
diff --git a/internal/pkg/onuadaptercore/device_handler.go b/internal/pkg/onuadaptercore/device_handler.go
index 1cf414a..edb44ce 100644
--- a/internal/pkg/onuadaptercore/device_handler.go
+++ b/internal/pkg/onuadaptercore/device_handler.go
@@ -473,13 +473,6 @@
 	ctx context.Context,
 	msg *ic.InterAdapterMessage) error {
 
-	logger.Infow(ctx, "delete-gem-port-request", log.Fields{"device-id": dh.deviceID})
-
-	pDevEntry := dh.getOnuDeviceEntry(ctx, true)
-	if pDevEntry == nil {
-		logger.Errorw(ctx, "No valid OnuDevice - aborting", log.Fields{"device-id": dh.deviceID})
-		return fmt.Errorf("no valid OnuDevice: %s", dh.deviceID)
-	}
 	if dh.pOnuTP == nil {
 		//should normally not happen ...
 		logger.Warnw(ctx, "onuTechProf instance not set up for DelGem request - ignoring request",
@@ -509,30 +502,18 @@
 		logger.Errorw(ctx, "error-extracting-tp-id-from-tp-path", log.Fields{"err": err, "tp-path": delGemPortMsg.TpInstancePath})
 		return err
 	}
-
+	logger.Infow(ctx, "delete-gem-port-request", log.Fields{"device-id": dh.deviceID, "uni-id": uniID, "tpID": tpID, "gem": delGemPortMsg.GemPortId})
 	//a removal of some GemPort would never remove the complete TechProfile entry (done on T-Cont)
 
-	// deadline context to ensure completion of background routines waited for
-	deadline := time.Now().Add(dh.pOpenOnuAc.maxTimeoutInterAdapterComm) //allowed run time to finish before execution
-	dctx, cancel := context.WithDeadline(context.Background(), deadline)
+	return dh.deleteTechProfileResource(ctx, uniID, tpID, delGemPortMsg.TpInstancePath,
+		cResourceGemPort, delGemPortMsg.GemPortId)
 
-	dh.pOnuTP.resetTpProcessingErrorIndication(uniID, tpID)
-
-	var wg sync.WaitGroup
-	wg.Add(1) // for the 1 go routine to finish
-	go dh.pOnuTP.deleteTpResource(log.WithSpanFromContext(dctx, ctx), uniID, tpID, delGemPortMsg.TpInstancePath,
-		cResourceGemPort, delGemPortMsg.GemPortId, &wg)
-	dh.waitForCompletion(ctx, cancel, &wg, "GemDelete") //wait for background process to finish
-
-	return dh.pOnuTP.getTpProcessingErrorIndication(uniID, tpID)
 }
 
 func (dh *deviceHandler) processInterAdapterDeleteTcontReqMessage(
 	ctx context.Context,
 	msg *ic.InterAdapterMessage) error {
 
-	logger.Infow(ctx, "delete-tcont-request", log.Fields{"device-id": dh.deviceID})
-
 	pDevEntry := dh.getOnuDeviceEntry(ctx, true)
 	if pDevEntry == nil {
 		logger.Errorw(ctx, "No valid OnuDevice - aborting", log.Fields{"device-id": dh.deviceID})
@@ -568,26 +549,64 @@
 		logger.Errorw(ctx, "error-extracting-tp-id-from-tp-path", log.Fields{"err": err, "tp-path": tpPath})
 		return err
 	}
+	logger.Infow(ctx, "delete-tcont-request", log.Fields{"device-id": dh.deviceID, "uni-id": uniID, "tpID": tpID, "tcont": delTcontMsg.AllocId})
 
-	if bTpModify := pDevEntry.updateOnuUniTpPath(ctx, uniID, tpID, ""); bTpModify {
-		pDevEntry.freeTcont(ctx, uint16(delTcontMsg.AllocId))
-		// deadline context to ensure completion of background routines waited for
-		deadline := time.Now().Add(dh.pOpenOnuAc.maxTimeoutInterAdapterComm) //allowed run time to finish before execution
-		dctx, cancel := context.WithDeadline(context.Background(), deadline)
+	pDevEntry.freeTcont(ctx, uint16(delTcontMsg.AllocId))
 
-		dh.pOnuTP.resetTpProcessingErrorIndication(uniID, tpID)
-		pDevEntry.resetKvProcessingErrorIndication()
+	return dh.deleteTechProfileResource(ctx, uniID, tpID, delTcontMsg.TpInstancePath,
+		cResourceTcont, delTcontMsg.AllocId)
 
-		var wg sync.WaitGroup
-		wg.Add(2) // for the 2 go routines to finish
-		go dh.pOnuTP.deleteTpResource(log.WithSpanFromContext(dctx, ctx), uniID, tpID, delTcontMsg.TpInstancePath,
-			cResourceTcont, delTcontMsg.AllocId, &wg)
-		// Removal of the tcont/alloc id mapping represents the removal of the tech profile
-		go pDevEntry.updateOnuKvStore(log.WithSpanFromContext(dctx, ctx), &wg)
-		dh.waitForCompletion(ctx, cancel, &wg, "TContDelete") //wait for background process to finish
+}
 
-		return dh.combineErrorStrings(dh.pOnuTP.getTpProcessingErrorIndication(uniID, tpID), pDevEntry.getKvProcessingErrorIndication())
+func (dh *deviceHandler) deleteTechProfileResource(ctx context.Context,
+	uniID uint8, tpID uint8, pathString string, resource resourceEntry, entryID uint32) error {
+	pDevEntry := dh.getOnuDeviceEntry(ctx, true)
+	if pDevEntry == nil {
+		logger.Errorw(ctx, "No valid OnuDevice - aborting", log.Fields{"device-id": dh.deviceID})
+		return fmt.Errorf("no valid OnuDevice: %s", dh.deviceID)
 	}
+	var resourceName string
+	if cResourceGemPort == resource {
+		resourceName = "Gem"
+	} else {
+		resourceName = "Tcont"
+	}
+
+	// deadline context to ensure completion of background routines waited for
+	deadline := time.Now().Add(dh.pOpenOnuAc.maxTimeoutInterAdapterComm) //allowed run time to finish before execution
+	dctx, cancel := context.WithDeadline(context.Background(), deadline)
+
+	dh.pOnuTP.resetTpProcessingErrorIndication(uniID, tpID)
+
+	var wg sync.WaitGroup
+	wg.Add(1) // for the 1 go routine to finish
+	go dh.pOnuTP.deleteTpResource(log.WithSpanFromContext(dctx, ctx), uniID, tpID, pathString,
+		resource, entryID, &wg)
+	dh.waitForCompletion(ctx, cancel, &wg, resourceName+"Delete") //wait for background process to finish
+	if err := dh.pOnuTP.getTpProcessingErrorIndication(uniID, tpID); err != nil {
+		logger.Errorw(ctx, err.Error(), log.Fields{"device-id": dh.deviceID})
+		return err
+	}
+
+	if dh.pOnuTP.isTechProfileConfigCleared(ctx, uniID, tpID) {
+		logger.Debugw(ctx, "techProfile-config-cleared", log.Fields{"device-id": dh.deviceID, "uni-id": uniID, "tpID": tpID})
+		if bTpModify := pDevEntry.updateOnuUniTpPath(ctx, uniID, tpID, ""); bTpModify {
+			pDevEntry.resetKvProcessingErrorIndication()
+			var wg2 sync.WaitGroup
+			dctx2, cancel2 := context.WithDeadline(context.Background(), deadline)
+			wg2.Add(1)
+			// Removal of the gem id mapping represents the removal of the tech profile
+			logger.Infow(ctx, "remove-techProfile-indication-in-kv", log.Fields{"device-id": dh.deviceID, "uni-id": uniID, "tpID": tpID})
+			go pDevEntry.updateOnuKvStore(log.WithSpanFromContext(dctx2, ctx), &wg2)
+			dh.waitForCompletion(ctx, cancel2, &wg2, "TechProfileDeleteOn"+resourceName) //wait for background process to finish
+			if err := pDevEntry.getKvProcessingErrorIndication(); err != nil {
+				logger.Errorw(ctx, err.Error(), log.Fields{"device-id": dh.deviceID})
+				return err
+			}
+		}
+	}
+	logger.Debugw(ctx, "delete-tech-profile-resource-completed", log.Fields{"device-id": dh.deviceID,
+		"uni-id": uniID, "tpID": tpID, "resource-type": resourceName, "resource-id": entryID})
 	return nil
 }
 
@@ -3283,19 +3302,6 @@
 	return dh.startWritingOnuDataToKvStore(ctx, pDevEntry)
 }
 
-func (dh *deviceHandler) combineErrorStrings(errS ...error) error {
-	var errStr string = ""
-	for _, err := range errS {
-		if err != nil {
-			errStr = errStr + err.Error() + " "
-		}
-	}
-	if errStr != "" {
-		return fmt.Errorf("%s: %s", errStr, dh.deviceID)
-	}
-	return nil
-}
-
 // getUniPortMEEntityID takes uniPortNo as the input and returns the Entity ID corresponding to this UNI-G ME Instance
 // nolint: unused
 func (dh *deviceHandler) getUniPortMEEntityID(uniPortNo uint32) (uint16, error) {
diff --git a/internal/pkg/onuadaptercore/omci_ani_config.go b/internal/pkg/onuadaptercore/omci_ani_config.go
index 3985f00..f617f41 100644
--- a/internal/pkg/onuadaptercore/omci_ani_config.go
+++ b/internal/pkg/onuadaptercore/omci_ani_config.go
@@ -62,6 +62,7 @@
 	aniEvReset             = "aniEvReset"
 	aniEvRestart           = "aniEvRestart"
 	aniEvSkipOmciConfig    = "aniEvSkipOmciConfig"
+	aniEvRemGemDone        = "aniEvRemGemDone"
 )
 const (
 	// states of config PON ANI port FSM
@@ -185,13 +186,14 @@
 			{Name: aniEvFlowRemDone, Src: []string{aniStWaitingFlowRem}, Dst: aniStRemovingGemIW},
 			{Name: aniEvRxRemGemiwResp, Src: []string{aniStRemovingGemIW}, Dst: aniStRemovingGemNCTP},
 			{Name: aniEvRxRemGemntpResp, Src: []string{aniStRemovingGemNCTP}, Dst: aniStRemovingTD},
-			{Name: aniEvRxRemTdResp, Src: []string{aniStRemovingTD}, Dst: aniStConfigDone},
+			{Name: aniEvRxRemTdResp, Src: []string{aniStRemovingTD}, Dst: aniStRemDot1PMapper},
+			{Name: aniEvRemGemDone, Src: []string{aniStRemDot1PMapper}, Dst: aniStConfigDone},
+			{Name: aniEvRxRem1pMapperResp, Src: []string{aniStRemDot1PMapper}, Dst: aniStRemAniBPCD},
+			{Name: aniEvRxRemAniBPCDResp, Src: []string{aniStRemAniBPCD}, Dst: aniStRemoveDone},
 
 			//for removing TCONT related resources
 			{Name: aniEvRemTcontPath, Src: []string{aniStConfigDone}, Dst: aniStResetTcont},
-			{Name: aniEvRxResetTcontResp, Src: []string{aniStResetTcont}, Dst: aniStRemDot1PMapper},
-			{Name: aniEvRxRem1pMapperResp, Src: []string{aniStRemDot1PMapper}, Dst: aniStRemAniBPCD},
-			{Name: aniEvRxRemAniBPCDResp, Src: []string{aniStRemAniBPCD}, Dst: aniStRemoveDone},
+			{Name: aniEvRxResetTcontResp, Src: []string{aniStResetTcont}, Dst: aniStConfigDone},
 
 			{Name: aniEvTimeoutSimple, Src: []string{aniStCreatingDot1PMapper, aniStCreatingMBPCD, aniStSettingTconts, aniStSettingDot1PMapper,
 				aniStRemovingGemIW, aniStRemovingGemNCTP, aniStRemovingTD,
@@ -998,6 +1000,29 @@
 func (oFsm *uniPonAniConfigFsm) enterRemoving1pMapper(ctx context.Context, e *fsm.Event) {
 	logger.Debugw(ctx, "uniPonAniConfigFsm - start deleting the .1pMapper", log.Fields{
 		"device-id": oFsm.deviceID, "uni-id": oFsm.pOnuUniPort.uniID})
+	mapGemPortParams := oFsm.pUniTechProf.mapPonAniConfig[oFsm.uniTpKey].mapGemPortParams
+	unicastGemCount := 0
+	for _, gemEntry := range mapGemPortParams {
+		if !gemEntry.isMulticast {
+			unicastGemCount++
+		}
+	}
+	if unicastGemCount > 1 {
+		logger.Debugw(ctx, "uniPonAniConfigFsm - Not the last gem in fsm. Skip the rest", log.Fields{
+			"device-id": oFsm.deviceID, "uni-id": oFsm.pOnuUniPort.uniID, "unicast-gem-count": unicastGemCount, "gem-count": len(mapGemPortParams)})
+		pConfigAniStateAFsm := oFsm.pAdaptFsm
+		if pConfigAniStateAFsm != nil {
+			// obviously calling some FSM event here directly does not work - so trying to decouple it ...
+			go func(aPAFsm *AdapterFsm) {
+				if aPAFsm != nil && aPAFsm.pFsm != nil {
+					_ = aPAFsm.pFsm.Event(aniEvRemGemDone)
+				}
+			}(pConfigAniStateAFsm)
+			return
+		}
+	}
+	logger.Debugw(ctx, "uniPonAniConfigFsm - Last gem in fsm. Continue with Mapper removal", log.Fields{
+		"device-id": oFsm.deviceID, "uni-id": oFsm.pOnuUniPort.uniID, "unicast-gem-count": unicastGemCount, "gem-count": len(mapGemPortParams)})
 
 	oFsm.mutexPLastTxMeInstance.Lock()
 	meInstance, err := oFsm.pOmciCC.sendDeleteDot1PMapper(log.WithSpanFromContext(context.TODO(), ctx), oFsm.pDeviceHandler.pOpenOnuAc.omciTimeout, true,
diff --git a/internal/pkg/onuadaptercore/onu_uni_tp.go b/internal/pkg/onuadaptercore/onu_uni_tp.go
index 7d05a37..6e365ab 100644
--- a/internal/pkg/onuadaptercore/onu_uni_tp.go
+++ b/internal/pkg/onuadaptercore/onu_uni_tp.go
@@ -20,11 +20,12 @@
 import (
 	"context"
 	"fmt"
-	"github.com/opencord/voltha-protos/v4/go/tech_profile"
 	"strconv"
 	"strings"
 	"sync"
 
+	"github.com/opencord/voltha-protos/v4/go/tech_profile"
+
 	"github.com/opencord/voltha-lib-go/v5/pkg/log"
 )
 
@@ -551,7 +552,6 @@
 		"device-id": onuTP.deviceID, "uni-id": aUniID, "path": aPathString, "Resource": aResource})
 	uniTPKey := uniTP{uniID: aUniID, tpID: aTpID}
 
-	bDeviceProcStatusUpdate := true
 	if cResourceGemPort == aResource {
 		logger.Debugw(ctx, "remove GemPort from the list of existing ones of the TP", log.Fields{
 			"device-id": onuTP.deviceID, "uni-id": aUniID, "path": aPathString, "GemPort": aEntryID})
@@ -669,18 +669,18 @@
 			//as a consequence a possible remove-flow does not see any dependency on the TechProfile anymore and is executed (pro forma) directly
 			//a later TechProfile removal would cause the device-reason to be updated to 'techProfile-delete-success' which is not the expected state
 			// and anyway is no real useful information at that stage
-			bDeviceProcStatusUpdate = false
 			logger.Debugw(ctx, "uniPonAniConfigFsm delete Gem on OMCI skipped based on device state", log.Fields{
 				"device-id": onuTP.deviceID, "device-state": onuTP.baseDeviceHandler.getDeviceReasonString()})
 		}
 		// remove GemPort from config DB
 		//ensure write protection for access to mapPonAniConfig
+		logger.Debugw(ctx, "uniPonAniConfigFsm removing gem from config data and clearing ani FSM", log.Fields{
+			"device-id": onuTP.deviceID, "gem-id": onuTP.mapRemoveGemEntry[uniTPKey].removeGemID, "uniTPKey": uniTPKey})
 		onuTP.mutexTPState.Lock()
 		delete(onuTP.mapPonAniConfig[uniTPKey].mapGemPortParams, onuTP.mapRemoveGemEntry[uniTPKey].removeGemID)
 		// remove the removeEntry
 		delete(onuTP.mapRemoveGemEntry, uniTPKey)
 		onuTP.mutexTPState.Unlock()
-		//  deviceHandler StatusEvent (reason update) (see end of function) is only generated in case some element really was removed
 	} else { //if cResourceTcont == aResource {
 		logger.Debugw(ctx, "reset TCont with AllocId", log.Fields{
 			"device-id": onuTP.deviceID, "uni-id": aUniID, "path": aPathString, "allocId": aEntryID})
@@ -718,8 +718,6 @@
 			onuTP.procResult[uniTpKey] = fmt.Errorf("TCont cleanup aborted: no AniConfigFsm available %d on %s",
 				aUniID, onuTP.deviceID)
 			*/
-			//if the FSM is not valid, also TP related data should not be valid - clear the internal store profile data
-			onuTP.clearAniSideConfig(ctx, aUniID, aTpID)
 			return
 		}
 		if _, ok := onuTP.pAniConfigFsm[uniTPKey]; !ok {
@@ -728,7 +726,6 @@
 			//even if the FSM invocation did not work we don't indicate a problem within procResult
 			//errors could exist also because there was nothing to delete - so we just accept that as 'deleted'
 			//if the FSM is not valid, also TP related data should not be valid - clear the internal store profile data
-			onuTP.clearAniSideConfig(ctx, aUniID, aTpID)
 			return
 		}
 		if onuTP.baseDeviceHandler.isReadyForOmciConfig() {
@@ -766,20 +763,36 @@
 			}
 		} else {
 			//see gemPort comments
-			bDeviceProcStatusUpdate = false
 			logger.Debugw(ctx, "uniPonAniConfigFsm TCont cleanup on OMCI skipped based on device state", log.Fields{
 				"device-id": onuTP.deviceID, "device-state": onuTP.baseDeviceHandler.getDeviceReasonString()})
 		}
-		//clear the internal store profile data
-		onuTP.clearAniSideConfig(ctx, aUniID, aTpID)
-		// reset also the FSM in order to admit a new OMCI configuration in case a new profile is created
-		// FSM stop maybe encapsulated as OnuTP method - perhaps later in context of module splitting
-		_ = onuTP.pAniConfigFsm[uniTPKey].pAdaptFsm.pFsm.Event(aniEvReset)
 	}
-	if bDeviceProcStatusUpdate {
-		// generate deviceHandler StatusEvent in case the FSM was not invoked and OMCI processing not locked due to device state
-		go onuTP.baseDeviceHandler.deviceProcStatusUpdate(ctx, OmciAniResourceRemoved)
+
+}
+
+func (onuTP *onuUniTechProf) isTechProfileConfigCleared(ctx context.Context, uniID uint8, tpID uint8) bool {
+	uniTPKey := uniTP{uniID: uniID, tpID: tpID}
+	logger.Debugw(ctx, "isTechProfileConfigCleared", log.Fields{"device-id": onuTP.deviceID})
+	if onuTP.mapPonAniConfig[uniTPKey] != nil {
+		mapGemPortParams := onuTP.mapPonAniConfig[uniTPKey].mapGemPortParams
+		unicastGemCount := 0
+		for _, gemEntry := range mapGemPortParams {
+			if !gemEntry.isMulticast {
+				unicastGemCount++
+			}
+		}
+		if unicastGemCount == 0 || onuTP.mapPonAniConfig[uniTPKey].tcontParams.allocID == 0 {
+			logger.Debugw(ctx, "clearing-ani-side-config", log.Fields{
+				"device-id": onuTP.deviceID, "uniTpKey": uniTPKey})
+			onuTP.clearAniSideConfig(ctx, uniID, tpID)
+			if _, ok := onuTP.pAniConfigFsm[uniTPKey]; ok {
+				_ = onuTP.pAniConfigFsm[uniTPKey].pAdaptFsm.pFsm.Event(aniEvReset)
+			}
+			go onuTP.baseDeviceHandler.deviceProcStatusUpdate(ctx, OmciAniResourceRemoved)
+			return true
+		}
 	}
+	return false
 }
 
 func (onuTP *onuUniTechProf) waitForTimeoutOrCompletion(