Skip to content

Commit 910ac66

Browse files
AlexBasinovcopybara-github
authored andcommitted
Include GCE Instance ID in the command locking key.
PiperOrigin-RevId: 856254303
1 parent 11f99b1 commit 910ac66

File tree

7 files changed

+53
-26
lines changed

7 files changed

+53
-26
lines changed

.github/workflows/go-build-and-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
cd workloadagentplatform
2424
# this is the hash of the workloadagentplatform submodule
2525
# get the hash by running: go list -m -json github.com/GoogleCloudPlatform/workloadagentplatform@main
26-
git checkout 27b82f043534ca3dd5965f7ebd92b5317e19ecf6
26+
git checkout 64b59c6392f2aaf421479a0c2f4bfdfafe85cf81
2727
cd ..
2828
find workloadagentplatform/sharedprotos -type f -exec sed -i 's|"sharedprotos|"workloadagentplatform/sharedprotos|g' {} +
2929
env:

.github/workflows/go-build-protos.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
cd workloadagentplatform
3232
# this is the hash of the workloadagentplatform submodule
3333
# get the hash by running: go list -m -json github.com/GoogleCloudPlatform/workloadagentplatform@main
34-
git checkout 27b82f043534ca3dd5965f7ebd92b5317e19ecf6
34+
git checkout 64b59c6392f2aaf421479a0c2f4bfdfafe85cf81
3535
cd ..
3636
find workloadagentplatform/sharedprotos -type f -exec sed -i 's|"sharedprotos|"workloadagentplatform/sharedprotos|g' {} +
3737
env:

build.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ if [ "${COMPILE_PROTOS}" == "TRUE" ] && [ ! -d "workloadagentplatform" ]; then
5656
cd workloadagentplatform
5757
# this is the hash of the workloadagentplatform submodule
5858
# get the hash by running: go list -m -json github.com/GoogleCloudPlatform/workloadagentplatform@main
59-
git checkout 27b82f043534ca3dd5965f7ebd92b5317e19ecf6
59+
git checkout 64b59c6392f2aaf421479a0c2f4bfdfafe85cf81
6060
cd ..
6161
# replace the proto imports in the platform that reference the platform
6262
find workloadagentplatform/sharedprotos -type f -exec sed -i 's|"sharedprotos|"workloadagentplatform/sharedprotos|g' {} +

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ require (
1313
github.com/DATA-DOG/go-sqlmock v1.5.0
1414
// Get the version by running:
1515
// go list -m -json github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries@main
16-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260109223652-27b82f043534
16+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260113220648-64b59c6392f2
1717
// Get the version by running:
1818
// go list -m -json github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos@main
19-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260109223652-27b82f043534
19+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260113220648-64b59c6392f2
2020
github.com/StackExchange/wmi v1.2.1
2121
github.com/cenkalti/backoff/v4 v4.3.0
2222
github.com/gammazero/workerpool v1.1.3

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20O
3131
github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM=
3232
github.com/GoogleCloudPlatform/agentcommunication_client v0.0.0-20250227185639-b70667e4a927 h1:nn31d5gg+ysSNqWTqSOxsKBj17GJZBqsBx7biZAgYtI=
3333
github.com/GoogleCloudPlatform/agentcommunication_client v0.0.0-20250227185639-b70667e4a927/go.mod h1:A1V05o309ZvTwy/FTBooYvvIhzM6mtsJcHJsAkeuAAM=
34-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260109223652-27b82f043534 h1:aNKg05es2YNDllzWmWmB6Yx9BYA9u3fouDrH/aarfZI=
35-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260109223652-27b82f043534/go.mod h1:WrwTr9HFkp+nbHba/tv36eCLkjKAeCx7mqiG/wL3PNU=
36-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260109223652-27b82f043534 h1:p/dzu0l6Xnz+zK/bTcCrc2WFQFkxC36QMapIs4MMHm4=
37-
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260109223652-27b82f043534/go.mod h1:8Ea8vdBuPsWhhwzL9sNK7BFQE9qbkPLZUHxcucWHXaM=
34+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260113220648-64b59c6392f2 h1:GoTr/c70uw/VjB0j2WEi21ckcpqDWBRRV5GXMVQFcIo=
35+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries v0.0.0-20260113220648-64b59c6392f2/go.mod h1:WrwTr9HFkp+nbHba/tv36eCLkjKAeCx7mqiG/wL3PNU=
36+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260113220648-64b59c6392f2 h1:jc8eNgr1QSd55M3ypCrOdRv2IAaDOf8sOaCCSBsLnh4=
37+
github.com/GoogleCloudPlatform/workloadagentplatform/sharedprotos v0.0.0-20260113220648-64b59c6392f2/go.mod h1:8Ea8vdBuPsWhhwzL9sNK7BFQE9qbkPLZUHxcucWHXaM=
3838
github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA=
3939
github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8=
4040
github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY=

internal/daemon/oracle/oracle.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,19 +206,21 @@ func runGuestActions(ctx context.Context, a any) {
206206

207207
// oracleCommandKey returns the locking key and timeout for a given command and whether the command
208208
// should be locked.
209-
// The locking key is formed by ORACLE_SID and ORACLE_HOME.
210-
func oracleCommandKey(cmd *gapb.Command) (key string, timeout time.Duration, lock bool) {
209+
// The locking key is formed by instance ID, oracle home, and oracle sid.
210+
func oracleCommandKey(ctx context.Context, cmd *gapb.Command, cp *metadataserver.CloudProperties) (key string, timeout time.Duration, lock bool) {
211211
params := cmd.GetAgentCommand().GetParameters()
212212
sid, sidOk := params["oracle_sid"]
213213
home, homeOk := params["oracle_home"]
214214

215215
if !sidOk || !homeOk {
216-
// Cannot form a unique key, don't lock
217-
// TODO: Add ctx to CommandConcurrencyKey to allow logging this case.
216+
log.CtxLogger(ctx).Warnw("Required parameters are missing, cannot form a unique key", "command", cmd.GetAgentCommand(), "cloud_properties", cp, "oracle_sid", sid, "oracle_home", home)
218217
return "", 0, false
219218
}
220-
// TODO: Add GCE Instance ID to the key to make it unique across different GCE instances.
221-
return fmt.Sprintf("%s:%s", sid, home), defaultLockTimeout, true
219+
if cp == nil {
220+
log.CtxLogger(ctx).Warnw("Cloud properties are nil, cannot form locking key", "command", cmd.GetAgentCommand(), "cloud_properties", cp, "oracle_sid", sid, "oracle_home", home)
221+
return "", 0, false
222+
}
223+
return fmt.Sprintf("%s:%s:%s", cp.InstanceID, home, sid), defaultLockTimeout, true
222224
}
223225

224226
func runDiscovery(ctx context.Context, a any) {

internal/daemon/oracle/oracle_test.go

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ func TestConvertCloudProperties(t *testing.T) {
4848
want *metadataserver.CloudProperties
4949
}{
5050
{
51-
name: "nil cloud properties",
51+
name: "NilCloudProperties",
5252
cp: nil,
5353
want: nil,
5454
},
5555
{
56-
name: "non-nil cloud properties",
56+
name: "NonNilCloudProperties",
5757
cp: &cpb.CloudProperties{
5858
ProjectId: "test-project",
5959
NumericProjectId: "12345",
@@ -95,32 +95,35 @@ func TestOracleCommandKey(t *testing.T) {
9595
tests := []struct {
9696
name string
9797
cmd *gapb.Command
98+
cp *metadataserver.CloudProperties
9899
wantKey string
99100
wantTimeout time.Duration
100101
wantLock bool
101102
}{
102103
{
103-
name: "nil agent command",
104+
name: "NilAgentCommand",
104105
cmd: &gapb.Command{},
106+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
105107
wantKey: "",
106108
wantTimeout: 0,
107109
wantLock: false,
108110
},
109111
{
110-
name: "nil parameters",
112+
name: "NilParameters",
111113
cmd: &gapb.Command{
112114
CommandType: &gapb.Command_AgentCommand{
113115
AgentCommand: &gapb.AgentCommand{
114116
Command: "oracle_start_database",
115117
},
116118
},
117119
},
120+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
118121
wantKey: "",
119122
wantTimeout: 0,
120123
wantLock: false,
121124
},
122125
{
123-
name: "empty parameters",
126+
name: "EmptyParameters",
124127
cmd: &gapb.Command{
125128
CommandType: &gapb.Command_AgentCommand{
126129
AgentCommand: &gapb.AgentCommand{
@@ -129,12 +132,13 @@ func TestOracleCommandKey(t *testing.T) {
129132
},
130133
},
131134
},
135+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
132136
wantKey: "",
133137
wantTimeout: 0,
134138
wantLock: false,
135139
},
136140
{
137-
name: "missing oracle_home",
141+
name: "MissingOracleHome",
138142
cmd: &gapb.Command{
139143
CommandType: &gapb.Command_AgentCommand{
140144
AgentCommand: &gapb.AgentCommand{
@@ -145,12 +149,13 @@ func TestOracleCommandKey(t *testing.T) {
145149
},
146150
},
147151
},
152+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
148153
wantKey: "",
149154
wantTimeout: 0,
150155
wantLock: false,
151156
},
152157
{
153-
name: "missing oracle_sid",
158+
name: "MissingOracleSID",
154159
cmd: &gapb.Command{
155160
CommandType: &gapb.Command_AgentCommand{
156161
AgentCommand: &gapb.AgentCommand{
@@ -161,12 +166,13 @@ func TestOracleCommandKey(t *testing.T) {
161166
},
162167
},
163168
},
169+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
164170
wantKey: "",
165171
wantTimeout: 0,
166172
wantLock: false,
167173
},
168174
{
169-
name: "with oracle_sid and oracle_home",
175+
name: "SIDAndHome_NilCloudProperties",
170176
cmd: &gapb.Command{
171177
CommandType: &gapb.Command_AgentCommand{
172178
AgentCommand: &gapb.AgentCommand{
@@ -178,17 +184,36 @@ func TestOracleCommandKey(t *testing.T) {
178184
},
179185
},
180186
},
181-
wantKey: "orcl:/u01/app/oracle/product/19.3.0/dbhome_1",
187+
cp: nil,
188+
wantKey: "",
189+
wantTimeout: 0,
190+
wantLock: false,
191+
},
192+
{
193+
name: "SIDAndHome_CloudProperties",
194+
cmd: &gapb.Command{
195+
CommandType: &gapb.Command_AgentCommand{
196+
AgentCommand: &gapb.AgentCommand{
197+
Command: "oracle_health_check",
198+
Parameters: map[string]string{
199+
"oracle_sid": "orcl",
200+
"oracle_home": "/u01/app/oracle/product/19.3.0/dbhome_1",
201+
},
202+
},
203+
},
204+
},
205+
cp: &metadataserver.CloudProperties{InstanceID: "test-instance"},
206+
wantKey: "test-instance:/u01/app/oracle/product/19.3.0/dbhome_1:orcl",
182207
wantTimeout: 24 * time.Hour,
183208
wantLock: true,
184209
},
185210
}
186211

187212
for _, tc := range tests {
188213
t.Run(tc.name, func(t *testing.T) {
189-
gotKey, gotTimeout, gotLock := oracleCommandKey(tc.cmd)
214+
gotKey, gotTimeout, gotLock := oracleCommandKey(context.Background(), tc.cmd, tc.cp)
190215
if gotKey != tc.wantKey || gotTimeout != tc.wantTimeout || gotLock != tc.wantLock {
191-
t.Errorf("oracleCommandKey(%v) = (%q, %v, %v), want (%q, %v, %v)", tc.cmd, gotKey, gotTimeout, gotLock, tc.wantKey, tc.wantTimeout, tc.wantLock)
216+
t.Errorf("oracleCommandKey(%v, %v) = (%q, %v, %v), want (%q, %v, %v)", tc.cmd, tc.cp, gotKey, gotTimeout, gotLock, tc.wantKey, tc.wantTimeout, tc.wantLock)
192217
}
193218
})
194219
}

0 commit comments

Comments
 (0)