Merge pull request #260 from tranchitella/2.2.x

Fix: internal EP to update device's status returns 409 on write conflict
mendersoftware · Mar 31, 2021 · 0c36fe2 · 0c36fe2
2 parents 2b442d3 + 51ceec1
commit 0c36fe2
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 9 deletions.
diff --git a/api/http/api_inventory.go b/api/http/api_inventory.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2021 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -887,7 +887,10 @@ func (i *inventoryHandlers) InternalDevicesStatusHandler(w rest.ResponseWriter,
 		)
 		return
 	}
-	if err != nil {
+	if err == store.ErrWriteConflict {
+		u.RestErrWithLog(w, r, l, err, http.StatusConflict)
+		return
+	} else if err != nil {
 		u.RestErrWithLogInternal(w, r, l, err)
 		return
 	}

diff --git a/api/http/api_inventory_test.go b/api/http/api_inventory_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2021 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -2357,6 +2357,20 @@ func TestApiInventoryInternalDevicesStatus(t *testing.T) {
 			},
 			callsInventory: true,
 		},
+		"error, conflict": {
+			inputDevices: []model.DeviceUpdate{
+				{Id: model.DeviceID(oid.NewUUIDv5("1").String()), Revision: 1},
+				{Id: model.DeviceID(oid.NewUUIDv5("2").String()), Revision: 1},
+			},
+			tenantID:     tenantId,
+			status:       acceptedStatus,
+			inventoryErr: store.ErrWriteConflict,
+			resp: utils.JSONResponseParams{
+				OutputStatus:     http.StatusConflict,
+				OutputBodyObject: RestError("write conflict"),
+			},
+			callsInventory: true,
+		},
 	}
 
 	for name, tc := range testCases {

diff --git a/docs/internal_api.yml b/docs/internal_api.yml
@@ -154,6 +154,10 @@ paths:
           description: Malformed request body. See error for details.
           schema:
             $ref: '#/definitions/Error'
+        409:
+          description: Write conflict, the request needs to be retried.
+          schema:
+            $ref: '#/definitions/Error'
         500:
           description: Internal server error.
           schema:

diff --git a/store/datastore.go b/store/datastore.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2021 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -27,9 +27,12 @@ var (
 
 	ErrGroupNotFound = errors.New("group not found")
 
-	// ErrNoAttrName is return if attributes are attempted upserted without
+	// ErrNoAttrName is returned if attributes are attempted upserted without
 	// a Name identifier.
 	ErrNoAttrName = errors.New("attribute name not present")
+
+	// ErrWriteConflict represents a write conflict in the storage layer
+	ErrWriteConflict = errors.New("write conflict")
 )
 
 //go:generate ../utils/mockgen.sh

diff --git a/store/mongo/datastore_mongo.go b/store/mongo/datastore_mongo.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2021 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -390,7 +390,7 @@ func (db *DataStoreMongo) upsertAttributes(
 		res, err = c.UpdateOne(ctx, filter, update, mopts.Update().SetUpsert(true))
 		if err != nil {
 			if strings.Contains(err.Error(), "duplicate key error") {
-				return &model.UpdateResult{}, nil
+				return nil, store.ErrWriteConflict
 			} else {
 				return nil, err
 			}
@@ -437,7 +437,10 @@ func (db *DataStoreMongo) upsertAttributes(
 		)
 		if err != nil {
 			if strings.Contains(err.Error(), "duplicate key error") {
-				return &model.UpdateResult{}, nil
+				// bulk mode, swallow the error as we already updated the other devices
+				// and the Matchedcount and CreatedCount values will tell the caller if
+				// all the operations succeeded or not
+				err = nil
 			} else {
 				return nil, err
 			}

diff --git a/store/mongo/datastore_mongo_test.go b/store/mongo/datastore_mongo_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2021 Northern.tech AS
 //
 //    Licensed under the Apache License, Version 2.0 (the "License");
 //    you may not use this file except in compliance with the License.
@@ -4414,6 +4414,8 @@ func TestMongoUpsertDevicesAttributesWithRevision(t *testing.T) {
 					CreatedTs: createdTs,
 				},
 			},
+
+			err: store.ErrWriteConflict,
 		},
 		"Update multiple device attributes": {
 			devs: []model.Device{{