Connectors flaky tests fix #1717
Conversation
…ment is deleted while conting operatin is in progress.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1717 +/- ##
=======================================
Coverage 82.26% 82.26%
=======================================
Files 161 161
Lines 14989 14989
=======================================
Hits 12330 12330
Misses 2223 2223
Partials 436 436
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
|
||
| if err := dbConn.Save(resource).Error; err != nil { | ||
| saveResult := dbConn.Save(resource) |
There was a problem hiding this comment.
As stated in the more general comment I would vote for merging this one since it fixes tests and current behavior and then reason about how to deal with new gorm version behavior.
| @@ -505,7 +523,7 @@ func (k *connectorClusterService) UpdateConnectorDeploymentStatus(ctx context.Co | |||
| return services.HandleGoneError("Connector deployment", "id", deploymentStatus.ID) | |||
| } | |||
|
|
|||
| if err := dbConn.Model(&deploymentStatus).Where("id = ? and version <= ?", deploymentStatus.ID, deploymentStatus.Version).Save(&deploymentStatus).Error; err != nil { | |||
| if err := dbConn.Where("id = ? and version <= ?", deploymentStatus.ID, deploymentStatus.Version).Save(&deploymentStatus).Error; err != nil { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Here I just removed the use of Model() with Save() as is not supported and can lead to undefined behavior (as per documentation first NOTE: https://gorm.io/docs/update.html)
So I believe we can merge this one and then reason about the whole // See https://github.com/go-gorm/gorm/issues/6139 // We can no longer rely on the Save throwing a constraint violation exception when upserting a record that already exists. // By first checking that the record existences we ensure the Save only updates an existing record vs trying to create a duplicate.
Yep that is a problem for the upgrade for sure. I fear an issue to make that behavior configurable needs to be open to gorm unless there is a solution to implement optimistic locking in scenarios where |
Description
Fixed flaky tests.
More in detail:
Verification Steps
i=0while /home/valdar/projects/communityProjects/kas-fleet-manager/bin/gotestsum --junitfile data/results/integraton-tests-connector.xml --format short-verbose -- -p 1 -ldflags -s -v -timeout 15m -count=1 ./internal/connector/test/integration/... ; do ((i=i+1)) && echo $i ; doneChecklist (Definition of Done)
All acceptance criteria specified in JIRA have been completedUnit and integration tests added that prove the fix is effective or the feature works (tested against emulated and non-emulated OCM environment)Documentation added for the featureRequired metrics/dashboards/alerts have been added (or PR created).Required Standard Operating Procedure (SOP) is added.JIRA has been created for changes required on the client side