Skip to content

Commit 790b579

Browse files
nammnnammn
authored
[fix] fixes a sts controller race condition with the mongodb-agent (mongodb#1198)
This patch fixes a bug where we move our agents into the version change mode (version change steps) and potentially move into a deadlock. The following can happen: - sts is ready - we write a new automation config - agent receives new automation config - mongodb-agents will start to delete pods in a rolling update fashion (not the agents itself, but the result is the same) - primary will be replaced last (rolling update) - every pods readiness probe fails (we do have some time until we call it unhealthy) - depending on the time it took a pod can get unhealthy as the readiness probe fails. - if the pod got unhealthy, the sts controller will not be able to start up a new pod that we just recently killed -> deadlock
1 parent a4ad55f commit 790b579

File tree

8 files changed

+182
-12
lines changed

8 files changed

+182
-12
lines changed

‎cmd/readiness/main.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ var riskySteps []string
2929
var logger *zap.SugaredLogger
3030

3131
func init() {
32-
riskySteps = []string{"WaitAllRsMembersUp", "WaitRsInit"}
32+
riskySteps = []string{"WaitAllRsMembersUp", "WaitRsInit", "WaitCanUpdate"}
3333

34-
// By default we log to the output (convenient for tests)
34+
// By default, we log to the output (convenient for tests)
3535
cfg := zap.NewDevelopmentConfig()
3636
log, err := cfg.Build()
3737
if err != nil {
@@ -77,7 +77,7 @@ func isPodReady(conf config.Config) (bool, error) {
7777
return true, nil
7878
}
7979

80-
// Failback logic: the agent is not in goal state and got stuck in some steps
80+
// Fallback logic: the agent is not in goal state and got stuck in some steps
8181
if !inGoalState && hasDeadlockedSteps(healthStatus) {
8282
return true, nil
8383
}
@@ -139,8 +139,8 @@ func findCurrentStep(processStatuses map[string]health.MmsDirectorStatus) *healt
139139
}
140140

141141
func isDeadlocked(status *health.StepStatus) bool {
142-
// Some logic behind 15 seconds: the health status file is dumped each 10 seconds so we are sure that if the agent
143-
// has been in the the step for 10 seconds - this means it is waiting for the other hosts and they are not available
142+
// Some logic behind 15 seconds: the health status file is dumped each 10 seconds, so we are sure that if the agent
143+
// has been in the step for 10 seconds - this means it is waiting for the other hosts, and they are not available
144144
fifteenSecondsAgo := time.Now().Add(time.Duration(-15) * time.Second)
145145
if contains.String(riskySteps, status.Step) && status.Completed == nil && status.Started.Before(fifteenSecondsAgo) {
146146
logger.Infof("Indicated a possible deadlock, status: %s, started at %s but hasn't finished "+

‎cmd/readiness/readiness_test.go

+6
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ func TestDeadlockDetection(t *testing.T) {
2727
assert.NoError(t, err)
2828
}
2929

30+
func TestDeadlockDetectionDuringVersionChange(t *testing.T) {
31+
ready, err := isPodReady(testConfig("testdata/health-status-deadlocked-with-prev-config.json"))
32+
assert.True(t, ready)
33+
assert.NoError(t, err)
34+
}
35+
3036
// TestNoDeadlock verifies that if the agent has started (but not finished) "WaitRsInit" and then there is another
3137
// started phase ("WaitFeatureCompatibilityVersionCorrect") then no deadlock is found as the latter is considered to
3238
// be the "current" step

‎cmd/readiness/testdata/health-status-deadlocked-with-prev-config.json

+159
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
{
2+
"statuses": {
3+
"mdb0-1": {
4+
"IsInGoalState": false,
5+
"LastMongoUpTime": 1674151493,
6+
"ExpectedToBeUp": true,
7+
"ReplicationStatus": 1
8+
}
9+
},
10+
"mmsStatus": {
11+
"mdb0-1": {
12+
"name": "mdb0-1",
13+
"lastGoalVersionAchieved": 2,
14+
"plans": [
15+
{
16+
"automationConfigVersion": 2,
17+
"started": "2023-01-19T17:27:17.438126081Z",
18+
"completed": "2023-01-19T17:27:22.74117999Z",
19+
"moves": [
20+
{
21+
"move": "Start",
22+
"moveDoc": "Start the process",
23+
"steps": [
24+
{
25+
"step": "StartFresh",
26+
"stepDoc": "Start a mongo instance (start fresh)",
27+
"isWaitStep": false,
28+
"started": "2023-01-19T17:27:17.438319285Z",
29+
"completed": "2023-01-19T17:27:21.672553263Z",
30+
"result": "success"
31+
}
32+
]
33+
},
34+
{
35+
"move": "WaitRsInit",
36+
"moveDoc": "Wait for the replica set to be initialized by another member",
37+
"steps": [
38+
{
39+
"step": "WaitRsInit",
40+
"stepDoc": "Wait for the replica set to be initialized by another member",
41+
"isWaitStep": true,
42+
"started": "2023-01-19T17:27:21.672605664Z",
43+
"completed": null,
44+
"result": "error"
45+
}
46+
]
47+
},
48+
{
49+
"move": "WaitFeatureCompatibilityVersionCorrect",
50+
"moveDoc": "Wait for featureCompatibilityVersion to be right",
51+
"steps": [
52+
{
53+
"step": "WaitFeatureCompatibilityVersionCorrect",
54+
"stepDoc": "Wait for featureCompatibilityVersion to be right",
55+
"isWaitStep": true,
56+
"started": null,
57+
"completed": null,
58+
"result": ""
59+
}
60+
]
61+
}
62+
]
63+
},
64+
{
65+
"automationConfigVersion": 2,
66+
"started": "2023-01-19T17:36:34.742889301Z",
67+
"completed": "2023-01-19T17:36:47.913043483Z",
68+
"moves": [
69+
{
70+
"move": "WaitHasCorrectAutomationCredentials",
71+
"moveDoc": "Wait for the automation user to be added (if needed)",
72+
"steps": [
73+
{
74+
"step": "WaitHasCorrectAutomationCredentials",
75+
"stepDoc": "Wait for the automation user to be added (if needed)",
76+
"isWaitStep": true,
77+
"started": "2023-01-19T17:36:34.742906201Z",
78+
"completed": null,
79+
"result": "wait"
80+
}
81+
]
82+
}
83+
]
84+
},
85+
{
86+
"automationConfigVersion": 3,
87+
"started": "2023-01-19T17:38:33.622622261Z",
88+
"completed": null,
89+
"moves": [
90+
{
91+
"move": "ChangeVersion",
92+
"moveDoc": "Change MongoDB Version",
93+
"steps": [
94+
{
95+
"step": "CheckWrongVersion",
96+
"stepDoc": "Check that MongoDB version is wrong",
97+
"isWaitStep": false,
98+
"started": "2023-01-19T17:38:33.622638561Z",
99+
"completed": "2023-01-19T17:38:33.622959367Z",
100+
"result": "success"
101+
},
102+
{
103+
"step": "CheckRsCorrect",
104+
"stepDoc": "Check that replica set configuration is correct",
105+
"isWaitStep": false,
106+
"started": "2023-01-19T17:38:33.622960067Z",
107+
"completed": "2023-01-19T17:38:33.623363973Z",
108+
"result": "success"
109+
},
110+
{
111+
"step": "WaitCanUpdate",
112+
"stepDoc": "Wait until the update can be made",
113+
"isWaitStep": true,
114+
"started": "2023-01-19T17:38:33.623364774Z",
115+
"completed": null,
116+
"result": "wait"
117+
},
118+
{
119+
"step": "DisableBalancerIfFirst",
120+
"stepDoc": "Disable the balancer (may take a while)",
121+
"isWaitStep": false,
122+
"started": null,
123+
"completed": null,
124+
"result": ""
125+
},
126+
{
127+
"step": "Stop",
128+
"stepDoc": "Shutdown the process",
129+
"isWaitStep": false,
130+
"started": null,
131+
"completed": null,
132+
"result": ""
133+
},
134+
{
135+
"step": "RemoveDbFilesIfArbiterDowngrade",
136+
"stepDoc": "Delete db files if this is an arbiter downgrade.",
137+
"isWaitStep": false,
138+
"started": null,
139+
"completed": null,
140+
"result": ""
141+
},
142+
{
143+
"step": "StartWithUpgrade",
144+
"stepDoc": "Start a mongo instance (upgrade)",
145+
"isWaitStep": false,
146+
"started": null,
147+
"completed": null,
148+
"result": ""
149+
}
150+
]
151+
}
152+
]
153+
}
154+
],
155+
"errorCode": 0,
156+
"errorString": ""
157+
}
158+
}
159+
}

‎controllers/replica_set_controller.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ func (r *ReplicaSetReconciler) buildService(mdb mdbv1.MongoDBCommunity, portMana
583583
}
584584

585585
// validateSpec checks if the MongoDB resource Spec is valid.
586-
// If there has not yet been a successful configuration, the function runs the intial Spec validations. Otherwise
586+
// If there has not yet been a successful configuration, the function runs the initial Spec validations. Otherwise,
587587
// it checks that the attempted Spec is valid in relation to the Spec that resulted from that last successful configuration.
588588
func (r ReplicaSetReconciler) validateSpec(mdb mdbv1.MongoDBCommunity) error {
589589
lastSuccessfulConfigurationSaved, ok := mdb.Annotations[lastSuccessfulConfiguration]

‎docs/architecture.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ When you update the MongoDB version in your resource definition and reapply it t
6161

6262
1. The MongoDB Agent chooses the first pod to upgrade and stops the `mongod` process using a local connection and [`db.shutdownServer`](https://www.mongodb.com/docs/manual/reference/method/db.shutdownServer/#db.shutdownServer).
6363

64-
1. Kubernetes will restart the `mongod` container causing the version change hook to run and check the state of the MongoDB Agent. If the MongoDB Agent expects the `mongod` process to start with a new version, the hook uses a Kubernetes API call to delete the pod.
64+
1. Kubernetes will restart the `mongod` container causing the version change hook to run before the `mongod` process and check the state of the MongoDB Agent. If the MongoDB Agent expects the `mongod` process to start with a new version, the hook uses a Kubernetes API call to delete the pod.
6565

6666
1. The Kubernetes Controller downloads the target version of MongoDB from its default docker registry and restarts the pod with the target version of `mongod` in the database container.
6767

‎scripts/ci/dump_diagnostics.sh

+6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ kubectl get crd mongodbcommunity.mongodbcommunity.mongodb.com -o yaml > diagnost
1010
echo "Dumping Pod list"
1111
kubectl get pods > diagnostics/pod-list.txt
1212

13+
echo "Dumping Event list"
14+
kubectl get events --sort-by='.lastTimestamp' -owide > diagnostics/events-list.txt
15+
16+
echo "Dumping yaml Event list"
17+
kubectl kubectl get events --sort-by='.lastTimestamp' -oyaml > diagnostics/events-list.yaml
18+
1319
# dump operator deployment information.
1420
for deployment_name in $(kubectl get deployment -n "${namespace}" --output=jsonpath={.items..metadata.name}); do
1521
echo "Writing Deployment describe for deployment ${deployment_name}"

‎test/e2e/feature_compatibility_version/feature_compatibility_version_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@ package feature_compatibility_version
22

33
import (
44
"fmt"
5+
. "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/util/mongotester"
56
"os"
67
"testing"
78
"time"
89

9-
. "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/util/mongotester"
10-
1110
e2eutil "github.com/mongodb/mongodb-kubernetes-operator/test/e2e"
1211
"github.com/mongodb/mongodb-kubernetes-operator/test/e2e/mongodbtests"
1312
setup "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/setup"

‎test/e2e/mongodbtests/mongodbtests.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func MongoDBReachesRunningPhase(mdb *mdbv1.MongoDBCommunity) func(t *testing.T)
253253
}
254254
}
255255

256-
// MongoDBReachesFailed ensure the MongoDB resource reaches the Failed phase.
256+
// MongoDBReachesFailedPhase ensure the MongoDB resource reaches the Failed phase.
257257
func MongoDBReachesFailedPhase(mdb *mdbv1.MongoDBCommunity) func(t *testing.T) {
258258
return func(t *testing.T) {
259259
err := wait.ForMongoDBToReachPhase(t, mdb, mdbv1.Failed, time.Second*15, time.Minute*5)
@@ -294,7 +294,7 @@ func AutomationConfigVersionHasTheExpectedVersion(mdb *mdbv1.MongoDBCommunity, e
294294
}
295295
}
296296

297-
// AutomationConfigVersionHasTheExpectedVersion verifies that the automation config has the expected version.
297+
// AutomationConfigReplicaSetsHaveExpectedArbiters verifies that the automation config has the expected version.
298298
func AutomationConfigReplicaSetsHaveExpectedArbiters(mdb *mdbv1.MongoDBCommunity, expectedArbiters int) func(t *testing.T) {
299299
return func(t *testing.T) {
300300
currentAc := getAutomationConfig(t, mdb)
@@ -454,7 +454,7 @@ func Scale(mdb *mdbv1.MongoDBCommunity, newMembers int) func(*testing.T) {
454454
}
455455
}
456456

457-
// Scale update the MongoDB with a new number of arbiters and updates the resource.
457+
// ScaleArbiters update the MongoDB with a new number of arbiters and updates the resource.
458458
func ScaleArbiters(mdb *mdbv1.MongoDBCommunity, newArbiters int) func(*testing.T) {
459459
return func(t *testing.T) {
460460
t.Logf("Scaling Mongodb %s, to %d members", mdb.Name, newArbiters)

0 commit comments

Comments
 (0)