Fix community image handling in launch instance form
Community images which are not owned by a focused project are not included in the normal image list API (i.e., without specifying "community" as visibility), so we need an extra call of the image list API specifying "community" visibility in a filter. It was originally fixed by [1] but was dropped in [2]. The main motivation of [2] is to address "Duplicates in a repeater are not allowed" error in the JavaScript console. The approach of [2] was to drop multiple glance API calls, but the right solution would be to check duplicated images included in both API calls. This situation with duplicated images happens when a community image is owned by a focused project. In such case, the community image is included in the responses of both API calls. This commit adds a logic to check image IDs already processed. [1] https://review.opendev.org/c/openstack/horizon/+/614688 [2] https://review.opendev.org/c/openstack/horizon/+/640762 Closes-Bug: #1914045 Related-Bug: #1779250 Related-Bug: #1818508 Change-Id: I2ed1b6064ddd6f62818d6112e98e5d5a98beae9d
This commit is contained in:
parent
3fe0802eaa
commit
c9bb0e95ea
|
@ -564,7 +564,12 @@
|
||||||
|
|
||||||
if (enabledImage || enabledSnapshot) {
|
if (enabledImage || enabledSnapshot) {
|
||||||
var filter = {status: 'active', sort_key: 'name', sort_dir: 'asc'};
|
var filter = {status: 'active', sort_key: 'name', sort_dir: 'asc'};
|
||||||
glanceAPI.getImages(filter).then(
|
var filterCommunity = angular.merge({}, filter, {visibility: 'community'});
|
||||||
|
var imagePromises = [
|
||||||
|
glanceAPI.getImages(filter),
|
||||||
|
glanceAPI.getImages(filterCommunity)
|
||||||
|
];
|
||||||
|
$q.all(imagePromises).then(
|
||||||
function(data) {
|
function(data) {
|
||||||
onGetImageSources(data, enabledImage, enabledSnapshot);
|
onGetImageSources(data, enabledImage, enabledSnapshot);
|
||||||
}
|
}
|
||||||
|
@ -670,13 +675,21 @@
|
||||||
model.imageSnapshots.length = 0;
|
model.imageSnapshots.length = 0;
|
||||||
model.images.length = 0;
|
model.images.length = 0;
|
||||||
|
|
||||||
angular.forEach(data.data.items, function(image) {
|
var imageIdsProcessed = [];
|
||||||
if (isValidSnapshot(image) && enabledSnapshot) {
|
|
||||||
model.imageSnapshots.push(image);
|
angular.forEach(data, function(data) {
|
||||||
} else if (isValidImage(image) && enabledImage) {
|
angular.forEach(data.data.items, function(image) {
|
||||||
image.name_or_id = image.name || image.id;
|
if (imageIdsProcessed.includes(image.id)) {
|
||||||
model.images.push(image);
|
return;
|
||||||
}
|
}
|
||||||
|
imageIdsProcessed.push(image.id);
|
||||||
|
if (isValidSnapshot(image) && enabledSnapshot) {
|
||||||
|
model.imageSnapshots.push(image);
|
||||||
|
} else if (isValidImage(image) && enabledImage) {
|
||||||
|
image.name_or_id = image.name || image.id;
|
||||||
|
model.images.push(image);
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
if (enabledImage) {
|
if (enabledImage) {
|
||||||
|
|
|
@ -130,19 +130,35 @@
|
||||||
|
|
||||||
beforeEach(module(function($provide) {
|
beforeEach(module(function($provide) {
|
||||||
$provide.value('horizon.app.core.openstack-service-api.glance', {
|
$provide.value('horizon.app.core.openstack-service-api.glance', {
|
||||||
getImages: function () {
|
getImages: function (params) {
|
||||||
var images = [
|
var images;
|
||||||
{container_format: 'aki', properties: {}},
|
if (params.visibility === 'community') {
|
||||||
{container_format: 'ari', properties: {}},
|
images = [
|
||||||
{container_format: 'ami', properties: {}, name: 'ami_image'},
|
{id: '10', container_format: 'raw', properties: {image_type: 'image'},
|
||||||
{container_format: 'raw', properties: {}, name: 'raw_image'},
|
visibility: 'community'},
|
||||||
{container_format: 'ami', properties: {image_type: 'image'}, id: '1'},
|
// ID 6 is already returned by the first call (below), so this should be ignored.
|
||||||
{container_format: 'raw', properties: {image_type: 'image'}, id: '2'},
|
// To clarify the difference, the content here is different intentionally.
|
||||||
{container_format: 'ami', properties: {
|
{id: '6', container_format: 'raw', properties: {image_type: 'image'},
|
||||||
block_device_mapping: '[{"source_type": "snapshot"}]'}},
|
visibility: 'community'},
|
||||||
{container_format: 'raw', properties: {
|
];
|
||||||
block_device_mapping: '[{"source_type": "snapshot"}]'}}
|
} else {
|
||||||
];
|
images = [
|
||||||
|
// container_format aki and ari are not considered as bootable
|
||||||
|
{id: '1', container_format: 'aki', properties: {}},
|
||||||
|
{id: '2', container_format: 'ari', properties: {}},
|
||||||
|
// The following images are considered as "image" sources.
|
||||||
|
{id: '3', container_format: 'ami', properties: {}, name: 'ami_image'},
|
||||||
|
{id: '4', container_format: 'raw', properties: {}, name: 'raw_image'},
|
||||||
|
{id: '5', container_format: 'ami', properties: {image_type: 'image'}},
|
||||||
|
{id: '6', container_format: 'raw', properties: {image_type: 'image'}},
|
||||||
|
// The following images are considered as "snapshot" sources.
|
||||||
|
{id: '7', container_format: 'ami',
|
||||||
|
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}},
|
||||||
|
{id: '8', container_format: 'raw',
|
||||||
|
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}},
|
||||||
|
{id: '9', container_format: 'raw', properties: {image_type: 'snapshot'}},
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
var deferred = $q.defer();
|
var deferred = $q.defer();
|
||||||
deferred.resolve({data: {items: images}});
|
deferred.resolve({data: {items: images}});
|
||||||
|
@ -371,22 +387,23 @@
|
||||||
expect(model.newInstanceSpec).toBeDefined();
|
expect(model.newInstanceSpec).toBeDefined();
|
||||||
|
|
||||||
var expectedImages = [
|
var expectedImages = [
|
||||||
{container_format: 'ami', properties: {}, name: 'ami_image', name_or_id: 'ami_image'},
|
{id: '3', container_format: 'ami', properties: {}, name: 'ami_image',
|
||||||
{container_format: 'raw', properties: {}, name: 'raw_image', name_or_id: 'raw_image'},
|
name_or_id: 'ami_image'},
|
||||||
{container_format: 'ami', properties: {image_type: 'image'}, id: '1', name_or_id: '1'},
|
{id: '4', container_format: 'raw', properties: {}, name: 'raw_image',
|
||||||
{container_format: 'raw', properties: {image_type: 'image'}, id: '2', name_or_id: '2'}
|
name_or_id: 'raw_image'},
|
||||||
|
{id: '5', container_format: 'ami', properties: {image_type: 'image'}, name_or_id: '5'},
|
||||||
|
{id: '6', container_format: 'raw', properties: {image_type: 'image'}, name_or_id: '6'},
|
||||||
|
{id: '10', container_format: 'raw', properties: {image_type: 'image'}, name_or_id: '10',
|
||||||
|
visibility: 'community'},
|
||||||
];
|
];
|
||||||
expect(model.images).toEqual(expectedImages);
|
expect(model.images).toEqual(expectedImages);
|
||||||
|
|
||||||
var expectedSnapshots = [
|
var expectedSnapshots = [
|
||||||
{
|
{id: '7', container_format: 'ami',
|
||||||
container_format: 'ami',
|
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}},
|
||||||
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}
|
{id: '8', container_format: 'raw',
|
||||||
},
|
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}},
|
||||||
{
|
{id: '9', container_format: 'raw', properties: {image_type: 'snapshot'}},
|
||||||
container_format: 'raw',
|
|
||||||
properties: {block_device_mapping: '[{"source_type": "snapshot"}]'}
|
|
||||||
}
|
|
||||||
];
|
];
|
||||||
expect(model.imageSnapshots).toEqual(expectedSnapshots);
|
expect(model.imageSnapshots).toEqual(expectedSnapshots);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue