split the set request into multiple Set in configuration controller (o…

…nosproject#1615) * split the set request into multiple Set in configuration controller * update api * update dep * rebase * go mod tidy * remove commented out code * add log
khu-mcl · Feb 7, 2023 · 87cd690 · 87cd690
1 parent 7ff40e0
commit 87cd690
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 39 deletions.
diff --git a/go.mod b/go.mod
@@ -10,7 +10,7 @@ require (
 	github.com/google/uuid v1.3.0
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/onosproject/config-models/models/testdevice-1.0.x v0.5.29
-	github.com/onosproject/onos-api/go v0.10.18
+	github.com/onosproject/onos-api/go v0.10.22
 	github.com/onosproject/onos-lib-go v0.10.7
 	github.com/openconfig/gnmi v0.0.0-20220920173703-480bf53a74d2
 	github.com/openconfig/goyang v1.1.0

diff --git a/go.sum b/go.sum
@@ -252,8 +252,8 @@ github.com/mitchellh/mapstructure v1.4.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RR
 github.com/onosproject/config-models v0.10.43 h1:v8yav4VhppviEzU2kktO/FRrRQLiP87NTFiRXjA5c2U=
 github.com/onosproject/config-models/models/testdevice-1.0.x v0.5.29 h1:qiNdq5jMZaxsrM6FIpxWY0c6oo8Kllg4QHYNXSW4XoI=
 github.com/onosproject/config-models/models/testdevice-1.0.x v0.5.29/go.mod h1:EErgiCOwX4CTrijcOgCKqESj2enPi7ehSfgFfD9OH/4=
-github.com/onosproject/onos-api/go v0.10.18 h1:3ztkZLOk0cxr5HAI/Xi3GHdB/JehEKwgxZnoAFVjgBA=
-github.com/onosproject/onos-api/go v0.10.18/go.mod h1:R882+8UcxQBLpCopnnbBnemXtJ77UXnL2I1y5yHbq10=
+github.com/onosproject/onos-api/go v0.10.22 h1:NUd3a2R5vxIg8Qv3wrF7gyLFXrEbl7VQG5QPU5p2Utk=
+github.com/onosproject/onos-api/go v0.10.22/go.mod h1:R882+8UcxQBLpCopnnbBnemXtJ77UXnL2I1y5yHbq10=
 github.com/onosproject/onos-lib-go v0.10.7 h1:fSEFis3ZX2E13s/CahK1vHJ1849tEFfQSA5LciVZWsw=
 github.com/onosproject/onos-lib-go v0.10.7/go.mod h1:7WJKZwkje6ft9vAHP+PHDsRJo4dMl5wqtIOavLJ0h6w=
 github.com/openconfig/gnmi v0.0.0-20200414194230-1597cc0f2600/go.mod h1:M/EcuapNQgvzxo1DDXHK4tx3QpYM/uG4l591v33jG2A=

diff --git a/pkg/controller/configuration/controller.go b/pkg/controller/configuration/controller.go
@@ -176,50 +176,52 @@ func (r *Reconciler) reconcileConfiguration(ctx context.Context, config *configa
 		return controller.Result{}, nil
 	}
 
-	pathValues := make([]*configapi.PathValue, 0, len(config.Status.Applied.Values))
+	indexedPathValues := make(map[configapi.Index][]*configapi.PathValue)
 	if config.Status.Applied.Values != nil {
 		for _, appliedValue := range config.Status.Applied.Values {
-			pathValues = append(pathValues, appliedValue)
+			indexedPathValues[appliedValue.Index] = append(indexedPathValues[appliedValue.Index], appliedValue)
 		}
 	}
-	log.Infof("Updating %d paths on target '%s'", len(pathValues), config.TargetID)
-
-	// Create a gNMI set request
-	setRequest, err := utilsv2.PathValuesToGnmiChange(pathValues, config.TargetID)
-	if err != nil {
-		log.Errorf("Failed constructing SetRequest for Configuration '%s'", config.ID, err)
-		return controller.Result{}, nil
-	}
+	log.Infof("Updating %d paths on target '%s'", len(indexedPathValues), config.TargetID)
+	for transactionIndex, pathValues := range indexedPathValues {
+		// Create a gNMI set request
+		log.Debugw("Creating Set request for changes in transaction", "TransactionIndex", transactionIndex)
+		setRequest, err := utilsv2.PathValuesToGnmiChange(pathValues, config.TargetID)
+		if err != nil {
+			log.Errorf("Failed constructing SetRequest for Configuration '%s'", config.ID, err)
+			return controller.Result{}, nil
+		}
 
-	// Add the master arbitration extension to provide concurrency control for multi-node controllers.
-	setRequest.Extension = append(setRequest.Extension, &gnmi_ext.Extension{
-		Ext: &gnmi_ext.Extension_MasterArbitration{
-			MasterArbitration: &gnmi_ext.MasterArbitration{
-				Role: &gnmi_ext.Role{
-					Id: "onos-config",
-				},
-				ElectionId: &gnmi_ext.Uint128{
-					Low: uint64(mastershipTerm),
+		// Add the master arbitration extension to provide concurrency control for multi-node controllers.
+		setRequest.Extension = append(setRequest.Extension, &gnmi_ext.Extension{
+			Ext: &gnmi_ext.Extension_MasterArbitration{
+				MasterArbitration: &gnmi_ext.MasterArbitration{
+					Role: &gnmi_ext.Role{
+						Id: "onos-config",
+					},
+					ElectionId: &gnmi_ext.Uint128{
+						Low: uint64(mastershipTerm),
+					},
 				},
 			},
-		},
-	})
-
-	// Execute the set request
-	log.Debugf("Sending SetRequest %+v", setRequest)
-	setResponse, err := conn.Set(ctx, setRequest)
-	if err != nil {
-		// The gNMI Set request can be denied if this master has been superseded by a master in a later term.
-		// Rather than reverting to the STALE state now, wait for this node to see the mastership state change
-		// to avoid flapping between states while the system converges.
-		if errors.IsForbidden(err) {
-			log.Warnf("Configuration '%s' mastership superseded for term %d", config.ID, mastershipTerm)
-			return controller.Result{}, nil
+		})
+
+		// Execute the set request
+		log.Debugf("Sending SetRequest %+v", setRequest)
+		setResponse, err := conn.Set(ctx, setRequest)
+		if err != nil {
+			// The gNMI Set request can be denied if this master has been superseded by a master in a later term.
+			// Rather than reverting to the STALE state now, wait for this node to see the mastership state change
+			// to avoid flapping between states while the system converges.
+			if errors.IsForbidden(err) {
+				log.Warnf("Configuration '%s' mastership superseded for term %d", config.ID, mastershipTerm)
+				return controller.Result{}, nil
+			}
+			log.Errorf("Failed sending SetRequest %+v", setRequest, err)
+			return controller.Result{}, err
 		}
-		log.Errorf("Failed sending SetRequest %+v", setRequest, err)
-		return controller.Result{}, err
+		log.Debugf("Received SetResponse %+v", setResponse)
 	}
-	log.Debugf("Received SetResponse %+v", setResponse)
 
 	// Update the configuration state and path statuses
 	log.Infof("Configuration '%s' synchronization complete", config.ID)

diff --git a/pkg/controller/transaction/controller.go b/pkg/controller/transaction/controller.go
@@ -129,6 +129,12 @@ func (r *Reconciler) reconcileInitialize(ctx context.Context, transaction *confi
 								targetTypeVersion = *ttv
 							}
 						}
+						changeValues := change.Values
+						changeValuesWithIndex := make(map[string]*configapi.PathValue)
+						for targetID, changeValue := range changeValues {
+							changeValue.Index = transaction.Index
+							changeValuesWithIndex[targetID] = changeValue
+						}
 
 						// Create a proposal for this target
 						proposal := &configapi.Proposal{
@@ -137,7 +143,7 @@ func (r *Reconciler) reconcileInitialize(ctx context.Context, transaction *confi
 							TargetID:         targetID,
 							Details: &configapi.Proposal_Change{
 								Change: &configapi.ChangeProposal{
-									Values: change.Values,
+									Values: changeValuesWithIndex,
 								},
 							},
 							TargetTypeVersion: targetTypeVersion,