Skip to content

Commit ce349d8

Browse files
andrijapanicsbandrijapanicsb
committed
Fix LINSTOR import pagination argument order
Pass offset before limit for resource definitions, resource views and node queries. Correct the matching test stubs and verify three-page traversal, late-page satellite discovery and busy replicas. Add real SDK query-parameter contract tests without controller access.
1 parent 01058cd commit ce349d8

3 files changed

Lines changed: 154 additions & 26 deletions

File tree

‎plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorImportHelper.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private static List<ResourceDefinition> definitions(DevelopersApi api, String pa
6464
List<String> names = path == null ? Collections.emptyList() : Collections.singletonList(resourceName(path));
6565
List<ResourceDefinition> result = new ArrayList<>();
6666
for (int offset = 0; ; offset += PAGE_SIZE) {
67-
List<ResourceDefinition> page = api.resourceDefinitionList(names, true, null, PAGE_SIZE, offset);
67+
List<ResourceDefinition> page = api.resourceDefinitionList(names, true, null, offset, PAGE_SIZE);
6868
if (page == null) {
6969
throw new CloudRuntimeException("Cannot read LINSTOR resource definitions");
7070
}
@@ -103,7 +103,7 @@ private static List<ResourceWithVolumes> resources(DevelopersApi api, String pat
103103
List<ResourceWithVolumes> result = new ArrayList<>();
104104
for (int offset = 0; ; offset += PAGE_SIZE) {
105105
List<ResourceWithVolumes> page = api.viewResources(Collections.emptyList(), names,
106-
Collections.emptyList(), null, PAGE_SIZE, offset);
106+
Collections.emptyList(), null, offset, PAGE_SIZE);
107107
if (page == null) {
108108
throw new CloudRuntimeException("Cannot read LINSTOR resource usage");
109109
}
@@ -122,7 +122,7 @@ private static boolean unavailable(List<String> flags) {
122122
private static Set<String> onlineNodes(DevelopersApi api) throws ApiException {
123123
Set<String> result = new HashSet<>();
124124
for (int offset = 0; ; offset += PAGE_SIZE) {
125-
List<Node> page = api.nodeList(Collections.emptyList(), Collections.emptyList(), PAGE_SIZE, offset);
125+
List<Node> page = api.nodeList(Collections.emptyList(), Collections.emptyList(), offset, PAGE_SIZE);
126126
if (page == null) {
127127
throw new CloudRuntimeException("Cannot verify LINSTOR satellite connectivity");
128128
}

‎plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorApiPaginationContractTest.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.storage.datastore.util;
18+
19+
import java.util.Collections;
20+
21+
import com.linbit.linstor.api.ApiClient;
22+
import com.linbit.linstor.api.DevelopersApi;
23+
import org.junit.Before;
24+
import org.junit.Test;
25+
26+
import static org.mockito.ArgumentMatchers.any;
27+
import static org.mockito.ArgumentMatchers.anyString;
28+
import static org.mockito.Mockito.mock;
29+
import static org.mockito.Mockito.times;
30+
import static org.mockito.Mockito.verify;
31+
import static org.mockito.Mockito.when;
32+
33+
/**
34+
* Check pagination parameter names using the real SDK, without contacting a controller.
35+
* Helper mocks alone cannot detect a mistaken interpretation of the SDK argument order.
36+
*/
37+
public class LinstorApiPaginationContractTest {
38+
private ApiClient client;
39+
private DevelopersApi api;
40+
41+
@Before
42+
public void setup() {
43+
client = mock(ApiClient.class);
44+
when(client.parameterToPairs(anyString(), anyString(), any())).thenReturn(Collections.emptyList());
45+
api = new DevelopersApi(client);
46+
}
47+
48+
private void verifyPaginationQueryParameters() {
49+
verify(client).parameterToPairs("", "offset", 0);
50+
verify(client).parameterToPairs("", "offset", 2000);
51+
verify(client, times(2)).parameterToPairs("", "limit", 1000);
52+
}
53+
54+
@Test
55+
public void resourceDefinitionListUsesOffsetThenLimit() throws Exception {
56+
api.resourceDefinitionList(Collections.emptyList(), true, null, 0, 1000);
57+
api.resourceDefinitionList(Collections.emptyList(), true, null, 2000, 1000);
58+
verifyPaginationQueryParameters();
59+
}
60+
61+
@Test
62+
public void viewResourcesUsesOffsetThenLimit() throws Exception {
63+
api.viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 0, 1000);
64+
api.viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 2000, 1000);
65+
verifyPaginationQueryParameters();
66+
}
67+
68+
@Test
69+
public void nodeListUsesOffsetThenLimit() throws Exception {
70+
api.nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
71+
api.nodeList(Collections.emptyList(), Collections.emptyList(), 2000, 1000);
72+
verifyPaginationQueryParameters();
73+
}
74+
}

‎plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorImportHelperTest.java‎

Lines changed: 77 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class LinstorImportHelperTest {
5050
@Before
5151
public void setup() throws Exception {
5252
api = mock(DevelopersApi.class);
53-
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 0))
53+
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000))
5454
.thenReturn(Collections.singletonList(new Node().name("remote-node").connectionStatus(Node.ConnectionStatusEnum.ONLINE)));
5555
}
5656

@@ -69,8 +69,8 @@ private ResourceWithVolumes resource(String name, Boolean inUse) {
6969
}
7070

7171
private void respond(List<ResourceDefinition> definitions, List<ResourceWithVolumes> resources) throws Exception {
72-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0))).thenReturn(definitions);
73-
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(1000), eq(0))).thenReturn(resources);
72+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000))).thenReturn(definitions);
73+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(0), eq(1000))).thenReturn(resources);
7474
}
7575

7676
private List<VolumeOnStorageTO> list(String path) {
@@ -88,19 +88,19 @@ public void listingIncludesRemoteVolumesDeduplicatesReplicasAndFiltersGroup() th
8888
assertEquals("RAW", volumes.get(0).getFormat());
8989
assertEquals(1024L * 1024L, volumes.get(0).getVirtualSize());
9090
assertEquals("false", volumes.get(0).getDetails().get(VolumeOnStorageTO.Detail.IS_LOCKED));
91-
verify(api).resourceDefinitionList(Collections.emptyList(), true, null, 1000, 0);
92-
verify(api).viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 1000, 0);
93-
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 0);
91+
verify(api).resourceDefinitionList(Collections.emptyList(), true, null, 0, 1000);
92+
verify(api).viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 0, 1000);
93+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
9494
verifyNoMoreInteractions(api); // In particular: no make-available, delete or property changes.
9595
}
9696

9797
@Test
9898
public void singleVolumeIsReadOnlyAndUsesExactResourceName() throws Exception {
9999
respond(Collections.singletonList(definition("x", "rg-ssd")), Collections.singletonList(resource("x", false)));
100100
assertEquals("x", list("x").get(0).getPath());
101-
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 1000, 0);
102-
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 1000, 0);
103-
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 0);
101+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
102+
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 0, 1000);
103+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
104104
verifyNoMoreInteractions(api);
105105
}
106106

@@ -133,7 +133,7 @@ public void noReplicasLocksVolume() throws Exception {
133133
@Test
134134
public void offlineSatelliteCannotReportAFreeVolume() throws Exception {
135135
respond(Collections.singletonList(definition("x", "rg-ssd")), Collections.singletonList(resource("x", false)));
136-
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 0)).thenReturn(Collections.emptyList());
136+
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000)).thenReturn(Collections.emptyList());
137137
assertEquals("true", list("x").get(0).getDetails().get(VolumeOnStorageTO.Detail.IS_LOCKED));
138138
}
139139

@@ -154,42 +154,42 @@ public void deletingResourceIsNotImportable() throws Exception {
154154

155155
@Test
156156
public void wrongGroupIsRejectedBeforeReadingUsage() throws Exception {
157-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0)))
157+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
158158
.thenReturn(Collections.singletonList(definition("x", "rg-hdd")));
159159
CloudRuntimeException error = assertThrows(CloudRuntimeException.class, () -> list("x"));
160160
assertTrue(error.getMessage().contains("rg-hdd"));
161161
assertTrue(error.getMessage().contains("rg-ssd"));
162-
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 1000, 0);
162+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
163163
verifyNoMoreInteractions(api);
164164
}
165165

166166
@Test
167167
public void managerValidationRejectsWrongGroupEvenWithoutRunningVm() throws Exception {
168-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0)))
168+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
169169
.thenReturn(Collections.singletonList(definition("x", "rg-hdd")));
170170
assertThrows(CloudRuntimeException.class, () -> LinstorImportHelper.validateResourceGroup(api, "rg-ssd", "x"));
171-
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 1000, 0);
171+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
172172
verifyNoMoreInteractions(api);
173173
}
174174

175175
@Test
176176
public void managerValidationAcceptsMatchingGroup() throws Exception {
177-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0)))
177+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
178178
.thenReturn(Collections.singletonList(definition("x", "rg-ssd")));
179179
LinstorImportHelper.validateResourceGroup(api, "rg-ssd", "x");
180-
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 1000, 0);
180+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
181181
verifyNoMoreInteractions(api);
182182
}
183183

184184
@Test
185185
public void missingDefinitionIsRejected() throws Exception {
186-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0))).thenReturn(Collections.emptyList());
186+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000))).thenReturn(Collections.emptyList());
187187
assertThrows(CloudRuntimeException.class, () -> list("x"));
188188
}
189189

190190
@Test
191191
public void unavailableControllerFailsClosed() throws Exception {
192-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0)))
192+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
193193
.thenThrow(new ApiException("unreachable"));
194194
assertThrows(CloudRuntimeException.class, () -> list("x"));
195195
assertThrows(CloudRuntimeException.class, () -> LinstorImportHelper.validateResourceGroup(api, "rg-ssd", "x"));
@@ -203,7 +203,7 @@ public void nullUsageResponseIsAnErrorNotAnEmptySuccessfulListing() throws Excep
203203

204204
@Test
205205
public void nullDefinitionsFailClosed() throws Exception {
206-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0))).thenReturn(null);
206+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000))).thenReturn(null);
207207
assertThrows(CloudRuntimeException.class, () -> list("x"));
208208
}
209209

@@ -236,17 +236,71 @@ public void sizeOverflowFailsClosed() throws Exception {
236236
}
237237

238238
@Test
239-
public void batchReadsFollowPagination() throws Exception {
240-
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(0)))
239+
public void batchReadsFollowThreePagesWithConstantLimit() throws Exception {
240+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
241241
.thenReturn(Collections.nCopies(1000, definition("foreign", "rg-hdd")));
242242
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(1000), eq(1000)))
243+
.thenReturn(Collections.nCopies(1000, definition("foreign", "rg-hdd")));
244+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(2000), eq(1000)))
243245
.thenReturn(Collections.singletonList(definition("x", "rg-ssd")));
244-
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(1000), eq(0)))
246+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(0), eq(1000)))
245247
.thenReturn(Collections.nCopies(1000, resource("foreign", false)));
246248
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(1000), eq(1000)))
249+
.thenReturn(Collections.nCopies(1000, resource("foreign", false)));
250+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(2000), eq(1000)))
247251
.thenReturn(Collections.singletonList(resource("x", false)));
248-
assertEquals("x", list(null).get(0).getPath());
252+
253+
List<VolumeOnStorageTO> volumes = list(null);
254+
assertEquals(1, volumes.size());
255+
assertEquals("x", volumes.get(0).getPath());
256+
assertEquals("false", volumes.get(0).getDetails().get(VolumeOnStorageTO.Detail.IS_LOCKED));
257+
verify(api).resourceDefinitionList(Collections.emptyList(), true, null, 0, 1000);
249258
verify(api).resourceDefinitionList(Collections.emptyList(), true, null, 1000, 1000);
259+
verify(api).resourceDefinitionList(Collections.emptyList(), true, null, 2000, 1000);
260+
verify(api).viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 0, 1000);
250261
verify(api).viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 1000, 1000);
262+
verify(api).viewResources(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), null, 2000, 1000);
263+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
264+
verifyNoMoreInteractions(api);
265+
}
266+
267+
@Test
268+
public void onlineSatelliteOnThirdPageAllowsImport() throws Exception {
269+
respond(Collections.singletonList(definition("x", "rg-ssd")), Collections.singletonList(resource("x", false)));
270+
Node unrelatedNode = new Node().name("unrelated-node").connectionStatus(Node.ConnectionStatusEnum.ONLINE);
271+
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000))
272+
.thenReturn(Collections.nCopies(1000, unrelatedNode));
273+
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 1000))
274+
.thenReturn(Collections.nCopies(1000, unrelatedNode));
275+
when(api.nodeList(Collections.emptyList(), Collections.emptyList(), 2000, 1000))
276+
.thenReturn(Collections.singletonList(new Node().name("remote-node").connectionStatus(Node.ConnectionStatusEnum.ONLINE)));
277+
278+
assertEquals("false", list("x").get(0).getDetails().get(VolumeOnStorageTO.Detail.IS_LOCKED));
279+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
280+
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 0, 1000);
281+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
282+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 1000, 1000);
283+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 2000, 1000);
284+
verifyNoMoreInteractions(api);
285+
}
286+
287+
@Test
288+
public void busyReplicaOnThirdPageStillLocksVolume() throws Exception {
289+
when(api.resourceDefinitionList(anyList(), eq(true), isNull(), eq(0), eq(1000)))
290+
.thenReturn(Collections.singletonList(definition("x", "rg-ssd")));
291+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(0), eq(1000)))
292+
.thenReturn(Collections.nCopies(1000, resource("x", false)));
293+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(1000), eq(1000)))
294+
.thenReturn(Collections.nCopies(1000, resource("x", false)));
295+
when(api.viewResources(anyList(), anyList(), anyList(), isNull(), eq(2000), eq(1000)))
296+
.thenReturn(Collections.singletonList(resource("x", true)));
297+
298+
assertEquals("true", list("x").get(0).getDetails().get(VolumeOnStorageTO.Detail.IS_LOCKED));
299+
verify(api).resourceDefinitionList(Collections.singletonList("cs-x"), true, null, 0, 1000);
300+
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 0, 1000);
301+
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 1000, 1000);
302+
verify(api).viewResources(Collections.emptyList(), Collections.singletonList("cs-x"), Collections.emptyList(), null, 2000, 1000);
303+
verify(api).nodeList(Collections.emptyList(), Collections.emptyList(), 0, 1000);
304+
verifyNoMoreInteractions(api);
251305
}
252306
}

0 commit comments

Comments
 (0)