You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "BryanMLima (via GitHub)" <gi...@apache.org> on 2023/09/27 15:51:08 UTC
[GitHub] [cloudstack] BryanMLima opened a new pull request, #7659: Add dynamic secondary storage selection
BryanMLima opened a new pull request, #7659:
URL: https://github.com/apache/cloudstack/pull/7659
### Description
This PR aims to add the possibility to direct resources (Volumes, Templates, Snapshots and ISOs) to a specific secondary storage through rules written in JavaScript that will only affect new allocated resources. This feature is fully described in issue #7654.
Fixes: #7654
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [x] Major
- [ ] Minor
### How Has This Been Tested?
Unit tests were added to validate the functionality of the implemented methods. Besides this, the overall functionality was tested in a local lab with two secondary storage available, creating basic JS rules and validating that the resource was allocated according to the heuristic rule. Below, there are some examples used in the tests, based on the variables available for each resource type specified in the specification of the feature proposal.
1. Allocate a resource type to a specific secondary storage.
```js
function findStorageWithSpecificUuid(pool) {
return pool.uuid === '7432f961-c602-4e8e-8580-2496ffbbc45d';
}
secondaryStorages.filter(findStorageWithSpecificUuid)[0].uuid
```
2. Dedicate storage pools for a type of template format.
```js
function directToDedicatedQCOW2Pool(pool) {
return pool.uuid === '7432f961-c602-4e8e-8580-2496ffbbc45d';
}
function directToDedicatedVHDPool(pool) {
return pool.uuid === '1ea0109a-299d-4e37-8460-3e9823f9f25c';
}
if (template.format === 'QCOW2') {
secondaryStorages.filter(directToDedicatedQCOW2Pool)[0].uuid
} else if (template.format === 'VHD') {
secondaryStorages.filter(directToDedicatedVHDPool)[0].uuid
}
```
3. Direct snapshot of volumes with the KVM hypervisor to a specific secondary storage.
```js
if (snapshot.hypervisorType === 'KVM') {
'7432f961-c602-4e8e-8580-2496ffbbc45d';
}
```
4. Direct resources to a specific domain:
```js
if (account.domain.id == '52d83793-26de-11ec-8dcf-5254005dcdac') {
'1ea0109a-299d-4e37-8460-3e9823f9f25c'
} else if (account.domain.id == 'c1186146-5ceb-4901-94a1-dd1d24bd849d') {
'7432f961-c602-4e8e-8580-2496ffbbc45d'
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236517217
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java:
##########
@@ -0,0 +1,44 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+import com.cloud.storage.Storage;
+
+public class Volume extends GenericHeuristicPresetVariable {
+
+ private Long size;
+
+ private Storage.ImageFormat format;
+
+ public Long getSize() {
+ return size;
+ }
+
+ public void setSize(Long size) {
+ this.size = size;
+ fieldNamesToIncludeInToString.add("size");
+ }
+
+ public Storage.ImageFormat getFormat() {
+ return format;
+ }
+
+ public void setFormat(Storage.ImageFormat format) {
+ this.format = format;
+ fieldNamesToIncludeInToString.add("format");
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1665213690
@blueorangutan test rocky8 kvm-rocky8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836217565
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1389664545
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -1882,6 +1894,65 @@ public StoragePool syncStoragePool(SyncStoragePoolCmd cmd) {
return (PrimaryDataStoreInfo) _dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
}
+
+ @Override
+ public Heuristic createSecondaryStorageHeuristic(CreateSecondaryStorageSelectorCmd cmd) {
+ String name = cmd.getName();
+ String description = cmd.getDescription();
+ long zoneId = cmd.getZoneId();
+ String heuristicRule = cmd.getHeuristicRule();
+ String purpose = cmd.getPurpose();
+ HeuristicPurpose formattedPurpose = EnumUtils.getEnumIgnoreCase(HeuristicPurpose.class, purpose);
+
+ if (formattedPurpose == null) {
+ throw new IllegalArgumentException(String.format("The given heuristic purpose [%s] is not valid for creating a new secondary storage selector." +
+ " The valid options are %s.", purpose, Arrays.asList(HeuristicPurpose.values())));
+ }
+
+ HeuristicVO heuristic = secondaryStorageHeuristicDao.findByZoneIdAndPurpose(zoneId, formattedPurpose);
Review Comment:
As mentioned, the method `findOneIncludingRemovedBy` only retrieves non-removed entries; therefore, this scenario does not occur.
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.dao;
+
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicPurpose;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class SecondaryStorageHeuristicDaoImpl extends GenericDaoBase<HeuristicVO, Long> implements SecondaryStorageHeuristicDao {
+ private SearchBuilder<HeuristicVO> zoneAndPurposeSearch;
+
+ @PostConstruct
+ public void init() {
+ zoneAndPurposeSearch = createSearchBuilder();
+ zoneAndPurposeSearch.and("zoneId", zoneAndPurposeSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
+ zoneAndPurposeSearch.and("purpose", zoneAndPurposeSearch.entity().getPurpose(), SearchCriteria.Op.IN);
+ zoneAndPurposeSearch.done();
+ }
+
+ @Override
+ public HeuristicVO findByZoneIdAndPurpose(long zoneId, HeuristicPurpose purpose) {
+ SearchCriteria<HeuristicVO> searchCriteria = zoneAndPurposeSearch.create();
+ searchCriteria.setParameters("zoneId", zoneId);
+ searchCriteria.setParameters("purpose", purpose.toString());
+ final Filter filter = new Filter(HeuristicVO.class, "created", false);
+
+ return findOneIncludingRemovedBy(searchCriteria, filter);
Review Comment:
The method name is wrong, its behaviour is as expected, only retrieving heuristic rules with removed null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1780527465
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1745545490
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1746280051
<b>[SF] Trillian test result (tid-7827)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42819 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7827-kvm-centos7.zip
Smoke tests completed. 113 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1806328867
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7710
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836125541
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1668695103
<b>[SF] Trillian test result (tid-7311)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 52371 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7311-kvm-centos7.zip
Smoke tests completed. 111 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 3661.62 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 109.25 | test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume | `Error` | 81.86 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 54.91 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1666269920
<b>[SF] Trillian test result (tid-7271)</b>
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 51482 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7271-kvm-rocky8.zip
Smoke tests completed. 110 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
test_01_vpc_site2site_vpn | `Failure` | 31.12 | test_vpc_vpn.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1664194363
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6653
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1680402347
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600265675
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#7659](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (397a3aa) into [main](https://app.codecov.io/gh/apache/cloudstack/commit/e26d49de4da310f071f7fa86537f8e435f36f784?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e26d49d) will **increase** coverage by `0.05%`.
> The diff coverage is `34.88%`.
```diff
@@ Coverage Diff @@
## main #7659 +/- ##
============================================
+ Coverage 13.13% 13.18% +0.05%
- Complexity 9147 9205 +58
============================================
Files 2728 2737 +9
Lines 256997 257262 +265
Branches 40067 40081 +14
============================================
+ Hits 33747 33916 +169
- Misses 219001 219091 +90
- Partials 4249 4255 +6
```
| [Impacted Files](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../main/java/com/cloud/template/TemplateManager.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL2NvbXBvbmVudHMtYXBpL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3RlbXBsYXRlL1RlbXBsYXRlTWFuYWdlci5qYXZh) | `100.00% <ø> (ø)` | |
| [.../org/apache/cloudstack/secstorage/HeuristicVO.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9zZWNzdG9yYWdlL0hldXJpc3RpY1ZPLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...cstorage/dao/SecondaryStorageHeuristicDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9zZWNzdG9yYWdlL2Rhby9TZWNvbmRhcnlTdG9yYWdlSGV1cmlzdGljRGFvSW1wbC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...e/image/manager/ImageStoreProviderManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3N0b3JhZ2UvaW1hZ2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svc3RvcmFnZS9pbWFnZS9tYW5hZ2VyL0ltYWdlU3RvcmVQcm92aWRlck1hbmFnZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...oudstack/storage/snapshot/SnapshotServiceImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3N0b3JhZ2Uvc25hcHNob3Qvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svc3RvcmFnZS9zbmFwc2hvdC9TbmFwc2hvdFNlcnZpY2VJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...dstack/storage/datastore/DataStoreManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3N0b3JhZ2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svc3RvcmFnZS9kYXRhc3RvcmUvRGF0YVN0b3JlTWFuYWdlckltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...src/main/java/com/cloud/api/ApiResponseHelper.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9BcGlSZXNwb25zZUhlbHBlci5qYXZh) | `3.79% <0.00%> (-0.02%)` | :arrow_down: |
| [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | `2.92% <0.00%> (-0.03%)` | :arrow_down: |
| [...in/java/com/cloud/server/ManagementServerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3NlcnZlci9NYW5hZ2VtZW50U2VydmVySW1wbC5qYXZh) | `5.64% <0.00%> (-0.01%)` | :arrow_down: |
| [...ain/java/com/cloud/storage/StorageManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3N0b3JhZ2UvU3RvcmFnZU1hbmFnZXJJbXBsLmphdmE=) | `1.94% <0.00%> (-0.04%)` | :arrow_down: |
| ... and [13 more](https://app.codecov.io/gh/apache/cloudstack/pull/7659?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/7659/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236513128
##########
api/src/main/java/org/apache/cloudstack/secstorage/heuristics/Heuristic.java:
##########
@@ -0,0 +1,40 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.heuristics;
+
+import org.apache.cloudstack.api.Identity;
+import org.apache.cloudstack.api.InternalIdentity;
+
+import java.util.Date;
+
+public interface Heuristic extends InternalIdentity, Identity {
+
+ String getName();
+
+ String getDescription();
+
+ Long getZoneId();
+
+ String getPurpose();
+
+ String getHeuristicRule();
+
+ Date getCreated();
+
+ Date getRemoved();
+
+}
Review Comment:
needs an extra newline
##########
api/src/main/java/org/apache/cloudstack/secstorage/heuristics/HeuristicPurpose.java:
##########
@@ -0,0 +1,25 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.heuristics;
+
+/**
+ * The heuristic purpose used in the allocation process of secondary storage resources.
+ * Valid options are: {@link #ISO}, {@link #SNAPSHOT}, {@link #TEMPLATE} and {@link #VOLUME}
+ */
+public enum HeuristicPurpose {
+ ISO, SNAPSHOT, TEMPLATE, VOLUME
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600341682
hey @BryanMLima , I saw the issue with functional description that came with this. I haven't yet read it and just reacted to the lint output:
~~~
Check hooks apply to the repository......................................Passed
check for case conflicts.................................................Passed
check vcs permalinks.....................................................Passed
fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/PresetVariables.java
Fixing api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java
Fixing api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/UpdateSecondaryStorageSelectorCmd.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java
Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java
Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/HeuristicVO.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java
Fixing api/src/main/java/org/apache/cloudstack/secstorage/heuristics/HeuristicPurpose.java
Fixing api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java
Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDao.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java
Fixing api/src/main/java/org/apache/cloudstack/secstorage/heuristics/Heuristic.java
Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java
mixed line ending........................................................Passed
~~~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836348590
> @blueorangutan rest
Even the CI needs to rest a bit, right? :joy:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1837088656
<b>[SF] Trillian test result (tid-8451)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 58299 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t8451-kvm-centos7.zip
Smoke tests completed. 119 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3611.13 | test_kubernetes_clusters.py
test_02_upgrade_kubernetes_cluster | `Failure` | 3618.21 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_05_basic_lifecycle_kubernetes_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_06_delete_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster | `Failure` | 50.03 | test_kubernetes_clusters.py
test_11_test_unmanaged_cluster_lifecycle | `Error` | 0.24 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 58.74 | test_kubernetes_clusters.py
test_08_migrate_vm | `Error` | 0.05 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392797750
##########
api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java:
##########
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements. See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership. The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License. You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied. See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.response;
+
+import com.cloud.serializer.Param;
+import com.google.gson.annotations.SerializedName;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import java.util.Date;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_TYPE_VALID_OPTIONS;
+
+@EntityReference(value = {Heuristic.class})
+public class SecondaryStorageHeuristicsResponse extends BaseResponse {
+
+ @SerializedName(ApiConstants.ID)
+ @Param(description = "ID of the heuristic.")
+ private String id;
+
+ @SerializedName(ApiConstants.NAME)
+ @Param(description = "Name of the heuristic.")
+ private String name;
+
+ @SerializedName(ApiConstants.DESCRIPTION)
+ @Param(description = "Description of the heuristic.")
+ private String description;
+
+ @SerializedName(ApiConstants.ZONE_ID)
+ @Param(description = "The zone which the heuristic is valid upon.")
+ private String zoneId;
+
+ @SerializedName(ApiConstants.TYPE)
+ @Param(description = "The resource type directed to a specific secondary storage by the selector. " + HEURISTIC_TYPE_VALID_OPTIONS)
+ private String type;
+
+ @SerializedName(ApiConstants.HEURISTIC_RULE)
+ @Param(description = "The heuristic rule, in JavaScript language, used to select a secondary storage to be directed.")
+ private String heuristicRule;
+
+ @SerializedName(ApiConstants.CREATED)
+ @Param(description = "When the heuristic was created.")
+ private Date created;
+
+ @SerializedName(ApiConstants.REMOVED)
+ @Param(description = "When the heuristic was removed.")
+ private Date removed;
+
+
+ public SecondaryStorageHeuristicsResponse(String id, String name, String description, String zoneId, String type, String heuristicRule, Date created, Date removed) {
+ super("heuristics");
+ this.id = id;
+ this.name = name;
+ this.description = description;
+ this.zoneId = zoneId;
+ this.type = type;
+ this.heuristicRule = heuristicRule;
+ this.created = created;
+ this.removed = removed;
Review Comment:
@BryanMLima Do we need a `removed` field in response?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836322732
@blueorangutan rest
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1745397811
@DaanHoogland could you trigger the smoke tests for this one as well? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1375831176
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
+ SecondaryStorageHeuristicsResponse.class, entityType = {Heuristic.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
+
+ @Parameter(name = ApiConstants.NAME, required = true, type = CommandType.STRING, description = "The name identifying the heuristic rule.")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, required = true, type = BaseCmd.CommandType.STRING, description = "The description of the heuristic rule.")
+ private String description;
+
+ @Parameter(name = ApiConstants.ZONE_ID, required = true, entityType = ZoneResponse.class, type = BaseCmd.CommandType.UUID, description = "The zone in which the heuristic " +
+ "rule will be applied.")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.PURPOSE, required = true, type = BaseCmd.CommandType.STRING, description =
+ "The resource type directed to a specific secondary storage by the selector. " + HEURISTIC_PURPOSE_VALID_OPTIONS)
+ private String purpose;
+
+ @Parameter(name = ApiConstants.HEURISTIC_RULE, required = true, type = BaseCmd.CommandType.STRING, description = "The heuristic rule, in JavaScript language. It is required " +
+ "that it returns the UUID of a secondary storage pool.", length = 65535)
Review Comment:
Add an example of what a rule would like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1806216764
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1663948710
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600258309
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6283
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236515886
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java:
##########
@@ -0,0 +1,222 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics;
+
+import com.cloud.api.ApiDBUtils;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StorageStats;
+import com.cloud.storage.VMTemplateVO;
+import com.cloud.storage.VolumeVO;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.dao.SecondaryStorageHeuristicDao;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicPurpose;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
+import org.apache.cloudstack.storage.heuristics.presetvariables.PresetVariables;
+import org.apache.cloudstack.storage.heuristics.presetvariables.SecondaryStorage;
+import org.apache.cloudstack.storage.heuristics.presetvariables.Snapshot;
+import org.apache.cloudstack.storage.heuristics.presetvariables.Template;
+import org.apache.cloudstack.storage.heuristics.presetvariables.Volume;
+import org.apache.cloudstack.utils.jsinterpreter.JsInterpreter;
+import org.apache.log4j.Logger;
+
+import javax.inject.Inject;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class for building and injecting the heuristics preset variables into the JS script.
+ */
+public class HeuristicRuleHelper {
+
+ protected static final Logger LOGGER = Logger.getLogger(HeuristicRuleHelper.class);
+
+ private static final Long HEURISTICS_SCRIPT_TIMEOUT = StorageManager.HEURISTICS_SCRIPT_TIMEOUT.value();
+
+ @Inject
+ private DataStoreManager dataStoreManager;
+
+ @Inject
+ private ImageStoreDao imageStoreDao;
+
+ @Inject
+ private SecondaryStorageHeuristicDao secondaryStorageHeuristicDao;
+
+ /**
+ * Returns the {@link DataStore} object if the zone, specified by the ID, has an active heuristic rule for the given {@link HeuristicPurpose}.
+ * It returns null otherwise.
+ * @param zoneId used to search for the heuristic rules.
+ * @param heuristicPurpose used for checking if there is a heuristic rule for the given {@link HeuristicPurpose}.
+ * @param obj can be from the following classes: {@link VMTemplateVO}, {@link SnapshotInfo} and {@link VolumeVO}.
+ * They are used to retrieve attributes for injecting in the JS rule.
+ * @return the corresponding {@link DataStore} if there is a heuristic rule, returns null otherwise.
+ */
+ public DataStore getImageStoreIfThereIsHeuristicRule(Long zoneId, HeuristicPurpose heuristicPurpose, Object obj) {
+ HeuristicVO heuristicsVO = secondaryStorageHeuristicDao.findByZoneIdAndPurpose(zoneId, heuristicPurpose);
+
+ if (heuristicsVO == null) {
+ LOGGER.debug(String.format("No heuristic rules found for zone with ID [%s] and heuristic purpose [%s]. Returning null.", zoneId, heuristicPurpose));
+ return null;
+ } else {
+ LOGGER.debug(String.format("Found the heuristic rule %s to apply for zone with ID [%s].", heuristicsVO, zoneId));
+ return interpretHeuristicRule(heuristicsVO.getHeuristicRule(), heuristicPurpose, obj, zoneId);
+ }
+ }
+
+ /**
+ * Build the preset variables ({@link Template}, {@link Snapshot} and {@link Volume}) for the JS script.
+ */
+ protected void buildPresetVariables(JsInterpreter jsInterpreter, HeuristicPurpose heuristicPurpose, long zoneId, Object obj) {
+ PresetVariables presetVariables = new PresetVariables();
+
+ switch (heuristicPurpose) {
+ case TEMPLATE:
+ case ISO:
+ presetVariables.setTemplate(setTemplatePresetVariable((VMTemplateVO) obj));
+ break;
+ case SNAPSHOT:
+ presetVariables.setSnapshot(setSnapshotPresetVariable((SnapshotInfo) obj));
+ break;
+ case VOLUME:
+ presetVariables.setVolume(setVolumePresetVariable((VolumeVO) obj));
+ break;
+ }
+ presetVariables.setSecondaryStorages(setSecondaryStoragesVariable(zoneId));
+
+ injectPresetVariables(jsInterpreter, presetVariables);
+ }
+
+ /**
+ * Inject the {@link PresetVariables} ({@link Template}, {@link Snapshot} and {@link Volume}) into the JS {@link JsInterpreter}.
+ * For each type, they can be accessed using the following variables in the script:
+ * <ul>
+ * <li>ISO/Template: using the variable <b>iso</b> or <b>template</b>, e.g. <b>iso.format</b> </li>
+ * <li>Snapshot: using the variable <b>snapshot</b>, e.g. <b>snapshot.size</b></li>
+ * <li>Volume: using the variable <b>volume</b>, e.g. <b>volume.format</b></li>
+ * </ul>
+ * @param jsInterpreter the JS script
+ * @param presetVariables used for injecting in the JS interpreter.
+ */
+ protected void injectPresetVariables(JsInterpreter jsInterpreter, PresetVariables presetVariables) {
+ jsInterpreter.injectVariable("secondaryStorages", presetVariables.getSecondaryStorages().toString());
+
+ if (presetVariables.getTemplate() != null) {
+ jsInterpreter.injectVariable("template", presetVariables.getTemplate().toString());
+ jsInterpreter.injectVariable("iso", presetVariables.getTemplate().toString());
+ }
+
+ if (presetVariables.getSnapshot() != null) {
+ jsInterpreter.injectVariable("snapshot", presetVariables.getSnapshot().toString());
+ }
+
+ if (presetVariables.getVolume() != null) {
+ jsInterpreter.injectVariable("volume", presetVariables.getVolume().toString());
+ }
+ }
+
+ protected List<SecondaryStorage> setSecondaryStoragesVariable(long zoneId) {
+ List<SecondaryStorage> secondaryStorageList = new ArrayList<>();
+ List<ImageStoreVO> imageStoreVOS = imageStoreDao.listStoresByZoneId(zoneId);
+
+ for (ImageStoreVO imageStore : imageStoreVOS) {
+ SecondaryStorage secondaryStorage = new SecondaryStorage();
+
+ secondaryStorage.setUuid(imageStore.getUuid());
+ secondaryStorage.setProtocol(imageStore.getProtocol());
+ StorageStats storageStats = ApiDBUtils.getSecondaryStorageStatistics(imageStore.getId());
+
+ if (storageStats != null) {
+ secondaryStorage.setUsedDiskSize(storageStats.getByteUsed());
+ secondaryStorage.setTotalDiskSize(storageStats.getCapacityBytes());
+ }
+
+ secondaryStorageList.add(secondaryStorage);
+ }
+ return secondaryStorageList;
+ }
+
+ protected Template setTemplatePresetVariable(VMTemplateVO templateVO) {
+ Template template = new Template();
+
+ template.setFormat(templateVO.getFormat());
+ template.setHypervisorType(templateVO.getHypervisorType());
+
+ return template;
+ }
+
+ protected Volume setVolumePresetVariable(VolumeVO volumeVO) {
+ Volume volume = new Volume();
+
+ volume.setFormat(volumeVO.getFormat());
+ volume.setSize(volumeVO.getSize());
+
+ return volume;
+ }
+
+ protected Snapshot setSnapshotPresetVariable(SnapshotInfo snapshotInfo) {
+ Snapshot snapshot = new Snapshot();
+
+ snapshot.setSize(snapshotInfo.getSize());
+ snapshot.setHypervisorType(snapshotInfo.getHypervisorType());
+
+ return snapshot;
+ }
+
+ /**
+ * This method calls {@link HeuristicRuleHelper#buildPresetVariables(JsInterpreter, HeuristicPurpose, long, Object)} for building the preset variables and
+ * execute the JS script specified in the <b>rule</b> ({@link String}) parameter. The script is pre-injected with the preset variables, to allow the JS script to reference them
+ * in the code scope.
+ * <br>
+ * <br>
+ * The JS script needs to return a valid UUID ({@link String}) of a secondary storage, otherwise a {@link CloudRuntimeException} is thrown.
+ * @param rule the {@link String} representing the JS script.
+ * @param heuristicPurpose used for building the preset variables accordingly to the {@link HeuristicPurpose} specified.
+ * @param obj can be from the following classes: {@link VMTemplateVO}, {@link SnapshotInfo} and {@link VolumeVO}.
+ * They are used to retrieve attributes for injecting in the JS rule.
+ * @param zoneId used for injecting the {@link SecondaryStorage} preset variables.
+ * @return the {@link DataStore} returned by the script.
+ */
+ public DataStore interpretHeuristicRule(String rule, HeuristicPurpose heuristicPurpose, Object obj, long zoneId) {
+ try (JsInterpreter jsInterpreter = new JsInterpreter(HEURISTICS_SCRIPT_TIMEOUT)) {
+ buildPresetVariables(jsInterpreter, heuristicPurpose, zoneId, obj);
+ Object scriptReturn = jsInterpreter.executeScript(rule);
+
+ if (!(scriptReturn instanceof String)) {
+ throw new CloudRuntimeException(String.format("Error while interpreting heuristic rule [%s], the rule did not return a String.", rule));
+ }
+
+ DataStore dataStore = dataStoreManager.getImageStoreByUuid((String) scriptReturn);
+
+ if (dataStore == null) {
+ throw new CloudRuntimeException(String.format("Unable to find a secondary storage with the UUID [%s] returned by the heuristic rule [%s]. Check if the rule is " +
+ "returning a valid UUID.", scriptReturn, rule));
+ }
+
+ return dataStore;
+ } catch (IOException ex) {
+ String message = String.format("Error while executing script [%s].", rule);
+ LOGGER.error(message, ex);
+ throw new CloudRuntimeException(message, ex);
+ }
+ }
+
+}
Review Comment:
needs an extra newline
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java:
##########
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public class GenericHeuristicPresetVariable {
+
+ protected transient Set<String> fieldNamesToIncludeInToString = new HashSet<>();
+
+ @Override
+ public String toString() {
+ return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, fieldNamesToIncludeInToString.toArray(new String[0]));
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1665077146
@blueorangutan test matrix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1680400598
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1680055184
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1680130514
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6790
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236512193
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
+ SecondaryStorageHeuristicsResponse.class, entityType = {Heuristic.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
+
+ @Parameter(name = ApiConstants.NAME, required = true, type = CommandType.STRING, description = "The name identifying the heuristic rule.")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, required = true, type = BaseCmd.CommandType.STRING, description = "The description of the heuristic rule.")
+ private String description;
+
+ @Parameter(name = ApiConstants.ZONE_ID, required = true, entityType = ZoneResponse.class, type = BaseCmd.CommandType.UUID, description = "The zone in which the heuristic " +
+ "rule will be applied.")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.PURPOSE, required = true, type = BaseCmd.CommandType.STRING, description =
+ "The resource type directed to a specific secondary storage by the selector. " + HEURISTIC_PURPOSE_VALID_OPTIONS)
+ private String purpose;
+
+ @Parameter(name = ApiConstants.HEURISTIC_RULE, required = true, type = BaseCmd.CommandType.STRING, description = "The heuristic rule, in JavaScript language. It is required " +
+ "that it returns the UUID of a secondary storage pool.", length = 65535)
+ private String heuristicRule;
+
+ public String getName() {
+ return name;
+ }
+
+ public String getDescription() {
+ return description;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ public String getPurpose() {
+ return purpose;
+ }
+
+ public String getHeuristicRule() {
+ return heuristicRule;
+ }
+
+ @Override
+ public void execute() {
+ Heuristic heuristic = _storageService.createSecondaryStorageHeuristic(this);
+
+ if (heuristic == null) {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create a secondary storage selector.");
+ }
+
+ SecondaryStorageHeuristicsResponse response = _responseGenerator.createSecondaryStorageSelectorResponse(heuristic);
+ response.setResponseName(getCommandName());
+ this.setResponseObject(response);
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return Account.ACCOUNT_ID_SYSTEM;
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392760554
##########
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java:
##########
@@ -907,6 +907,15 @@ public T findOneBy(SearchCriteria<T> sc) {
return findOneIncludingRemovedBy(sc);
}
+ @Override
+ @DB()
+ public T findOneBy(SearchCriteria<T> sc, final Filter filter) {
Review Comment:
Since we are fetching results including removed entries, this should be renamed as `findOneIncludingRemovedBy`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392771399
##########
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java:
##########
@@ -907,6 +907,15 @@ public T findOneBy(SearchCriteria<T> sc) {
return findOneIncludingRemovedBy(sc);
}
+ @Override
+ @DB()
+ public T findOneBy(SearchCriteria<T> sc, final Filter filter) {
Review Comment:
Or use search in #L915.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1787410589
@BryanMLima , can you answer @vishesh92 's comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836353896
@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1673179489
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836317376
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7892
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1788683767
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1667300869
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1774559173
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1737455423
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7129
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] BryanMLima commented on pull request #7659: Add dynamic secondary storage selection
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1737319200
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600090973
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236514444
##########
engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql:
##########
@@ -137,3 +137,18 @@ UPDATE `cloud`.`console_session` SET removed=now();
ALTER TABLE `cloud`.`console_session` DROP `acquired`, ADD `acquired` datetime COMMENT 'When the session was acquired' AFTER `host_id`;
-- create_public_parameter_on_roles. #6960
ALTER TABLE `cloud`.`roles` ADD COLUMN `public_role` tinyint(1) NOT NULL DEFAULT '1' COMMENT 'Indicates whether the role will be visible to all users (public) or only to root admins (private). If this parameter is not specified during the creation of the role its value will be defaulted to true (public).';
+
+-- Create heuristic table for dynamic allocating resources to the secondary storage
+CREATE TABLE IF NOT EXISTS `cloud`.`heuristics` (
+ `id` bigint(20) unsigned NOT NULL auto_increment,
+ `uuid` varchar(255) UNIQUE NOT NULL,
+ `name` text NOT NULL,
+ `description` text DEFAULT NULL,
+ `zone_id` bigint(20) unsigned NOT NULL COMMENT 'ID of the zone to apply the heuristic, foreign key to `data_center` table',
+ `purpose` varchar(255) NOT NULL,
+ `heuristic_rule` text NOT NULL COMMENT 'JS script that defines to which secondary storage the resource will be allocated.',
+ `created` datetime NOT NULL,
+ `removed` datetime DEFAULT NULL,
+ PRIMARY KEY (`id`),
+ CONSTRAINT `fk_heuristics__zone_id` FOREIGN KEY(`zone_id`) REFERENCES `cloud`.`data_center`(`id`)
+);
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600091256
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1606956869
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1812111168
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392788241
##########
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java:
##########
@@ -907,6 +907,15 @@ public T findOneBy(SearchCriteria<T> sc) {
return findOneIncludingRemovedBy(sc);
}
+ @Override
+ @DB()
+ public T findOneBy(SearchCriteria<T> sc, final Filter filter) {
Review Comment:
@vishesh92 we are fetching only non-removed entries here, and the method in line 915 does not limit to one result, as I need. The other method I could use is `findOneBy(final SearchCriteria<T> sc)`, however, I cannot pass an already created filter. That's why I created this method, as I needed to order the results by the `created` column, and limit to only one result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1808141349
> CLGTM, not tested and I think this deserves a few paragraphs in the documentation. Can you consider that @BryanMLima ? I think this is a feature that larger installations will start using as it settles in.
Sure, no problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1811984043
@vishesh92 can you approve of this now?
Anybody besides @BryanMLima (and @blueorangutan ) tested this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "rajujith (via GitHub)" <gi...@apache.org>.
rajujith commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1812133055
@BryanMLima any documentation available that provides some guidance on the usage of this feature?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1745548507
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1680053781
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland closed pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #7659: Add dynamic secondary storage selection
URL: https://github.com/apache/cloudstack/pull/7659
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1663948317
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1666070206
<b>[SF] Trillian test result (tid-7260)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44814 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7260-xenserver-71.zip
Smoke tests completed. 112 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestDeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
test_10_attachAndDetach_iso | `Failure` | 1510.91 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1681357412
<b>[SF] Trillian test result (tid-7436)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39919 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7436-kvm-centos7.zip
Smoke tests completed. 113 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "rajujith (via GitHub)" <gi...@apache.org>.
rajujith commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1838088094
@BryanMLima I updated the rules for scenarios 3) and 4). Now they are working fine.
list secondarystorageselectors zoneid=a2a9fafd-43fb-4508-915e-d3c836a7e972
{
"count": 2,
"heuristics": [
{
"created": "2023-12-04T08:39:01+0000",
"description": "KVMtemplatesonly-specific",
"heuristicrule": "function findStorageWithSpecificId(pool) { return pool.id === '92dc82ff-c3cc-4756-928f-6802c358f599'; } secondaryStorages.filter(findStorageWithSpecificId)[0].id",
"id": "2fa48314-1a01-4d17-a16f-78c4719fdc4c",
"name": "KVMtemplatesonly-specifc",
"type": "TEMPLATE",
"zoneid": "a2a9fafd-43fb-4508-915e-d3c836a7e972"
},
{
"created": "2023-12-04T08:45:23+0000",
"description": "MultiDomain",
"heuristicrule": "if (account.domain.id == '3df7450a-db07-4bfd-a54b-fa0716013917') {'92dc82ff-c3cc-4756-928f-6802c358f599'} else if {account.domain.id == '7db040fa-1ebe-404e-b15c-fbc9c60118d4') {fc9f710e-452f-4233-aa51-91747e5bb071}",
"id": "7757ba9d-34e0-40e6-9e6a-47dac301cadb",
"name": "Multi-Domain-Dedication",
"type": "SNAPSHOT",
"zoneid": "a2a9fafd-43fb-4508-915e-d3c836a7e972"
}
]
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836352622
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236513786
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/HeuristicVO.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage;
+
+import com.cloud.utils.db.GenericDao;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.persistence.Temporal;
+import javax.persistence.TemporalType;
+import java.util.Date;
+import java.util.UUID;
+
+@Entity
+@Table(name = "heuristics")
+public class HeuristicVO implements Heuristic {
+ @Id
+ @GeneratedValue(strategy = GenerationType.IDENTITY)
+ @Column(name = "id", nullable = false)
+ private Long id;
+
+ @Column(name = "uuid", nullable = false)
+ private String uuid = UUID.randomUUID().toString();
+
+ @Column(name = "name")
+ private String name;
+
+ @Column(name = "description")
+ private String description;
+
+ @Column(name = "zone_id", nullable = false)
+ private Long zoneId;
+
+ @Column(name = "purpose", nullable = false)
+ private String purpose;
+
+ @Column(name = "heuristic_rule", nullable = false, length = 65535)
+ private String heuristicRule;
+
+ @Column(name = GenericDao.CREATED_COLUMN, nullable = false)
+ @Temporal(value = TemporalType.TIMESTAMP)
+ private Date created;
+
+ @Column(name = GenericDao.REMOVED_COLUMN)
+ @Temporal(value = TemporalType.TIMESTAMP)
+ private Date removed = null;
+
+ public HeuristicVO() {
+ }
+
+ public HeuristicVO(String name, String description, Long zoneId, String purpose, String heuristicRule, Date created, Date removed) {
+ this.name = name;
+ this.description = description;
+ this.zoneId = zoneId;
+ this.purpose = purpose;
+ this.heuristicRule = heuristicRule;
+ this.created = created;
+ this.removed = removed;
+ }
+
+ @Override
+ public long getId() {
+ return id;
+ }
+
+ public String getUuid() {
+ return uuid;
+ }
+
+ @Override
+ public String getName() {
+ return name;
+ }
+
+ public String getDescription() {
+ return description;
+ }
+
+ public Long getZoneId() {
+ return zoneId;
+ }
+
+ public String getPurpose() {
+ return purpose;
+ }
+
+ public String getHeuristicRule() {
+ return heuristicRule;
+ }
+
+ public Date getCreated() {
+ return created;
+ }
+
+ public Date getRemoved() {
+ return removed;
+ }
+
+ public void setHeuristicRule(String heuristicRule) {
+ this.heuristicRule = heuristicRule;
+ }
+
+ @Override
+ public String toString() {
+ return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "heuristicRule", "purpose");
+ }
+}
Review Comment:
needs an extra newline
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDao.java:
##########
@@ -0,0 +1,26 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.dao;
+
+import com.cloud.utils.db.GenericDao;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicPurpose;
+
+public interface SecondaryStorageHeuristicDao extends GenericDao<HeuristicVO, Long> {
+
+ HeuristicVO findByZoneIdAndPurpose(long zoneId, HeuristicPurpose purpose);
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236516856
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java:
##########
@@ -0,0 +1,44 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+import com.cloud.hypervisor.Hypervisor;
+
+public class Snapshot extends GenericHeuristicPresetVariable {
+
+ private Long size;
+
+ private Hypervisor.HypervisorType hypervisorType;
+
+ public Long getSize() {
+ return size;
+ }
+
+ public void setSize(Long size) {
+ this.size = size;
+ fieldNamesToIncludeInToString.add("size");
+ }
+
+ public Hypervisor.HypervisorType getHypervisorType() {
+ return hypervisorType;
+ }
+
+ public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) {
+ this.hypervisorType = hypervisorType;
+ fieldNamesToIncludeInToString.add("hypervisorType");
+ }
+}
Review Comment:
needs an extra newline
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java:
##########
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.Storage;
+
+public class Template extends GenericHeuristicPresetVariable {
+
+ private Hypervisor.HypervisorType hypervisorType;
+
+ private Storage.ImageFormat format;
+
+ public Hypervisor.HypervisorType getHypervisorType() {
+ return hypervisorType;
+ }
+
+ public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) {
+ this.hypervisorType = hypervisorType;
+ fieldNamesToIncludeInToString.add("hypervisorType");
+ }
+
+ public Storage.ImageFormat getFormat() {
+ return format;
+ }
+
+ public void setFormat(Storage.ImageFormat format) {
+ this.format = format;
+ fieldNamesToIncludeInToString.add("format");
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236514114
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.dao;
+
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicPurpose;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class SecondaryStorageHeuristicDaoImpl extends GenericDaoBase<HeuristicVO, Long> implements SecondaryStorageHeuristicDao {
+ private SearchBuilder<HeuristicVO> zoneAndPurposeSearch;
+
+ @PostConstruct
+ public void init() {
+ zoneAndPurposeSearch = createSearchBuilder();
+ zoneAndPurposeSearch.and("zoneId", zoneAndPurposeSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
+ zoneAndPurposeSearch.and("purpose", zoneAndPurposeSearch.entity().getPurpose(), SearchCriteria.Op.IN);
+ zoneAndPurposeSearch.done();
+ }
+
+ @Override
+ public HeuristicVO findByZoneIdAndPurpose(long zoneId, HeuristicPurpose purpose) {
+ SearchCriteria<HeuristicVO> searchCriteria = zoneAndPurposeSearch.create();
+ searchCriteria.setParameters("zoneId", zoneId);
+ searchCriteria.setParameters("purpose", purpose.toString());
+ final Filter filter = new Filter(HeuristicVO.class, "created", false);
+
+ return findOneIncludingRemovedBy(searchCriteria, filter);
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1390746004
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
+ SecondaryStorageHeuristicsResponse.class, entityType = {Heuristic.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
+
+ @Parameter(name = ApiConstants.NAME, required = true, type = CommandType.STRING, description = "The name identifying the heuristic rule.")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, required = true, type = BaseCmd.CommandType.STRING, description = "The description of the heuristic rule.")
+ private String description;
+
+ @Parameter(name = ApiConstants.ZONE_ID, required = true, entityType = ZoneResponse.class, type = BaseCmd.CommandType.UUID, description = "The zone in which the heuristic " +
+ "rule will be applied.")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.PURPOSE, required = true, type = BaseCmd.CommandType.STRING, description =
Review Comment:
I agree, "type", "target" or maybe "targettype"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1375769078
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
+ SecondaryStorageHeuristicsResponse.class, entityType = {Heuristic.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
+
+ @Parameter(name = ApiConstants.NAME, required = true, type = CommandType.STRING, description = "The name identifying the heuristic rule.")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, required = true, type = BaseCmd.CommandType.STRING, description = "The description of the heuristic rule.")
+ private String description;
+
+ @Parameter(name = ApiConstants.ZONE_ID, required = true, entityType = ZoneResponse.class, type = BaseCmd.CommandType.UUID, description = "The zone in which the heuristic " +
+ "rule will be applied.")
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.PURPOSE, required = true, type = BaseCmd.CommandType.STRING, description =
Review Comment:
IMO, type would be a better parameter name here.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/RemoveSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,54 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+@APICommand(name = "removeSecondaryStorageSelector", description = "Removes an existing secondary storage selector.", responseObject = SecondaryStorageHeuristicsResponse.class,
Review Comment:
Add
```java
since = "4.19.0",
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/ListSecondaryStorageSelectorsCmd.java:
##########
@@ -0,0 +1,63 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "listSecondaryStorageSelectors", description = "Lists the secondary storage selectors and their rules.", responseObject = SecondaryStorageHeuristicsResponse.class,
Review Comment:
Add
```java
since = "4.19.0",
```
##########
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java:
##########
@@ -191,4 +198,12 @@ public ImageStoreVO findOneByZoneAndProtocol(long dataCenterId, String protocol)
List<ImageStoreVO> results = listBy(sc, filter);
return results.size() == 0 ? null : results.get(0);
}
+
+
+ @Override
+ public List<ImageStoreVO> listImageStoresByZoneIds(Long... zoneIds) {
+ SearchCriteria<ImageStoreVO> sc = zonesInSearch.create();
+ sc.setParametersIfNotNull("zonesIn" ,zoneIds);
Review Comment:
```suggestion
sc.setParametersIfNotNull("zonesIn", zoneIds);
```
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/HeuristicVO.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage;
+
+import com.cloud.utils.db.GenericDao;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.persistence.Temporal;
+import javax.persistence.TemporalType;
+import java.util.Date;
+import java.util.UUID;
+
+@Entity
+@Table(name = "heuristics")
+public class HeuristicVO implements Heuristic {
+ @Id
+ @GeneratedValue(strategy = GenerationType.IDENTITY)
+ @Column(name = "id", nullable = false)
+ private Long id;
+
+ @Column(name = "uuid", nullable = false)
+ private String uuid = UUID.randomUUID().toString();
+
+ @Column(name = "name")
+ private String name;
+
+ @Column(name = "description")
+ private String description;
+
+ @Column(name = "zone_id", nullable = false)
+ private Long zoneId;
+
+ @Column(name = "purpose", nullable = false)
+ private String purpose;
+
+ @Column(name = "heuristic_rule", nullable = false, length = 65535)
+ private String heuristicRule;
+
+ @Column(name = GenericDao.CREATED_COLUMN, nullable = false)
+ @Temporal(value = TemporalType.TIMESTAMP)
+ private Date created;
+
+ @Column(name = GenericDao.REMOVED_COLUMN)
+ @Temporal(value = TemporalType.TIMESTAMP)
+ private Date removed = null;
+
+ public HeuristicVO() {
+ }
+
+ public HeuristicVO(String name, String description, Long zoneId, String purpose, String heuristicRule, Date created, Date removed) {
+ this.name = name;
+ this.description = description;
+ this.zoneId = zoneId;
+ this.purpose = purpose;
+ this.heuristicRule = heuristicRule;
+ this.created = created;
Review Comment:
Do we need `created` and `removed` in the constructor? AFAIK, `created` is populated by default on resource creation. And I don't think we will ever be creating an object which is `removed` from the beginning.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
Review Comment:
Add
```java
since = "4.19.0",
```
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java:
##########
@@ -0,0 +1,41 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+public class Account extends GenericHeuristicPresetVariable{
Review Comment:
```suggestion
public class Account extends GenericHeuristicPresetVariable {
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/UpdateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+@APICommand(name = "updateSecondaryStorageSelector", description = "Updates an existing secondary storage selector.", responseObject = SecondaryStorageHeuristicsResponse.class,
Review Comment:
Add
```java
since = "4.19.0",
```
##########
server/src/main/java/com/cloud/api/ApiResponseHelper.java:
##########
@@ -5088,4 +5090,22 @@ public FirewallResponse createIpv6FirewallRuleResponse(FirewallRule fwRule) {
response.setObjectName("firewallrule");
return response;
}
+
+ @Override
+ public SecondaryStorageHeuristicsResponse createSecondaryStorageSelectorResponse(Heuristic heuristic) {
+ SecondaryStorageHeuristicsResponse secondaryStorageHeuristicsResponse = new SecondaryStorageHeuristicsResponse();
+ String zoneUuid = ApiDBUtils.findZoneById(heuristic.getZoneId()).getUuid();
+
+ secondaryStorageHeuristicsResponse.setId(heuristic.getUuid());
Review Comment:
Directly use the constructor here to create the response object.
##########
engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secstorage.dao;
+
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import org.apache.cloudstack.secstorage.HeuristicVO;
+import org.apache.cloudstack.secstorage.heuristics.HeuristicPurpose;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class SecondaryStorageHeuristicDaoImpl extends GenericDaoBase<HeuristicVO, Long> implements SecondaryStorageHeuristicDao {
+ private SearchBuilder<HeuristicVO> zoneAndPurposeSearch;
+
+ @PostConstruct
+ public void init() {
+ zoneAndPurposeSearch = createSearchBuilder();
+ zoneAndPurposeSearch.and("zoneId", zoneAndPurposeSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
+ zoneAndPurposeSearch.and("purpose", zoneAndPurposeSearch.entity().getPurpose(), SearchCriteria.Op.IN);
+ zoneAndPurposeSearch.done();
+ }
+
+ @Override
+ public HeuristicVO findByZoneIdAndPurpose(long zoneId, HeuristicPurpose purpose) {
+ SearchCriteria<HeuristicVO> searchCriteria = zoneAndPurposeSearch.create();
+ searchCriteria.setParameters("zoneId", zoneId);
+ searchCriteria.setParameters("purpose", purpose.toString());
+ final Filter filter = new Filter(HeuristicVO.class, "created", false);
+
+ return findOneIncludingRemovedBy(searchCriteria, filter);
Review Comment:
Why do we need to get removed entries here?
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -1882,6 +1894,65 @@ public StoragePool syncStoragePool(SyncStoragePoolCmd cmd) {
return (PrimaryDataStoreInfo) _dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
}
+
+ @Override
+ public Heuristic createSecondaryStorageHeuristic(CreateSecondaryStorageSelectorCmd cmd) {
+ String name = cmd.getName();
+ String description = cmd.getDescription();
+ long zoneId = cmd.getZoneId();
+ String heuristicRule = cmd.getHeuristicRule();
+ String purpose = cmd.getPurpose();
+ HeuristicPurpose formattedPurpose = EnumUtils.getEnumIgnoreCase(HeuristicPurpose.class, purpose);
+
+ if (formattedPurpose == null) {
+ throw new IllegalArgumentException(String.format("The given heuristic purpose [%s] is not valid for creating a new secondary storage selector." +
+ " The valid options are %s.", purpose, Arrays.asList(HeuristicPurpose.values())));
+ }
+
+ HeuristicVO heuristic = secondaryStorageHeuristicDao.findByZoneIdAndPurpose(zoneId, formattedPurpose);
Review Comment:
This is getting removed entries as well. Which means if I already created an entry then removed it, I will never be able to create another one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1664028951
Packaging result [SF]: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: el9 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 6651
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1666192534
<b>[SF] Trillian test result (tid-7261)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 51862 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7261-vmware-67u3.zip
Smoke tests completed. 110 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_trigger_shutdown | `Failure` | 336.56 | test_safe_shutdown.py
test_01_isolate_network_FW_PF_default_routes_egress_true | `Error` | 190.71 | test_routers_network_ops.py
ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1667412251
@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1667381302
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6687
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1784605683
@vishesh92 can you review this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "andrijapanicsb (via GitHub)" <gi...@apache.org>.
andrijapanicsb commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1774609804
Hi, is there a global setting that would allow to keep the original behaviour, and activate this new feature as required/if wanted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1812127922
@BryanMLima , conflict :( can you resolve it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1727204195
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1664086368
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1664085874
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1665123358
<b>[SF] Trillian Build Failed (tid-7262)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1665214686
@DaanHoogland a [SF] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1606960817
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236512545
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/UpdateSecondaryStorageSelectorCmd.java:
##########
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage.heuristics;
+
+import com.cloud.user.Account;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+@APICommand(name = "updateSecondaryStorageSelector", description = "Updates an existing secondary storage selector.", responseObject = SecondaryStorageHeuristicsResponse.class,
+ requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+public class UpdateSecondaryStorageSelectorCmd extends BaseCmd {
+ @Parameter(name = ApiConstants.ID, type = BaseCmd.CommandType.UUID, entityType = SecondaryStorageHeuristicsResponse.class, required = true,
+ description = "The unique identifier of the secondary storage selector.")
+ private Long id;
+
+ @Parameter(name = ApiConstants.HEURISTIC_RULE, required = true, type = BaseCmd.CommandType.STRING, description = "The heuristic rule, in JavaScript language. It is required " +
+ "that it returns the UUID of a secondary storage pool.", length = 65535)
+ private String heuristicRule;
+
+ public Long getId() {
+ return id;
+ }
+
+ public String getHeuristicRule() {
+ return heuristicRule;
+ }
+
+ @Override
+ public void execute() {
+ Heuristic heuristic = _storageService.updateSecondaryStorageHeuristic(this);
+
+ if (heuristic == null) {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update the secondary storage selector.");
+ }
+
+ SecondaryStorageHeuristicsResponse response = _responseGenerator.createSecondaryStorageSelectorResponse(heuristic);
+ response.setResponseName(getCommandName());
+ this.setResponseObject(response);
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ return Account.ACCOUNT_ID_SYSTEM;
+ }
+}
Review Comment:
needs an extra newline
##########
api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java:
##########
@@ -0,0 +1,132 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements. See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership. The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License. You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied. See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.response;
+
+import com.cloud.serializer.Param;
+import com.google.gson.annotations.SerializedName;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import java.util.Date;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
+
+@EntityReference(value = {Heuristic.class})
+public class SecondaryStorageHeuristicsResponse extends BaseResponse {
+
+ @SerializedName(ApiConstants.ID)
+ @Param(description = "ID of the heuristic.")
+ private String id;
+
+ @SerializedName(ApiConstants.NAME)
+ @Param(description = "Name of the heuristic.")
+ private String name;
+
+ @SerializedName(ApiConstants.DESCRIPTION)
+ @Param(description = "Description of the heuristic.")
+ private String description;
+
+ @SerializedName(ApiConstants.ZONE_ID)
+ @Param(description = "The zone which the heuristic is valid upon.")
+ private String zoneId;
+
+ @SerializedName(ApiConstants.PURPOSE)
+ @Param(description = "The resource type directed to a specific secondary storage by the selector. " + HEURISTIC_PURPOSE_VALID_OPTIONS)
+ private String purpose;
+
+ @SerializedName(ApiConstants.HEURISTIC_RULE)
+ @Param(description = "The heuristic rule, in JavaScript language, used to select a secondary storage to be directed.")
+ private String heuristicRule;
+
+ @SerializedName(ApiConstants.CREATED)
+ @Param(description = "When the heuristic was created.")
+ private Date created;
+
+ @SerializedName(ApiConstants.REMOVED)
+ @Param(description = "When the heuristic was removed.")
+ private Date removed;
+
+ public SecondaryStorageHeuristicsResponse() {
+ super("heuristics");
+ }
+
+ public String getId() {
+ return id;
+ }
+
+ public void setId(String id) {
+ this.id = id;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ public String getDescription() {
+ return description;
+ }
+
+ public void setDescription(String description) {
+ this.description = description;
+ }
+
+ public String getZoneId() {
+ return zoneId;
+ }
+
+ public void setZoneId(String zoneId) {
+ this.zoneId = zoneId;
+ }
+
+ public String getPurpose() {
+ return purpose;
+ }
+
+ public void setPurpose(String purpose) {
+ this.purpose = purpose;
+ }
+
+ public String getHeuristicRule() {
+ return heuristicRule;
+ }
+
+ public void setHeuristicRule(String heuristicRule) {
+ this.heuristicRule = heuristicRule;
+ }
+
+ public Date getCreated() {
+ return created;
+ }
+
+ public void setCreated(Date created) {
+ this.created = created;
+ }
+
+ public Date getRemoved() {
+ return removed;
+ }
+
+ public void setRemoved(Date removed) {
+ this.removed = removed;
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1236516281
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/PresetVariables.java:
##########
@@ -0,0 +1,62 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+import java.util.List;
+
+public class PresetVariables {
+
+ private List<SecondaryStorage> secondaryStorages;
+
+ private Template template;
+
+ private Snapshot snapshot;
+
+ private Volume volume;
+
+ public List<SecondaryStorage> getSecondaryStorages() {
+ return secondaryStorages;
+ }
+
+ public void setSecondaryStorages(List<SecondaryStorage> secondaryStorages) {
+ this.secondaryStorages = secondaryStorages;
+ }
+
+ public Template getTemplate() {
+ return template;
+ }
+
+ public void setTemplate(Template template) {
+ this.template = template;
+ }
+
+ public Snapshot getSnapshot() {
+ return snapshot;
+ }
+
+ public void setSnapshot(Snapshot snapshot) {
+ this.snapshot = snapshot;
+ }
+
+ public Volume getVolume() {
+ return volume;
+ }
+
+ public void setVolume(Volume volume) {
+ this.volume = volume;
+ }
+}
Review Comment:
needs an extra newline
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java:
##########
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.heuristics.presetvariables;
+
+public class SecondaryStorage extends GenericHeuristicPresetVariable {
+
+ private String uuid;
+
+ private Long usedDiskSize;
+
+ private Long totalDiskSize;
+
+ private String protocol;
+
+ public String getUuid() {
+ return uuid;
+ }
+
+ public void setUuid(String uuid) {
+ this.uuid = uuid;
+ fieldNamesToIncludeInToString.add("uuid");
+ }
+
+ public Long getUsedDiskSize() {
+ return usedDiskSize;
+ }
+
+ public void setUsedDiskSize(Long usedDiskSize) {
+ this.usedDiskSize = usedDiskSize;
+ fieldNamesToIncludeInToString.add("usedDiskSize");
+ }
+
+ public Long getTotalDiskSize() {
+ return totalDiskSize;
+ }
+
+ public void setTotalDiskSize(Long totalDiskSize) {
+ this.totalDiskSize = totalDiskSize;
+ fieldNamesToIncludeInToString.add("totalDiskSize");
+ }
+
+ public String getProtocol() {
+ return protocol;
+ }
+
+ public void setProtocol(String protocol) {
+ this.protocol = protocol;
+ fieldNamesToIncludeInToString.add("protocol");
+ }
+}
Review Comment:
needs an extra newline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] BryanMLima commented on pull request #7659: Add dynamic secondary storage selection
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1600868549
> hey @BryanMLima , I saw the issue with functional description that came with this. I haven't yet read it and just reacted to the lint output:
>
> ```
> Check hooks apply to the repository......................................Passed
> check for case conflicts.................................................Passed
> check vcs permalinks.....................................................Passed
> fix end of files.........................................................Failed
> - hook id: end-of-file-fixer
> - exit code: 1
> - files were modified by this hook
>
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/PresetVariables.java
> Fixing api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java
> Fixing api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/UpdateSecondaryStorageSelectorCmd.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java
> Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java
> Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/HeuristicVO.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java
> Fixing api/src/main/java/org/apache/cloudstack/secstorage/heuristics/HeuristicPurpose.java
> Fixing api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java
> Fixing engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDao.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java
> Fixing api/src/main/java/org/apache/cloudstack/secstorage/heuristics/Heuristic.java
> Fixing server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java
>
> mixed line ending........................................................Passed
> ```
Thanks, I always forget about these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1667300000
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1665078185
@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7659: Add dynamic secondary storage selection
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1667411780
@blueorangutan test matrix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7659: Add dynamic secondary storage selection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1673179754
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1668634296
<b>[SF] Trillian test result (tid-7310)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47749 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7659-t7310-vmware-67u3.zip
Smoke tests completed. 113 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1809703566
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1809702328
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1816895066
> @BryanMLima any documentation available that provides some guidance on the usage of this feature? Any plans to implement a UI?
@rajujith currently, the only documentation (more like a feature specification) is described in issue #7654; though, I pretend to add a proper documentation of this feature in the official documentation of CloudStack. Regarding the implementation of the UI for this feature, it is planned, however, it is not a priority at the moment.
> @BryanMLima
>
> 1. I could dedicate a secondary storage for KVM snapshots.
>
> 2. I could dedicate a secondary storage for a domain.
>
> 3. I was unable to dedicate another secondary storage for a second domain in the same zone. I believe this might be a limitation?
>
> 4. I faced the below issues while registering template with the following rules:
>
>
> ```
> (SS-SelectorPR) 🐱 > create secondarystorageselector name=KVMtemplatesonly description=KVMtemplatesonly type=TEMPLATE zoneid=a2a9fafd-43fb-4508-915e-d3c836a7e972 heuristicrule="function directToDedicatedQCOW2Pool(pool) {return pool.uuid === '92dc82ff-c3cc-4756-928f-6802c358f599';} if (template.format === 'QCOW2') {secondaryStorages.filter(directToDedicatedQCOW2Pool)[0].uuid}"
>
> {
> "heuristics": {
> "created": "2023-11-15T11:42:50+0000",
> "description": "KVMtemplatesonly",
> "heuristicrule": "function directToDedicatedQCOW2Pool(pool) {return pool.uuid === '92dc82ff-c3cc-4756-928f-6802c358f599';} if (template.format === 'QCOW2') {secondaryStorages.filter(directToDedicatedQCOW2Pool)[0].uuid}",
> "id": "1d9070e9-5d12-4159-9e7c-de0793c87e24",
> "name": "KVMtemplatesonly",
> "type": "TEMPLATE",
> "zoneid": "a2a9fafd-43fb-4508-915e-d3c836a7e972"
> }
> }
> ```
>
> On register template:
>
> ```
> due to [javax.script.ScriptException: TypeError: Cannot read property "uuid" from undefined in <eval> at line number 1]
> at org.apache.cloudstack.utils.jsinterpreter.JsInterpreter.executeScriptInThread(JsInterpreter.java:121)
> ```
>
> Specific UUID:
>
> `create secondarystorageselector name=KVMtemplatesonly-specifc description=KVMtemplatesonly-specific type=TEMPLATE zoneid=a2a9fafd-43fb-4508-915e-d3c836a7e972 heuristicrule="function findStorageWithSpecificUuid(pool) { return pool.uuid === '92dc82ff-c3cc-4756-928f-6802c358f599'; } secondaryStorages.filter(findStorageWithSpecificUuid)[0].uuid" ` On register template:
>
> ```
> java.util.concurrent.ExecutionException: javax.script.ScriptException: TypeError: Cannot read property "uuid" from undefined in <eval> at line number 1
> at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
> ```
@rajujith thanks for testing this PR.
Regarding item **3)**, could you provide more details of this testing? This feature only allows one heuristic rule per type, if you tried to dedicate a secondary storage to a domain for a specific type, you need to edit the same heuristic rule to dedicate to another domain.
Concisely, all heuristic rules for a given type should be in the same secondary storage selector, otherwise, an exception will be thrown and the message `There is already a heuristic rule in the specified Zone {...} with the type [...].` will be presented.
In the future, this feature could allow multiple heuristic rules for the same type in a zone, allowing operators to specify the order in which each rule would be verified.
Regarding item **4)**, the description of this PR and the issue presented the `uuid` for the `secondarystorage` variable in the JS script; however, it is the `id` that is provided in the script. I apologise for the confusion, I already fixed this in both the PR and the issue description's.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1836215919
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7659: Add dynamic secondary storage selection
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1737321415
@BryanMLima a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1806223069
@BryanMLima a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#issuecomment-1810179020
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392793793
##########
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java:
##########
@@ -907,6 +907,15 @@ public T findOneBy(SearchCriteria<T> sc) {
return findOneIncludingRemovedBy(sc);
}
+ @Override
+ @DB()
+ public T findOneBy(SearchCriteria<T> sc, final Filter filter) {
Review Comment:
Got it. I got a little confused because of the method names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add dynamic secondary storage selection [cloudstack]
Posted by "BryanMLima (via GitHub)" <gi...@apache.org>.
BryanMLima commented on code in PR #7659:
URL: https://github.com/apache/cloudstack/pull/7659#discussion_r1392910229
##########
api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java:
##########
@@ -0,0 +1,141 @@
+//Licensed to the Apache Software Foundation (ASF) under one
+//or more contributor license agreements. See the NOTICE file
+//distributed with this work for additional information
+//regarding copyright ownership. The ASF licenses this file
+//to you under the Apache License, Version 2.0 (the
+//"License"); you may not use this file except in compliance
+//with the License. You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+//Unless required by applicable law or agreed to in writing,
+//software distributed under the License is distributed on an
+//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+//KIND, either express or implied. See the License for the
+//specific language governing permissions and limitations
+//under the License.
+package org.apache.cloudstack.api.response;
+
+import com.cloud.serializer.Param;
+import com.google.gson.annotations.SerializedName;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+import org.apache.cloudstack.secstorage.heuristics.Heuristic;
+
+import java.util.Date;
+
+import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_TYPE_VALID_OPTIONS;
+
+@EntityReference(value = {Heuristic.class})
+public class SecondaryStorageHeuristicsResponse extends BaseResponse {
+
+ @SerializedName(ApiConstants.ID)
+ @Param(description = "ID of the heuristic.")
+ private String id;
+
+ @SerializedName(ApiConstants.NAME)
+ @Param(description = "Name of the heuristic.")
+ private String name;
+
+ @SerializedName(ApiConstants.DESCRIPTION)
+ @Param(description = "Description of the heuristic.")
+ private String description;
+
+ @SerializedName(ApiConstants.ZONE_ID)
+ @Param(description = "The zone which the heuristic is valid upon.")
+ private String zoneId;
+
+ @SerializedName(ApiConstants.TYPE)
+ @Param(description = "The resource type directed to a specific secondary storage by the selector. " + HEURISTIC_TYPE_VALID_OPTIONS)
+ private String type;
+
+ @SerializedName(ApiConstants.HEURISTIC_RULE)
+ @Param(description = "The heuristic rule, in JavaScript language, used to select a secondary storage to be directed.")
+ private String heuristicRule;
+
+ @SerializedName(ApiConstants.CREATED)
+ @Param(description = "When the heuristic was created.")
+ private Date created;
+
+ @SerializedName(ApiConstants.REMOVED)
+ @Param(description = "When the heuristic was removed.")
+ private Date removed;
+
+
+ public SecondaryStorageHeuristicsResponse(String id, String name, String description, String zoneId, String type, String heuristicRule, Date created, Date removed) {
+ super("heuristics");
+ this.id = id;
+ this.name = name;
+ this.description = description;
+ this.zoneId = zoneId;
+ this.type = type;
+ this.heuristicRule = heuristicRule;
+ this.created = created;
+ this.removed = removed;
Review Comment:
Yes, the API `listSecondaryStorageSelectors` has the parameter `showRemoved`. When this parameter is set to true, the removed field is used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org