Skip to content

Commit e6f9562

Browse files
committed
Merge remote-tracking branch 'public/master' into optimize
# Conflicts: # rust/crates/scheduler/src/accounting/redis_client.rs # rust/crates/scheduler/src/dao/layer_dao.rs # rust/crates/scheduler/src/host_cache/actor.rs # rust/crates/scheduler/src/host_cache/cache.rs # rust/crates/scheduler/src/pipeline/entrypoint.rs # rust/crates/scheduler/src/pipeline/matcher.rs
2 parents 0a21910 + f9acdd0 commit e6f9562

115 files changed

Lines changed: 5714 additions & 223 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cuebot/src/main/java/com/imageworks/spcue/servant/ManageShow.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ public void setCommentEmail(ShowSetCommentEmailRequest request,
448448
adminManager.updateShowCommentEmail(getShowEntity(request.getShow()),
449449
request.getEmail().split(","));
450450
responseObserver.onNext(ShowSetCommentEmailResponse.newBuilder().build());
451+
responseObserver.onCompleted();
451452
}
452453

453454
public AdminManager getAdminManager() {

cuebot/src/test/java/com/imageworks/spcue/test/servant/ManageShowTests.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,15 @@
2828
import com.imageworks.spcue.config.TestAppConfig;
2929
import com.imageworks.spcue.dao.ShowDao;
3030
import com.imageworks.spcue.grpc.show.Show;
31+
import com.imageworks.spcue.grpc.show.ShowSetCommentEmailRequest;
32+
import com.imageworks.spcue.grpc.show.ShowSetCommentEmailResponse;
3133
import com.imageworks.spcue.grpc.show.ShowSetSchedulerManagedRequest;
3234
import com.imageworks.spcue.grpc.show.ShowSetSchedulerManagedResponse;
3335
import com.imageworks.spcue.servant.ManageShow;
3436

37+
import io.grpc.stub.StreamObserver;
38+
39+
import static org.junit.Assert.assertArrayEquals;
3540
import static org.junit.Assert.assertFalse;
3641
import static org.junit.Assert.assertTrue;
3742

@@ -47,6 +52,55 @@ public class ManageShowTests extends AbstractTransactionalJUnit4SpringContextTes
4752

4853
private static final String SHOW_NAME = "pipe";
4954

55+
/**
56+
* StreamObserver that records whether onNext / onCompleted were called, so a servant method
57+
* that forgets to close the stream (onCompleted) is caught.
58+
*/
59+
private static class RecordingStreamObserver<T> implements StreamObserver<T> {
60+
boolean nextCalled = false;
61+
boolean completed = false;
62+
Throwable error = null;
63+
64+
@Override
65+
public void onNext(T value) {
66+
nextCalled = true;
67+
}
68+
69+
@Override
70+
public void onError(Throwable t) {
71+
error = t;
72+
}
73+
74+
@Override
75+
public void onCompleted() {
76+
completed = true;
77+
}
78+
}
79+
80+
@Test
81+
@Transactional
82+
@Rollback(true)
83+
public void testSetCommentEmail() {
84+
ShowEntity show = showDao.findShowDetail(SHOW_NAME);
85+
Show showProto = Show.newBuilder().setId(show.id).setName(show.name).build();
86+
87+
ShowSetCommentEmailRequest request = ShowSetCommentEmailRequest.newBuilder()
88+
.setShow(showProto).setEmail("first@example.com,second@example.com").build();
89+
RecordingStreamObserver<ShowSetCommentEmailResponse> observer =
90+
new RecordingStreamObserver<ShowSetCommentEmailResponse>();
91+
92+
manageShow.setCommentEmail(request, observer);
93+
94+
// The RPC must both emit a response and CLOSE the stream; a missing
95+
// onCompleted() leaves callers hanging until they time out.
96+
assertTrue(observer.nextCalled);
97+
assertTrue(observer.completed);
98+
99+
// And the email is persisted (split on comma, matching the servant).
100+
assertArrayEquals(new String[] {"first@example.com", "second@example.com"},
101+
showDao.findShowDetail(SHOW_NAME).commentMail);
102+
}
103+
50104
@Test
51105
@Transactional
52106
@Rollback(true)
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright Contributors to the OpenCue Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import {
18+
computeAllocationHostStats,
19+
buildAllocationRows,
20+
} from '@/app/allocations/allocation-utils';
21+
import type { Allocation, Host } from '@/app/utils/get_utils';
22+
23+
const host = (over: Partial<Host>): Host => ({
24+
id: 'h', name: 'h', state: 'UP', lockState: 'OPEN', nimbyEnabled: false,
25+
cores: 0, idleCores: 0, memory: '0', idleMemory: '0', totalMemory: '0',
26+
freeMcp: '0', bootTime: 0, pingTime: 0, ...over,
27+
});
28+
29+
describe('computeAllocationHostStats', () => {
30+
it('sums DOWN/REPAIR cores and counts REPAIR hosts per allocation', () => {
31+
const hosts: Host[] = [
32+
host({ allocName: 'local.general', state: 'DOWN', cores: 8 }),
33+
host({ allocName: 'local.general', state: 'REPAIR', cores: 4 }),
34+
host({ allocName: 'local.general', state: 'REPAIR', cores: 2 }),
35+
host({ allocName: 'local.general', state: 'UP', cores: 16 }),
36+
host({ allocName: 'cloud.general', state: 'DOWN', cores: 32 }),
37+
];
38+
39+
const stats = computeAllocationHostStats(hosts);
40+
41+
expect(stats['local.general']).toEqual({ downCores: 8, repairCores: 6, repairHosts: 2 });
42+
expect(stats['cloud.general']).toEqual({ downCores: 32, repairCores: 0, repairHosts: 0 });
43+
});
44+
45+
it('ignores hosts with no allocName', () => {
46+
const stats = computeAllocationHostStats([host({ state: 'DOWN', cores: 5 })]);
47+
expect(stats).toEqual({});
48+
});
49+
50+
it('returns {} for no hosts', () => {
51+
expect(computeAllocationHostStats([])).toEqual({});
52+
});
53+
});
54+
55+
describe('buildAllocationRows', () => {
56+
const alloc = (name: string): Allocation => ({
57+
id: name, name, tag: 'general', facility: 'local',
58+
stats: {
59+
cores: 20, availableCores: 10, idleCores: 10, runningCores: 0,
60+
lockedCores: 0, hosts: 2, lockedHosts: 0, downHosts: 0,
61+
},
62+
});
63+
64+
it('merges derived host stats onto each allocation', () => {
65+
const rows = buildAllocationRows(
66+
[alloc('local.general')],
67+
{ 'local.general': { downCores: 8, repairCores: 6, repairHosts: 2 } },
68+
);
69+
expect(rows[0].name).toBe('local.general');
70+
expect(rows[0].stats?.cores).toBe(20);
71+
expect(rows[0].downCores).toBe(8);
72+
expect(rows[0].repairCores).toBe(6);
73+
expect(rows[0].repairHosts).toBe(2);
74+
});
75+
76+
it('defaults derived stats to 0 when an allocation has no matching hosts', () => {
77+
const rows = buildAllocationRows([alloc('cloud.unassigned')], {});
78+
expect(rows[0].downCores).toBe(0);
79+
expect(rows[0].repairCores).toBe(0);
80+
expect(rows[0].repairHosts).toBe(0);
81+
});
82+
});

cueweb/app/__tests__/api/utils/action_utils.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import {
1919
eatJobsDeadFrames,
2020
killJobs,
21+
reparentGroups,
22+
reparentJobs,
2123
unpauseJobs
2224
} from '@/app/utils/action_utils';
2325
import { accessActionApi } from '@/app/utils/api_utils';
@@ -138,6 +140,94 @@ describe('action_utils', () => {
138140
});
139141
});
140142

143+
// Reparent groups under a new parent group
144+
describe('reparentGroups', () => {
145+
it('posts a single ReparentGroups request with the new parent and group ids', async () => {
146+
(accessActionApi as jest.Mock).mockResolvedValue({ success: true });
147+
148+
await reparentGroups('parent-1', ['g1', 'g2']);
149+
150+
expect(accessActionApi).toHaveBeenCalledWith(
151+
'/api/group/action/reparentgroups',
152+
[JSON.stringify({
153+
group: { id: 'parent-1' },
154+
groups: { groups: [{ id: 'g1' }, { id: 'g2' }] },
155+
})]
156+
);
157+
});
158+
159+
it('returns true on success (the group tree relies on this to refetch)', async () => {
160+
(accessActionApi as jest.Mock).mockResolvedValue({ success: true });
161+
162+
const result = await reparentGroups('parent-1', ['g1']);
163+
164+
expect(result).toBe(true);
165+
});
166+
167+
it('handles API errors gracefully', async () => {
168+
(accessActionApi as jest.Mock).mockRejectedValue(new Error('API Error'));
169+
170+
await reparentGroups('parent-1', ['g1']);
171+
172+
expect(handleError).toHaveBeenCalledWith(
173+
new Error('API Error'),
174+
`Error performing action for: /api/group/action/reparentgroups`
175+
);
176+
});
177+
178+
it('returns false on failure (the group tree relies on this to roll back)', async () => {
179+
(accessActionApi as jest.Mock).mockRejectedValue(new Error('API Error'));
180+
181+
const result = await reparentGroups('parent-1', ['g1']);
182+
183+
expect(result).toBe(false);
184+
});
185+
});
186+
187+
// Reparent jobs under a new parent group
188+
describe('reparentJobs', () => {
189+
it('posts a single ReparentJobs request with the new parent and job ids', async () => {
190+
(accessActionApi as jest.Mock).mockResolvedValue({ success: true });
191+
192+
await reparentJobs('parent-1', ['j1', 'j2']);
193+
194+
expect(accessActionApi).toHaveBeenCalledWith(
195+
'/api/group/action/reparentjobs',
196+
[JSON.stringify({
197+
group: { id: 'parent-1' },
198+
jobs: { jobs: [{ id: 'j1' }, { id: 'j2' }] },
199+
})]
200+
);
201+
});
202+
203+
it('returns true on success (the group tree relies on this to refetch)', async () => {
204+
(accessActionApi as jest.Mock).mockResolvedValue({ success: true });
205+
206+
const result = await reparentJobs('parent-1', ['j1']);
207+
208+
expect(result).toBe(true);
209+
});
210+
211+
it('handles API errors gracefully', async () => {
212+
(accessActionApi as jest.Mock).mockRejectedValue(new Error('API Error'));
213+
214+
await reparentJobs('parent-1', ['j1']);
215+
216+
expect(handleError).toHaveBeenCalledWith(
217+
new Error('API Error'),
218+
`Error performing action for: /api/group/action/reparentjobs`
219+
);
220+
});
221+
222+
it('returns false on failure (the group tree relies on this to roll back)', async () => {
223+
(accessActionApi as jest.Mock).mockRejectedValue(new Error('API Error'));
224+
225+
const result = await reparentJobs('parent-1', ['j1']);
226+
227+
expect(result).toBe(false);
228+
});
229+
});
230+
141231
// Testing unpauseJobs with error handling
142232
describe('unpauseJobs', () => {
143233
it('should handle API errors and trigger handleError', async () => {
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright Contributors to the OpenCue Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import {
18+
getShowGroups,
19+
getGroupJobs,
20+
} from '@/app/utils/get_utils';
21+
import { accessGetApi } from '@/app/utils/api_utils';
22+
23+
jest.mock('@/app/utils/api_utils', () => ({
24+
accessGetApi: jest.fn(),
25+
}));
26+
27+
jest.mock('@/app/utils/notify_utils', () => ({
28+
handleError: jest.fn(),
29+
}));
30+
31+
describe('get_utils group helpers', () => {
32+
beforeEach(() => {
33+
jest.clearAllMocks();
34+
});
35+
36+
const mockGroups = [
37+
{ id: 'g1', name: 'root', parentId: '' },
38+
{ id: 'g2', name: 'sub', parentId: 'g1' },
39+
];
40+
41+
describe('getShowGroups', () => {
42+
it('posts to /api/show/getgroups with the show id and returns the groups array', async () => {
43+
(accessGetApi as jest.Mock).mockResolvedValue(mockGroups);
44+
45+
const result = await getShowGroups('show-123');
46+
47+
expect(accessGetApi).toHaveBeenCalledWith(
48+
'/api/show/getgroups',
49+
JSON.stringify({ show: { id: 'show-123' } })
50+
);
51+
expect(result).toEqual(mockGroups);
52+
});
53+
54+
it('returns [] when the API responds with null', async () => {
55+
(accessGetApi as jest.Mock).mockResolvedValue(null);
56+
57+
const result = await getShowGroups('show-123');
58+
59+
expect(result).toEqual([]);
60+
});
61+
});
62+
63+
describe('getGroupJobs', () => {
64+
const mockJobs = [{ id: 'j1', name: 'job-1' }];
65+
66+
it('posts to /api/group/getjobs with the group id and returns the jobs array', async () => {
67+
(accessGetApi as jest.Mock).mockResolvedValue(mockJobs);
68+
69+
const result = await getGroupJobs('group-abc');
70+
71+
expect(accessGetApi).toHaveBeenCalledWith(
72+
'/api/group/getjobs',
73+
JSON.stringify({ group: { id: 'group-abc' } })
74+
);
75+
expect(result).toEqual(mockJobs);
76+
});
77+
78+
it('returns [] when the API responds with null', async () => {
79+
(accessGetApi as jest.Mock).mockResolvedValue(null);
80+
81+
const result = await getGroupJobs('group-abc');
82+
83+
expect(result).toEqual([]);
84+
});
85+
});
86+
});

0 commit comments

Comments
 (0)