[cloudstack] branch master updated (36ef850 -> ba6e2ac)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.
from 36ef850 Merge remote-tracking branch 'origin/4.14'
add ba6e2ac plugins: Redfish Client & Redfish OOBM Driver (#4175)
No new revisions were added by this update.
Summary of changes:
.../outofbandmanagement/OutOfBandManagement.java | 12 +-
client/pom.xml | 5 +
.../dao/OutOfBandManagementDao.java| 5 +-
.../dao/OutOfBandManagementDaoImpl.java| 6 +
.../{ipmitool => redfish}/pom.xml | 4 +-
.../redfish/RedfishOutOfBandManagementDriver.java | 120 +++
.../driver/redfish/RedfishWrapper.java | 66
.../META-INF/cloudstack/redfish}/module.properties | 2 +-
.../cloudstack/redfish/spring-redfish-context.xml | 5 +-
plugins/pom.xml| 8 +-
ui/scripts/system.js | 4 +
.../cloudstack/utils/redfish/RedfishClient.java| 379 +
.../RedfishException.java} | 14 +-
.../utils/redfish/RedfishClientTest.java | 163 +
14 files changed, 774 insertions(+), 19 deletions(-)
copy plugins/outofbandmanagement-drivers/{ipmitool => redfish}/pom.xml (91%)
create mode 100644
plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
create mode 100644
plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishWrapper.java
copy
plugins/outofbandmanagement-drivers/{ipmitool/src/main/resources/META-INF/cloudstack/ipmitool
=> redfish/src/main/resources/META-INF/cloudstack/redfish}/module.properties
(98%)
copy
engine/storage/datamotion/src/main/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml
=>
plugins/outofbandmanagement-drivers/redfish/src/main/resources/META-INF/cloudstack/redfish/spring-redfish-context.xml
(88%)
create mode 100644
utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java
copy
utils/src/main/java/org/apache/cloudstack/utils/{graphite/GraphiteException.java
=> redfish/RedfishException.java} (77%)
create mode 100644
utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java
[GitHub] [cloudstack] andrijapanicsb commented on pull request #4068: Adding Centos8, Ubuntu 20.04, XCPNG8.1 Support
andrijapanicsb commented on pull request #4068: URL: https://github.com/apache/cloudstack/pull/4068#issuecomment-665762139 The failed test in this last env, the "test_01_redundant_vpc_site2site_vpn" from the "test_vpc_vpn.py" is passing fine when repeated manually (via margin) - i.e. intermittent failures, just like the other failing tests i.e. "test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL" and "test_05_rvpc_multi_tiers" from the "test_vpc_redundant.py" tests - sometimes they would pass, sometimes will fail - I've opened a new issue to ask dev community to address these not-robust-enough tests - https://github.com/apache/cloudstack/issues/4224 AS far as XCP-ng 8.x testing goes, this PR looks good to me. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4035: Document how to pass CIDRs lists API calls
DaanHoogland commented on pull request #4035: URL: https://github.com/apache/cloudstack/pull/4035#issuecomment-665520825 textual changes only no furhter testing needed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] ravening opened a new pull request #4230: Enable resetting config values to default value
ravening opened a new pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230
## Description
Provide reset button to zone,cluster,domain,account,
primary and secondary storage so that config values
can be reset to the default value
## Types of changes
- [X] Enhancement (improves an existing feature and functionality)
## Screenshots (if appropriate):
Reset button has been provided under zone,cluster, domain, account, primary
storage and secondary storage level
## How Has This Been Tested?
Tested throguh cmk also
```
(local) mgt01 > list configurations name=allow.user.expunge.recover.vm
accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
"configuration": [
{
"category": "Advanced",
"description": "Determines whether users can expunge or recover their
vm",
"isdynamic": true,
"name": "allow.user.expunge.recover.vm",
"scope": "account",
"value": "false"
}
],
"count": 1
}
(local) mgt01 > update configuration name=allow.user.expunge.recover.vm
accountid=4048bfec-c28e-11ea-9a43-0617bc003377 value=true
{
"configuration": {
"category": "Advanced",
"description": "Determines whether users can expunge or recover their
vm",
"isdynamic": true,
"name": "allow.user.expunge.recover.vm",
"scope": "account",
"value": "true"
}
}
(local) mgt01 > reset configuration name=allow.user.expunge.recover.vm
accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
"configuration": {
"category": "Advanced",
"description": "Determines whether users can expunge or recover their
vm",
"isdynamic": true,
"name": "allow.user.expunge.recover.vm",
"scope": "account",
"value": "false"
}
}
(local) mgt01 > list configurations name=allow.user.expunge.recover.vm
accountid=4048bfec-c28e-11ea-9a43-0617bc003377
{
"configuration": [
{
"category": "Advanced",
"description": "Determines whether users can expunge or recover their
vm",
"isdynamic": true,
"name": "allow.user.expunge.recover.vm",
"scope": "account",
"value": "false"
}
],
"count": 1
}
```
Run integration test case using
```
nosetests --with-marvin --marvin-config=
test_reset_configuration_settings.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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4047: Look for active templates for VR deployment
DaanHoogland commented on pull request #4047: URL: https://github.com/apache/cloudstack/pull/4047#issuecomment-665506324 as per your description @ravening, "If the template from which VR is created got deleted, the state is set to inactive and removed to null.", it seems the 'real' bug is that removed is set to null whilst it should be set to the time of deletion. Your code will work, but is it not hiding this 'real' problem? Being very defensive, we might want to address both issues. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3997: New API endpoint: UpdateVlanIpRange
DaanHoogland commented on pull request #3997: URL: https://github.com/apache/cloudstack/pull/3997#issuecomment-665510566 @kioie is this ready for review/ testing again? And do you intent to move this forward before 4.15? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] gaaray2k commented on issue #4153: Remote Access VPN does not work
gaaray2k commented on issue #4153: URL: https://github.com/apache/cloudstack/issues/4153#issuecomment-665621027 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #4229: Creating a network offering
DaanHoogland commented on issue #4229: URL: https://github.com/apache/cloudstack/issues/4229#issuecomment-665546509 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #4111: API-call to declare host as dead
wido commented on pull request #4111: URL: https://github.com/apache/cloudstack/pull/4111#issuecomment-665742105 Suggestions: - Unavailable - Degraded - Offline - Defect - Inactive 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4019: server: Move restoreVM to vm work job queue
weizhouapache commented on pull request #4019: URL: https://github.com/apache/cloudstack/pull/4019#issuecomment-665507567 > code looks good @weizhouapache but two questions: > > 1. I see the test was deleted and not replaced. any directive on other than manual testing (it might be covoured by marvin already?) > 2. when you say "Same steps succeed with this PR (the vm will be restored twice)., "How was this tesgted", do you mean to say that the VM will exist twice after restore? Is this the functionality we want? @DaanHoogland Hi, 1. I removed the unit tests as the method has been moved frm UserVmManagerImpl.java to VirtualMachineManagerImpl.java 2. if vm is restored twice, without this patch, the two restoration will be performed in parallel, there will be two ROOT disks, vm cannot be started . With this patch, the second restoration will be performed when the first restoration is done, hence there is only 1 ROOT disk. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] andrijapanicsb commented on issue #4153: Remote Access VPN does not work
andrijapanicsb commented on issue #4153: URL: https://github.com/apache/cloudstack/issues/4153#issuecomment-665632509 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4205: WIP - Integrate tungsten with cloudstack
DaanHoogland commented on pull request #4205: URL: https://github.com/apache/cloudstack/pull/4205#issuecomment-665598103 Nice to see this moving on. Please let us know as soon as you need review or other feedback, @radu-todirica 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #4217: How can I use LVM volume storage for my private CloudStack
DaanHoogland commented on issue #4217: URL: https://github.com/apache/cloudstack/issues/4217#issuecomment-665532055 there is [the users list](mailto:us...@cloudstack.apache.org>) for these questions. hope that helps. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4215: Enable account settings to be visible under domain settings
DaanHoogland commented on pull request #4215: URL: https://github.com/apache/cloudstack/pull/4215#issuecomment-665526708 This quite a functional change @ravening . Did you test settings for domains linked to ldap? I read in your description that you are (kind of) mixing account level settings with domain level settings. some settings do not apply to domains and others do not apply to accounts. I like your idea that the setting system should be hierarchical and agree, but it just isn't designed this way. the same issue exists in zone-pod-cluster-host. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4212: Migrate/Stop VMs with local storage when preparing host for maintenance
DaanHoogland commented on a change in pull request #4212:
URL: https://github.com/apache/cloudstack/pull/4212#discussion_r462174163
##
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##
@@ -5619,7 +5619,7 @@ public VirtualMachine migrateVirtualMachine(Long vmId,
Host destinationHost) thr
throw new InvalidParameterValueException("Unsupported Hypervisor
Type for User VM migration, we support XenServer/VMware/KVM/Ovm/Hyperv/Ovm3
only");
}
-if (isVMUsingLocalStorage(vm)) {
+if (isVMUsingLocalStorage(vm.getId())) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(vm + " is using Local Storage, cannot migrate
this VM.");
Review comment:
1. As for the message, looks like an extra check for the strategy is in
order, but the flow might already guarantee it's validity.
2. As for the second interpretation of the question, as a matter of
principle it is always good to check the logging level as parameters are always
evaluated before passing and you never know how often the code is being
executed.
3. As for the level, this is an obvious mis-use of debug. An error or at
least warning or info situation is occurring here; the user might expect a
migration but there is something impairing it. This is not a message for a
developer, hence not a debug level message.
my €0,03 ;)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3952: vrouter: remove a POSTROUTING rule for port forwarding in VPC router
blueorangutan commented on pull request #3952: URL: https://github.com/apache/cloudstack/pull/3952#issuecomment-665796142 Trillian test result (tid-2223) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 33746 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3952-t2223-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Smoke tests completed. 82 look OK, 1 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test03SecuredVmMigration>:setup | `Error` | 0.00 | 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4207: Human readable sizes in logs
Spaceman1984 commented on a change in pull request #4207:
URL: https://github.com/apache/cloudstack/pull/4207#discussion_r462165542
##
File path: utils/src/test/java/com/cloud/utils/HumanReadableJsonTest.java
##
@@ -0,0 +1,53 @@
+//
+// 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 com.cloud.utils;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson;
+
+public class HumanReadableJsonTest {
+
+@Test
+public void parseJsonObjectTest() {
+assertEquals("{}", getHumanReadableBytesJson("{}"));
+}
+@Test
+public void parseJsonArrayTest() {
+assertEquals("[]", getHumanReadableBytesJson("[]"));
+assertEquals("[{},{}]", getHumanReadableBytesJson("[{},{}]"));
+}
+@Test
+public void parseSimpleJsonTest() {
+assertEquals("[{\"object\":{}}]",
getHumanReadableBytesJson("[{\"object\":{}}]"));
+}
+@Test
+public void parseComplexJsonTest() {
+assertEquals("[{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[]}]"));
+assertEquals("[{\"object\":[{},{}]}]",
getHumanReadableBytesJson("[{\"object\":[{},{}]}]"));
+assertEquals("[{\"object\":[]},{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[]},{\"object\":[]}]"));
+assertEquals("[{\"object\":[{\"object\":[]}]},{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[{\"object\":[]}]},{\"object\":[]}]"));
+}
+@Test
+public void parseMatchJsonTest() {
Review comment:
@rhtyd these tests already exist in NumbersUtilTest.java
##
File path: utils/src/main/java/com/cloud/utils/NumbersUtil.java
##
@@ -93,6 +95,13 @@ public static String toReadableSize(long bytes) {
return builder.toString();
}
+public static String toHumanReadableSize(long size) {
Review comment:
I have added a check and a test for null which prevents an NPE.
##
File path: utils/src/test/java/com/cloud/utils/HumanReadableJsonTest.java
##
@@ -50,4 +51,14 @@ public void parseMatchJsonTest() {
assertEquals("[{\"size\":\"(0 bytes) 0\"}]",
getHumanReadableBytesJson("[{\"size\": \"0\"}]"));
assertEquals("[{\"size\":\"(0 bytes) 0\",\"bytesSent\":\"(0 bytes)
0\"}]", getHumanReadableBytesJson("[{\"size\": \"0\", \"bytesSent\": \"0\"}]"));
}
+
+@Test
+public void localeTest() {
+Locale.setDefault(Locale.UK); // UK test
Review comment:
@rhtyd From Java Docs:
"The Java Virtual Machine sets the default locale during startup based on
the host environment. It is used by many locale-sensitive methods if no locale
is explicitly specified. It can be changed using the setDefault method."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
ravening commented on pull request #4200: URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-665579509 > @ravening when creating an offering in the domain admins domain, are those automatically visible in subdomains and in parent domain(-chain)? it will be visible only for that domain. > > > If an offering is created on a domain then all its child > > domains should see it and all the parents domain > > should see it as well but > > ... > > > isrecursive=true ensures that all child domains can see it > > but if a service offering is created for the domain at the leaf > > of the domain path then its parent cant see it if isrecurise > > is true. so we need to pass isrecursive=false so that the > > parent domain can see the child offerings > > this sounds contradictory. the isrecursive flag should have an intuitive meaning that is consistent for a 'leave' or a 'node' domain. So whether or not isrecursive is set should have the same consequences in bith domain types. > > Am I misunderstanding something? yeah the `isrecursive` kind of confusing sometimes. Lets say I have this domain relationship ROOT -> child1 -> child11 if I try to list offering for domain ROOT by passing `isrecursive=true` then i see all offerings of ROOT, child1 and child11. Same applies for child1 now if i try to list offerings for domain child11 by passing `isrecursive=true` then it wont list the offering belonging to parent domain `child1` even though it should be displayed if you dont pass `isrecursive=true` and try listing offerings for child11 then you will see all the parent domain offerings. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4150: [HEALTH][4.14] Health Check Run
blueorangutan commented on pull request #4150: URL: https://github.com/apache/cloudstack/pull/4150#issuecomment-665766362 Trillian test result (tid-2221) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 38742 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4150-t2221-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Smoke tests completed. 82 look OK, 1 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test03SecuredVmMigration>:setup | `Error` | 0.00 | 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4225: vmware: volume utilisation is always zero
rhtyd closed pull request #4225: URL: https://github.com/apache/cloudstack/pull/4225 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #4035: Document how to pass CIDRs lists API calls
DaanHoogland merged pull request #4035: URL: https://github.com/apache/cloudstack/pull/4035 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4220: Fix cpuallocated value in findHostsForMIgration api
blueorangutan commented on pull request #4220: URL: https://github.com/apache/cloudstack/pull/4220#issuecomment-665569508 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] andrijapanicsb commented on issue #4227: confused about User Permission
andrijapanicsb commented on issue #4227: URL: https://github.com/apache/cloudstack/issues/4227#issuecomment-665799984 @gaaray2k the correct place for QUESTIONS is the mailing list - you'll get better help there. - USER = just username/password (or API/SECRET key) combination to become an ACCOUNT - i.e. once you log in with your credentials, you "become" an ACCOUNT - and everything you do / all resources created belong to this ACCOUNT. - - ACCOUNT = owner of all resources, with optionally limits on a per-account basis. - DOMAIN ADMIN ACCOUNT = same as regular user-role ACCOUNT, but can see/manage all resources (including those of other ACCOUNTS) in his own domain. Resources can also be limited on a per-domain basis (but keep in mind that sum of all resources for all accounts in a domain, can not be more than the limits set on a domain level) - USER ACCOUNT - well, same as the domain account. but it has the user role - so can only create/manage his own resoruces (VMs, volumes, networks etc) Does that gives you enough info? In your case, inside your acme.com domain, you would create a DOMAIN ADMIN ACCOUNT called "peter" (i.e. an account with a "domain admin" role, not "user" role). Here, also a "user" (user/password) called "peter" will be automatically created for you DOMAIN ADMIN ACCOUNT named 'peter"... this is perhaps what confuses you (so there is account peter and automatically created user peter for that account) Your DOMAIN ADMIN "peter" would then usually create ACCOUNTS of the "user" role (that will automatically create a "user" (user/password) for that account Hope that makes sense. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4226: Removed check on SSLEngine client mode
blueorangutan commented on pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226#issuecomment-665726754 Trillian test result (tid-2217) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 39959 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4226-t2217-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_outofbandmanagement.py Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py Smoke tests completed. 81 look OK, 2 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test03SecuredVmMigration>:setup | `Error` | 0.00 | test_vm_life_cycle.py test_hostha_enable_ha_when_host_disabled | `Error` | 2.56 | test_hostha_kvm.py test_hostha_enable_ha_when_host_in_maintenance | `Error` | 303.39 | test_hostha_kvm.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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4225: vmware: volume utilisation is always zero
Spaceman1984 commented on pull request #4225: URL: https://github.com/apache/cloudstack/pull/4225#issuecomment-665738409 @rhtyd I had a look at this, the physical size is reflecting correctly, but that utilization field is a bit strange on the front end, seems like the value should be multiplied by 100? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #4175: Redfish Client & Redfish OOBM Driver
wido commented on pull request #4175: URL: https://github.com/apache/cloudstack/pull/4175#issuecomment-665553410 @DaanHoogland The problem is that for Redfish testing (and IPMI) you need a physical machine do test this on. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4207: Human readable sizes in logs
Spaceman1984 commented on pull request #4207: URL: https://github.com/apache/cloudstack/pull/4207#issuecomment-665508759 @rhtyd, I checked the commas vs dots in the conversion, looks like it changes based on the locale and JDK version. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #4211: VM HA does not work when server is stopped gracefully
DaanHoogland commented on issue #4211: URL: https://github.com/apache/cloudstack/issues/4211#issuecomment-665518391 This doesn't sound familiar @weizhouapache . To recap, - a hosts is HA enabled? (genuine question , here) - a vm with a ha enabled offering is created on it. - the server is powered off. Is by means of an remote (ipmi) action or by logging on to the console and issueing a halt command? I would not expect this behaviour as a user but it might be a side effect of the host-ha, hence my questions. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] andrijapanicsb edited a comment on issue #4227: confused about User Permission
andrijapanicsb edited a comment on issue #4227: URL: https://github.com/apache/cloudstack/issues/4227#issuecomment-665799984 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4111: API-call to declare host as dead
DaanHoogland commented on pull request #4111: URL: https://github.com/apache/cloudstack/pull/4111#issuecomment-665535646 Like your thinking, mr operator ;). one reservation @GabrielBrascher; is the word 'dead' the best to use here? the life/dead metaphore for hosts might incite associations that are not fit for purpose. is it an idea to find some other word to use, like 'broken' or 'corrupted'? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4200: Allow domain admins to create offering without mentioning domainid
DaanHoogland commented on pull request #4200: URL: https://github.com/apache/cloudstack/pull/4200#issuecomment-665577184 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4215: Enable account settings to be visible under domain settings
ravening commented on pull request #4215: URL: https://github.com/apache/cloudstack/pull/4215#issuecomment-665547798 > This quite a functional change @ravening . > Did you test settings for domains linked to ldap? > I read in your description that you are (kind of) mixing account level settings with domain level settings. some settings do not apply to domains and others do not apply to accounts. > > I like your idea that the setting system should be hierarchical and agree, but it just isn't designed this way. the same issue exists in zone-pod-cluster-host. @DaanHoogland I am enabling only account settings for domain and not the other way around. Also there are two global settings to control this. 1. To enable all the account settings visible under domain 2. To fetch values from domain if account setting value is not set The code is not making domain settings visible under account If the (1)st global setting is true then you can configure value under a domain and all accounts will see that value. If its disabled then account will get value from global setting. I didnt test it for ldap linked domains 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #4227: confused about User Permission
DaanHoogland commented on issue #4227: URL: https://github.com/apache/cloudstack/issues/4227#issuecomment-665793646 @gaaray2k I think your question makes sense, even when not understanding it completely; you want the resource count to be on domain level, or shared over the domain admin and the regular user account, do you? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 edited a comment on pull request #4207: Human readable sizes in logs
Spaceman1984 edited a comment on pull request #4207: URL: https://github.com/apache/cloudstack/pull/4207#issuecomment-665508759 @rhtyd, I checked the commas vs dots in the conversion, looks like it changes based on the locale. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] gaaray2k commented on issue #4229: Creating a network offering
gaaray2k commented on issue #4229: URL: https://github.com/apache/cloudstack/issues/4229#issuecomment-665561519 Which requirements have to be met?. If I create an offering and choose the options that I want, shouldn't it just work? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4111: API-call to declare host as dead
GabrielBrascher commented on pull request #4111: URL: https://github.com/apache/cloudstack/pull/4111#issuecomment-665738632 @DaanHoogland your observation makes sense, I will look for a better word ;) Thanks for the input! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4047: Look for active templates for VR deployment
ravening commented on pull request #4047: URL: https://github.com/apache/cloudstack/pull/4047#issuecomment-665551768 > as per your description @ravening, "If the template from which VR is created got deleted, the state > is set to inactive and removed to null.", it seems the 'real' bug is that removed is set to null whilst it should be set to the time of deletion. Your code will work, but is it not hiding this 'real' problem? Being very defensive, we might want to address both issues. I will see if I can reproduce the issue to check which part of the code is setting it to null instead of removed timestamp 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4207: Human readable sizes in logs
rhtyd commented on a change in pull request #4207:
URL: https://github.com/apache/cloudstack/pull/4207#discussion_r462184183
##
File path: utils/src/test/java/com/cloud/utils/HumanReadableJsonTest.java
##
@@ -50,4 +51,14 @@ public void parseMatchJsonTest() {
assertEquals("[{\"size\":\"(0 bytes) 0\"}]",
getHumanReadableBytesJson("[{\"size\": \"0\"}]"));
assertEquals("[{\"size\":\"(0 bytes) 0\",\"bytesSent\":\"(0 bytes)
0\"}]", getHumanReadableBytesJson("[{\"size\": \"0\", \"bytesSent\": \"0\"}]"));
}
+
+@Test
+public void localeTest() {
+Locale.setDefault(Locale.UK); // UK test
Review comment:
@Spaceman1984 what's the default locale for CloudStack, is there a
global setting or does it pick it up from OS?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4175: Redfish Client & Redfish OOBM Driver
DaanHoogland commented on pull request #4175: URL: https://github.com/apache/cloudstack/pull/4175#issuecomment-665547812 > Do you mean [@TestTemplate](https://junit.org/junit5/docs/5.0.2/api/org/junit/jupiter/api/TestTemplate.html) from JUnit5? no, nice link though, @GabrielBrascher . What i mean is a template VM that can serve as hypervisor for testing in. virtual nested environment. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4220: Fix cpuallocated value in findHostsForMIgration api
rhtyd commented on pull request #4220: URL: https://github.com/apache/cloudstack/pull/4220#issuecomment-665569219 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[cloudstack-primate] branch master updated: components: Adding quick-view options to list view (#458)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack-primate.git
The following commit(s) were added to refs/heads/master by this push:
new 41aa289 components: Adding quick-view options to list view (#458)
41aa289 is described below
commit 41aa289778aafe0d5d49d31566c78a5c1ea438b3
Author: davidjumani
AuthorDate: Wed Jul 29 16:12:52 2020 +0530
components: Adding quick-view options to list view (#458)
Fixes #449
Signed-off-by: Rohit Yadav
---
src/components/view/ActionButton.vue | 8 +++-
src/components/view/ListView.vue | 31 ++---
src/components/view/QuickView.vue| 85
src/config/section/storage.js| 2 +-
src/views/AutogenView.vue| 3 ++
5 files changed, 122 insertions(+), 7 deletions(-)
diff --git a/src/components/view/ActionButton.vue
b/src/components/view/ActionButton.vue
index 6aa57a8..66b88ec 100644
--- a/src/components/view/ActionButton.vue
+++ b/src/components/view/ActionButton.vue
@@ -17,7 +17,7 @@
-
+
{{ $t(action.label) }}
@@ -56,6 +57,7 @@
:type="action.icon === 'delete' ? 'danger' : (action.icon === 'plus' ?
'primary' : 'default')"
:shape="!dataView && action.icon === 'plus' ? 'round' : 'circle'"
style="margin-left: 5px"
+:size="size"
@click="execAction(action)">
{{ $t(action.label) }}
@@ -108,6 +110,10 @@ export default {
loading: {
type: Boolean,
default: false
+},
+size: {
+ type: String,
+ default: 'default'
}
},
watch: {
diff --git a/src/components/view/ListView.vue b/src/components/view/ListView.vue
index fe0740d..4f12eea 100644
--- a/src/components/view/ListView.vue
+++ b/src/components/view/ListView.vue
@@ -17,9 +17,9 @@
-
+
+
-
{{ text }}
@@ -276,6 +281,7 @@ import Console from '@/components/widgets/Console'
import OsLogo from '@/components/widgets/OsLogo'
import Status from '@/components/widgets/Status'
import InfoCard from '@/components/view/InfoCard'
+import QuickView from '@/components/view/QuickView'
export default {
name: 'ListView',
@@ -283,7 +289,8 @@ export default {
Console,
OsLogo,
Status,
-InfoCard
+InfoCard,
+QuickView
},
props: {
columns: {
@@ -297,6 +304,10 @@ export default {
loading: {
type: Boolean,
default: false
+},
+actions: {
+ type: Array,
+ default: () => []
}
},
inject: ['parentFetchData', 'parentToggleLoading', 'parentEditTariffAction'],
@@ -313,6 +324,16 @@ export default {
}
},
methods: {
+quickViewEnabled () {
+ return new RegExp(['/vm', '/kubernetes', '/ssh', '/vmgroup',
'/affinitygroup',
+'/volume', '/snapshot', '/backup',
+'/guestnetwork', '/vpc', '/vpncustomergateway',
+'/template', '/iso',
+'/project', '/account',
+'/zone', '/pod', '/cluster', '/host', '/storagepool', '/imagestore',
'/systemvm', '/router', '/ilbvm',
+'/computeoffering', '/systemoffering', '/diskoffering',
'/backupoffering', '/networkoffering', '/vpcoffering'].join('|'))
+.test(this.$route.path)
+},
fetchColumns () {
if (this.isOrderUpdatable()) {
return this.columns
diff --git a/src/components/view/QuickView.vue
b/src/components/view/QuickView.vue
new file mode 100644
index 000..05ffe3d
--- /dev/null
+++ b/src/components/view/QuickView.vue
@@ -0,0 +1,85 @@
+// 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.
+
+
+
+
+
+
+
+
+
+
+
+import ActionButton from '@/components/view/ActionButton'
+
+export default {
+ name: 'QuickView',
+ components: {
+ActionButton
+ },
+ props: {
+actions: {
+ type: Array,
+ default: () => []
+},
+enabled: {
+ type: Boolean,
+ default: true
+},
+size: {
+ type: String,
+ default: 'default'
+},
+resource: {
+ type: Object,
+
[cloudstack] branch master updated (7e3b61b -> c856614)
This is an automated email from the ASF dual-hosted git repository. dahn pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from 7e3b61b Merge branch '4.14' add c856614 Document how to pass CIDRs lists API calls (#4035) No new revisions were added by this update. Summary of changes: .../api/command/user/firewall/CreateEgressFirewallRuleCmd.java| 4 ++-- .../cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java | 2 +- .../api/command/user/firewall/CreatePortForwardingRuleCmd.java| 2 +- .../api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java | 2 +- .../cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java| 2 +- .../cloudstack/api/command/user/network/CreateNetworkACLCmd.java | 4 ++-- .../cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java | 2 +- .../command/user/securitygroup/AuthorizeSecurityGroupEgressCmd.java | 2 +- .../command/user/securitygroup/AuthorizeSecurityGroupIngressCmd.java | 2 +- .../cloudstack/api/command/user/vpn/CreateVpnCustomerGatewayCmd.java | 2 +- .../cloudstack/api/command/user/vpn/UpdateVpnCustomerGatewayCmd.java | 2 +- .../java/org/apache/cloudstack/api/response/FirewallResponse.java | 4 ++-- .../java/org/apache/cloudstack/api/response/FirewallRuleResponse.java | 2 +- .../java/org/apache/cloudstack/api/response/LoadBalancerResponse.java | 2 +- .../org/apache/cloudstack/api/response/NetworkACLItemResponse.java| 2 +- .../cloudstack/api/response/Site2SiteCustomerGatewayResponse.java | 2 +- .../cloudstack/api/response/Site2SiteVpnConnectionResponse.java | 2 +- 17 files changed, 20 insertions(+), 20 deletions(-)
[GitHub] [cloudstack] DaanHoogland commented on pull request #4078: Cleanup download urls when SSVM destroyed
DaanHoogland commented on pull request #4078: URL: https://github.com/apache/cloudstack/pull/4078#issuecomment-665057020 @sureshanaparti do you approve the changes? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4020: server: move UpdateDefaultNic to vm work job queue
DaanHoogland commented on pull request #4020: URL: https://github.com/apache/cloudstack/pull/4020#issuecomment-665082680 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rvalle closed issue #4223: wrong bridge interfaces
rvalle closed issue #4223: URL: https://github.com/apache/cloudstack/issues/4223 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[cloudstack] 01/01: Merge remote-tracking branch 'origin/4.14'
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 36ef85012f69af59657c87ad88184769549d8792
Merge: c856614 eec5602
Author: Rohit Yadav
AuthorDate: Wed Jul 29 14:08:02 2020 +0530
Merge remote-tracking branch 'origin/4.14'
Signed-off-by: Rohit Yadav
.../org/apache/cloudstack/acl/RoleService.java | 8
.../api/command/admin/acl/ListRolesCmd.java| 25 ++--
.../cloudstack/api/command/user/vm/ListVMsCmd.java | 12 ++
.../cloudstack/api/response/HostResponse.java | 6 +--
.../org/apache/cloudstack/acl/dao/RoleDao.java | 6 +++
.../org/apache/cloudstack/acl/dao/RoleDaoImpl.java | 15 ++-
.../cloudstack/ca/provider/RootCAProviderTest.java | 2 -
.../java/com/cloud/api/query/QueryManagerImpl.java | 15 +++
.../com/cloud/api/query/dao/HostJoinDaoImpl.java | 12 +-
.../cloud/api/query/dao/TemplateJoinDaoImpl.java | 10 ++---
.../cloud/network/vpc/NetworkACLServiceImpl.java | 2 +-
.../org/apache/cloudstack/acl/RoleManagerImpl.java | 46 --
.../apache/cloudstack/acl/RoleManagerImplTest.java | 19 -
13 files changed, 123 insertions(+), 55 deletions(-)
diff --cc api/src/main/java/org/apache/cloudstack/acl/RoleService.java
index d7eef92,86f8a77..2e181dc
--- a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
+++ b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
@@@ -18,8 -18,9 +18,10 @@@
package org.apache.cloudstack.acl;
import java.util.List;
+import java.util.Map;
+ import com.cloud.utils.Pair;
+
import org.apache.cloudstack.acl.RolePermission.Permission;
import org.apache.cloudstack.framework.config.ConfigKey;
@@@ -81,7 -82,7 +87,9 @@@ public interface RoleService
*/
List findRolesByType(RoleType roleType);
+ Pair, Integer> findRolesByType(RoleType roleType, Long
startIndex, Long limit);
+
List findAllPermissionsBy(Long roleId);
+
+Permission getRolePermission(String permission);
}
diff --cc engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java
index 4a53965,06c07ba..ec1051b
--- a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java
@@@ -17,8 -17,8 +17,9 @@@
package org.apache.cloudstack.acl.dao;
+ import com.cloud.utils.Pair;
import com.cloud.utils.db.GenericDao;
+
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.RoleVO;
@@@ -26,7 -26,10 +27,12 @@@ import java.util.List
public interface RoleDao extends GenericDao {
List findAllByName(String roleName);
+
+ Pair, Integer> findAllByName(final String roleName, Long
offset, Long limit);
+
List findAllByRoleType(RoleType type);
+List findByName(String roleName);
+RoleVO findByNameAndType(String roleName, RoleType type);
+
+ Pair, Integer> findAllByRoleType(RoleType type, Long offset,
Long limit);
}
diff --cc
engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java
index 4dc11fc,b220049..dda836a
--- a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java
@@@ -59,23 -59,12 +66,27 @@@ public class RoleDaoImpl extends Generi
@Override
public List findAllByRoleType(final RoleType type) {
+ return findAllByRoleType(type, null, null).first();
+ }
+
+ public Pair, Integer> findAllByRoleType(final RoleType type,
Long offset, Long limit) {
SearchCriteria sc = RoleByTypeSearch.create();
sc.setParameters("roleType", type);
- return listBy(sc);
+ return searchAndCount(sc, new Filter(RoleVO.class, "id", true,
offset, limit));
}
+
+@Override
+public List findByName(String roleName) {
+SearchCriteria sc = RoleByNameSearch.create();
+sc.setParameters("roleName", roleName);
+return listBy(sc);
+}
+
+@Override
+public RoleVO findByNameAndType(String roleName, RoleType type) {
+SearchCriteria sc = RoleByNameAndTypeSearch.create();
+sc.setParameters("roleName", roleName);
+sc.setParameters("roleType", type);
+return findOneBy(sc);
+}
}
[GitHub] [cloudstack] rafaelweingartner commented on a change in pull request #4194: enable update tags on disk offerings
rafaelweingartner commented on a change in pull request #4194:
URL: https://github.com/apache/cloudstack/pull/4194#discussion_r461945911
##
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##
@@ -3183,7 +3184,7 @@ public DiskOffering updateDiskOffering(final
UpdateDiskOfferingCmd cmd) {
throw new InvalidParameterValueException(String.format("Unable to
update disk offering: %s by id user: %s because it is not root-admin or
domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
}
-final boolean updateNeeded = name != null || displayText != null ||
sortKey != null || displayDiskOffering != null;
+final boolean updateNeeded = isUpdateDiskOfferingNeeded(name,
displayText, sortKey, displayDiskOffering, tags);
Review comment:
`shouldUpdateDiskOffering` sounds better to me. However, I am not a
native speaker. Therefore, change only if you also agree with me.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4131: [HEALTH] Master/4.15 Health Check please don't merge this
rhtyd commented on pull request #4131: URL: https://github.com/apache/cloudstack/pull/4131#issuecomment-665425178 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4165: Allow renaming cluster, host, and storage
blueorangutan commented on pull request #4165: URL: https://github.com/apache/cloudstack/pull/4165#issuecomment-664785902 Trillian test result (tid-2205) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 66215 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4165-t2205-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Smoke tests completed. 81 look OK, 2 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_05_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 2121.32 | test_kubernetes_clusters.py ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test03SecuredVmMigration>:setup | `Error` | 0.00 | 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[cloudstack] branch master updated (c856614 -> 36ef850)
This is an automated email from the ASF dual-hosted git repository. rohit pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from c856614 Document how to pass CIDRs lists API calls (#4035) add e225db4 ca: Removed check on client mode (#4226) add eec5602 api: Bug fixes for primate (#4214) new 36ef850 Merge remote-tracking branch 'origin/4.14' The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../org/apache/cloudstack/acl/RoleService.java | 8 .../api/command/admin/acl/ListRolesCmd.java| 25 ++-- .../cloudstack/api/command/user/vm/ListVMsCmd.java | 12 ++ .../cloudstack/api/response/HostResponse.java | 6 +-- .../org/apache/cloudstack/acl/dao/RoleDao.java | 6 +++ .../org/apache/cloudstack/acl/dao/RoleDaoImpl.java | 15 ++- .../cloudstack/ca/provider/RootCAProviderTest.java | 2 - .../java/com/cloud/api/query/QueryManagerImpl.java | 15 +++ .../com/cloud/api/query/dao/HostJoinDaoImpl.java | 12 +- .../cloud/api/query/dao/TemplateJoinDaoImpl.java | 10 ++--- .../cloud/network/vpc/NetworkACLServiceImpl.java | 2 +- .../org/apache/cloudstack/acl/RoleManagerImpl.java | 46 -- .../apache/cloudstack/acl/RoleManagerImplTest.java | 19 - 13 files changed, 123 insertions(+), 55 deletions(-)
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4078: Cleanup download urls when SSVM destroyed
DaanHoogland commented on a change in pull request #4078:
URL: https://github.com/apache/cloudstack/pull/4078#discussion_r461600605
##
File path:
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java
##
@@ -307,12 +307,15 @@ public Answer
handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
//We just need to remove the UUID.vhd
String extractUrl = cmd.getExtractUrl();
-command.add("unlink /var/www/html/userdata/" +
extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
-String result = command.execute();
-if (result != null) {
-// FIXME - Ideally should bail out if you cant delete symlink. Not
doing it right now.
-// This is because the ssvm might already be destroyed and the
symlinks do not exist.
-s_logger.warn("Error in deleting symlink :" + result);
+String result;
+if (extractUrl != null) {
+command.add("unlink /var/www/html/userdata/" +
extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
+result = command.execute();
+if (result != null) {
+// FIXME - Ideally should bail out if you cant delete symlink.
Not doing it right now.
+// This is because the ssvm might already be destroyed and the
symlinks do not exist.
+s_logger.warn("Error in deleting symlink :" + result);
Review comment:
hm, that would be an easy check, wonder if that is all.
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2698,9 +2701,40 @@ public String extractVolume(ExtractVolumeCmd cmd) {
}
// Check if the url already exists
-VolumeDataStoreVO volumeStoreRef =
_volumeStoreDao.findByVolume(volumeId);
+SearchCriteria sc =
_volumeStoreDao.createSearchCriteria();
+sc.addAnd("state", SearchCriteria.Op.EQ,
ObjectInDataStoreStateMachine.State.Ready.toString());
+sc.addAnd("volumeId", SearchCriteria.Op.EQ, volumeId);
+sc.addAnd("destroyed", SearchCriteria.Op.EQ, false);
+// the volume should not change (attached/detached, vm not updated)
after created
+if (volume.getVolumeType() == Volume.Type.ROOT) { // for ROOT disk
+VMInstanceVO vm = _vmInstanceDao.findById(volume.getInstanceId());
+sc.addAnd("updated", SearchCriteria.Op.GTEQ, vm.getUpdateTime());
+} else if (volume.getVolumeType() == Volume.Type.DATADISK &&
volume.getInstanceId() == null) { // for not attached DATADISK
+sc.addAnd("updated", SearchCriteria.Op.GTEQ, volume.getUpdated());
+} else { // for attached DATA DISK
+VMInstanceVO vm = _vmInstanceDao.findById(volume.getInstanceId());
+sc.addAnd("updated", SearchCriteria.Op.GTEQ, vm.getUpdateTime());
+sc.addAnd("updated", SearchCriteria.Op.GTEQ, volume.getUpdated());
+}
+Filter filter = new Filter(VolumeDataStoreVO.class, "created", false,
0L, 1L);
+List volumeStoreRefs = _volumeStoreDao.search(sc,
filter);
+VolumeDataStoreVO volumeStoreRef = null;
+if (volumeStoreRefs != null && !volumeStoreRefs.isEmpty()) {
+volumeStoreRef = volumeStoreRefs.get(0);
+}
if (volumeStoreRef != null && volumeStoreRef.getExtractUrl() != null) {
return volumeStoreRef.getExtractUrl();
+} else if (volumeStoreRef != null) {
+s_logger.debug("volume " + volumeId + " is already installed on
secondary storage, install path is " +
+volumeStoreRef.getInstallPath());
+ImageStoreEntity secStore = (ImageStoreEntity)
dataStoreMgr.getDataStore(volumeStoreRef.getDataStoreId(), DataStoreRole.Image);
+String extractUrl =
secStore.createEntityExtractUrl(volumeStoreRef.getInstallPath(),
volume.getFormat(), null);
+volumeStoreRef = _volumeStoreDao.findByVolume(volumeId);
+volumeStoreRef.setExtractUrl(extractUrl);
+volumeStoreRef.setExtractUrlCreated(DateUtil.now());
+_volumeStoreDao.update(volumeStoreRef.getId(), volumeStoreRef);
+return extractUrl;
Review comment:
can you extract these two blocks in methods with good descriptive names
please?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4150: [HEALTH][4.14] Health Check Run
rhtyd commented on pull request #4150: URL: https://github.com/apache/cloudstack/pull/4150#issuecomment-665425111 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 opened a new pull request #4226: Removed check on SSLEngine client mode
Spaceman1984 opened a new pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226 ## Description The SSL engine defaults to "Server mode" when doing handshaking. This PR removes the check on the mode which was causing tests to fail. This behavior was changed in OpenJDK 11.0.8 ``` JDK-8245077: Default SSLEngine Created in Server Role = In JDK 11 and later, `javax.net.ssl.SSLEngine` by default used client mode when handshaking. As a result, the set of default enabled protocols may differ to what is expected. `SSLEngine` would usually be used in server mode. From this JDK release onwards, `SSLEngine` will default to server mode. The `javax.net.ssl.SSLEngine.setUseClientMode(boolean mode)` method may be used to configure the mode. ``` ## Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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) ## Screenshots (if appropriate): ## How Has This Been Tested? This was tested by building Cloudstack and allowing the tests to run. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] ravening opened a new pull request #4228: Dont add host back after agent service restart
ravening opened a new pull request #4228: URL: https://github.com/apache/cloudstack/pull/4228 ## Description Fixes #4218 If a host is removed from cloudstack, it will be added back if we restart the agent service on the host. It should not be added back if manualy removed. So when host is removed, guid is set to null. When service is restarted, check if guid is null for the host it send to mgt server. If guid is null then dont add it back. When we are adding host, temporarily set guid to non null value so that when agent sends startuprouting command it knows that the host is getting explicitly added. Once host is added, set it back to null ## Types of changes - [X] Bug fix (non-breaking change which fixes an issue) ## Screenshots (if appropriate): ## How Has This Been Tested? 1. Add a host from ui. It will be successfully added 2. Now remove the host 3. ssh to host and restart cloudstack-agent service 4. The host is not added back 5. Try to add host from ui again. It will be added 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4144: Fix Usage failed to get pid
blueorangutan commented on pull request #4144: URL: https://github.com/apache/cloudstack/pull/4144#issuecomment-665077347 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4144: Fix Usage failed to get pid
DaanHoogland commented on pull request #4144: URL: https://github.com/apache/cloudstack/pull/4144#issuecomment-665076769 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] andrijapanicsb opened a new issue #4224: test_vpc_redundant sometimes fails
andrijapanicsb opened a new issue #4224:
URL: https://github.com/apache/cloudstack/issues/4224
Pasting below the runinfo.txt related parts - these are making it hard to
see if the tests are not robust enough, or if there is some kind of failure on
the ACS side.
Could someone please look into the root cause of failing tests with
redundant VPCs?
`2020-07-27 11:00:51,863 - DEBUG - Sending GET Cmd :
listRouters===
2020-07-27 11:00:51,900 - DEBUG - Response : [{domain : u'ROOT', domainid :
u'3f9835f1-cff3-11ea-a290-1e00800139e7', guestmacaddress :
u'02:00:42:e6:00:03', templatename : u'SystemVM Template (XenServer)', scrip
tsversion : u'199a9ebcfb29d7f92dc5209461655a32\n', healthchecksfailed :
True, linklocalip : u'169.254.168.24', zoneid :
u'3b87962d-43e3-43e6-aec8-44e752adb4b2', linklocalmacaddress :
u'0e:00:a9:fe:a8:18', linklo
calnetworkid : u'8ee75f93-69c3-4972-b87d-196a5c2e84db', linklocalnetmask :
u'255.255.0.0', publicmacaddress : u'1e:00:51:00:00:03', id :
u'86667242-1455-4c34-9168-b98b0bcbf3e0', networkdomain : u'cs5cloud.intern
al', guestnetworkid : u'fa1d1a22-231c-4dd9-b2fb-9560c6f7bed7', hostname :
u'pr4068-t2204-xcpng81-xs2', gateway : u'10.1.63.254', publicip :
u'10.1.37.223', state : u'Running', version : u'4.15.0', role : u'VIRTU
AL_ROUTER', podid : u'bd593c3a-f865-42a8-a4fe-6bcf4e8a8533',
serviceofferingid : u'17eb7c2a-3219-4453-ab71-44f5ac1b8dfa', zonename :
u'pr4068-t2204-xcpng81', podname : u'Pod1', name : u'r-4-VM', guestnetworkname
: u'NETWORK-10.1.2.1', nic : [{networkid :
u'8ee75f93-69c3-4972-b87d-196a5c2e84db', macaddress : u'0e:00:a9:fe:a8:18', id
: u'6bedde43-5ab2-46d5-9a18-ac640d976991', traffictype : u'Control', netmask :
u'255.255
.0.0', ipaddress : u'169.254.168.24', gateway : u'169.254.0.1', isdefault :
False}, {networkid : u'55e27b1c-8205-4b3b-be9c-ae113e2a4342', macaddress :
u'1e:00:51:00:00:03', isolationuri : u'vlan://7', broadcastu
ri : u'vlan://7', traffictype : u'Public', netmask : u'255.255.224.0', id :
u'13496314-fa8d-43a2-a30b-d51478d86363', ipaddress : u'10.1.37.223', gateway :
u'10.1.63.254', isdefault : True}, {networkid : u'7cbed1
13-ed53-4e45-845f-9e6d5d533175', macaddress : u'02:00:76:6c:00:03',
isolationuri : u'vlan://1986', type : u'Isolated', broadcasturi :
u'vlan://1986', traffictype : u'Guest', netmask : u'255.255.255.0', ipaddress
: u'10.1.1.109', id : u'879e4c7b-d0b3-4dc6-9c14-516cc44a7d8c', networkname
: u'NETWORK-10.1.1.1', gateway : u'10.1.1.1', isdefault : False}, {networkid :
u'fa1d1a22-231c-4dd9-b2fb-9560c6f7bed7', macaddress : u'
02:00:42:e6:00:03', isolationuri : u'vlan://1992', type : u'Isolated',
broadcasturi : u'vlan://1992', traffictype : u'Guest', netmask :
u'255.255.255.0', ipaddress : u'10.1.2.113', id : u'a324e4a7-127e-4530-a438
-f1f00bd2169d', networkname : u'NETWORK-10.1.2.1', gateway : u'10.1.2.1',
isdefault : False}], publicnetworkid : u'55e27b1c-8205-4b3b-be9c-ae113e2a4342',
redundantstate : u'BACKUP', hostid : u'83d38508-d3a1-49f3
-acc1-4a191c1eb0c7', templateid : u'22bea20c-649e-4745-b73b-d44b72f46a16',
requiresupgrade : False, publicnetmask : u'255.255.224.0', account :
u'test-TestVPCRedundancy-test_01_create_redundant_VPC_2tiers_4VMs_4
IPs_4PF_ACL-UM7Q58', vpcid : u'f6a98c6a-7ca4-4f08-a8bd-b44c55ca2ee7',
isredundantrouter : True, created : u'2020-07-27T10:50:57+', hypervisor :
u'XenServer', dns1 : u'10.2.0.50', vpcname : u'TestVPC-1QKJV0',
dns2 : u'8.8.8.8', guestnetmask : u'255.255.255.0', guestipaddress :
u'10.1.2.113', serviceofferingname : u'System Offering For Software Router'},
{domain : u'ROOT', domainid : u'3f9835f1-cff3-11ea-a290-1e00800
139e7', guestmacaddress : u'02:00:1e:af:00:02', templatename : u'SystemVM
Template (XenServer)', scriptsversion : u'199a9ebcfb29d7f92dc5209461655a32\n',
healthchecksfailed : False, linklocalip : u'169.254.73.84'
, zoneid : u'3b87962d-43e3-43e6-aec8-44e752adb4b2', linklocalmacaddress :
u'0e:00:a9:fe:49:54', linklocalnetworkid :
u'8ee75f93-69c3-4972-b87d-196a5c2e84db', linklocalnetmask : u'255.255.0.0',
publicmacaddress :
u'1e:00:51:00:00:03', id : u'4142944f-de32-4628-94c6-b062ff8ea5a7',
networkdomain : u'cs5cloud.internal', guestnetworkid :
u'fa1d1a22-231c-4dd9-b2fb-9560c6f7bed7', hostname :
u'pr4068-t2204-xcpng81-xs1', gatewa
y : u'10.1.63.254', publicip : u'10.1.37.223', state : u'Running', version :
u'4.15.0', role : u'VIRTUAL_ROUTER', podid :
u'bd593c3a-f865-42a8-a4fe-6bcf4e8a8533', serviceofferingid :
u'17eb7c2a-3219-4453-ab71-44
f5ac1b8dfa', zonename : u'pr4068-t2204-xcpng81', podname : u'Pod1', name :
u'r-3-VM', guestnetworkname : u'NETWORK-10.1.2.1', nic : [{networkid :
u'8ee75f93-69c3-4972-b87d-196a5c2e84db', macaddress : u'0e:00:a9:
fe:49:54', id : u'87429061-035f-402d-9398-a73b31218b68', traffictype :
u'Control', netmask : u'255.255.0.0', ipaddress : u'169.254.73.84', gateway :
u'169.254.0.1', isdefault : False}, {networkid :
[GitHub] [cloudstack] rhtyd commented on pull request #4207: Human readable sizes in logs
rhtyd commented on pull request #4207: URL: https://github.com/apache/cloudstack/pull/4207#issuecomment-665472945 @Spaceman1984 failures related to vm lifecycle seen here as well, I suspect it may be related to the fix you added in #4226 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] gaaray2k opened a new issue #4227: confused aboutUser Permission
gaaray2k opened a new issue #4227: URL: https://github.com/apache/cloudstack/issues/4227 I am a little confused as to how user accounts work on CS. right now I have an account called A which is domain admin in domain lab.com . if I add another user in account A, it gets the same "domain admin" permission. if I want to create another user with the "user" role, I would have to create another account with the "user" role under the same domain, but the new account would get difference resource count. how do I setup a domain as follow? if I have a customer call acme.com buying resources, I would create a domain called acme.com, then I would create an account called peter which is going to be an admin on that domain. then peter can create users that would get the "user" role under the same domain. resources consumed by a regular user need to counted from the sources available in the whole "account". please dont bother sending me the link to CS docs. I already read it a 1000 and still dont get it. apologized if I am not being clear as to what I am lookig for. the way CS implemented the user permission model is very confusing in my opinion. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4068: Adding Centos8, Ubuntu 20.04, XCPNG8.1 Support
blueorangutan commented on pull request #4068: URL: https://github.com/apache/cloudstack/pull/4068#issuecomment-664677157 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4144: Fix Usage failed to get pid
DaanHoogland commented on a change in pull request #4144:
URL: https://github.com/apache/cloudstack/pull/4144#discussion_r461636110
##
File path: usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
##
@@ -275,7 +277,14 @@ public boolean configure(String name, Map
params) throws Configu
s_logger.error("Unhandled exception configuring UsageManger", e);
throw new ConfigurationException("Unhandled exception configuring
UsageManager " + e.toString());
}
-_pid = Integer.parseInt(System.getProperty("pid"));
+
+try {
+String processName =
ManagementFactory.getRuntimeMXBean().getName();
+_pid = Integer.parseInt(processName.split("@")[0]);
+} catch (Exception e) {
+s_logger.error("Unable to get process pid ", e);
+throw new ConfigurationException("Unable to get process pid " +
e.toString());
+}
Review comment:
1. twice the same message should be in a string constand
1. rethrown should not abandon the stack trace for the original problem
```suggestion
String processName;
try {
processName = ManagementFactory.getRuntimeMXBean().getName();
_pid = Integer.parseInt(processName.split("@")[0]);
} catch (Exception e) {
String msg = String.format("Unable to get process Id for %s!",
processName);
s_logger.error(msg);
throw new ConfigurationException(msg, e);
}
```
or
```suggestion
String processName;
try {
processName = ManagementFactory.getRuntimeMXBean().getName();
_pid = Integer.parseInt(processName.split("@")[0]);
} catch (Exception e) {
String msg = String.format("Unable to get process Id for %s!",
processName);
s_logger.debug(msg , e);
throw new ConfigurationException(msg, e);
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4213: Search vm snapshots using tags
blueorangutan commented on pull request #4213: URL: https://github.com/apache/cloudstack/pull/4213#issuecomment-664762505 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4207: Human readable sizes in logs
rhtyd commented on a change in pull request #4207:
URL: https://github.com/apache/cloudstack/pull/4207#discussion_r462080672
##
File path: utils/src/test/java/com/cloud/utils/HumanReadableJsonTest.java
##
@@ -0,0 +1,53 @@
+//
+// 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 com.cloud.utils;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson;
+
+public class HumanReadableJsonTest {
+
+@Test
+public void parseJsonObjectTest() {
+assertEquals("{}", getHumanReadableBytesJson("{}"));
+}
+@Test
+public void parseJsonArrayTest() {
+assertEquals("[]", getHumanReadableBytesJson("[]"));
+assertEquals("[{},{}]", getHumanReadableBytesJson("[{},{}]"));
+}
+@Test
+public void parseSimpleJsonTest() {
+assertEquals("[{\"object\":{}}]",
getHumanReadableBytesJson("[{\"object\":{}}]"));
+}
+@Test
+public void parseComplexJsonTest() {
+assertEquals("[{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[]}]"));
+assertEquals("[{\"object\":[{},{}]}]",
getHumanReadableBytesJson("[{\"object\":[{},{}]}]"));
+assertEquals("[{\"object\":[]},{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[]},{\"object\":[]}]"));
+assertEquals("[{\"object\":[{\"object\":[]}]},{\"object\":[]}]",
getHumanReadableBytesJson("[{\"object\":[{\"object\":[]}]},{\"object\":[]}]"));
+}
+@Test
+public void parseMatchJsonTest() {
Review comment:
@Spaceman1984 can you add tests for non zero values, and see if the
expected results match? (for kbs, mbs, gbs)
##
File path: utils/src/main/java/com/cloud/utils/NumbersUtil.java
##
@@ -93,6 +95,13 @@ public static String toReadableSize(long bytes) {
return builder.toString();
}
+public static String toHumanReadableSize(long size) {
Review comment:
@Spaceman1984 can you add unit test for this method, giving some values
such as zero and non-zero (to test KB, MB, GB, TB). Have you verified that all
consumes would pass `long` to it and not `Long` which may potentially cause a
NPE?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4225: vmware: volume utilisation is always zero
rhtyd commented on pull request #4225: URL: https://github.com/apache/cloudstack/pull/4225#issuecomment-665015182 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4068: Adding Centos8, Ubuntu 20.04, XCPNG8.1 Support
blueorangutan removed a comment on pull request #4068: URL: https://github.com/apache/cloudstack/pull/4068#issuecomment-664193373 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4226: Removed check on SSLEngine client mode
blueorangutan commented on pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226#issuecomment-665052216 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] ravening commented on issue #4209: use X-forward-for address instead of the load balancer ip.
ravening commented on issue #4209: URL: https://github.com/apache/cloudstack/issues/4209#issuecomment-665208650 we are using haproxy as a loadbalancer and when transparent mode is enabled, it uses XFF address instead of loadbalancer ip. This will be fixed in https://github.com/apache/cloudstack/pull/4141 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4128: Role based users in Projects
blueorangutan commented on pull request #4128: URL: https://github.com/apache/cloudstack/pull/4128#issuecomment-664838452 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4213: Search vm snapshots using tags
Pearl1594 commented on pull request #4213: URL: https://github.com/apache/cloudstack/pull/4213#issuecomment-664762152 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4214: Bug fixes for primate
blueorangutan commented on pull request #4214: URL: https://github.com/apache/cloudstack/pull/4214#issuecomment-664627461 Trillian test result (tid-2196) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 50718 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4214-t2196-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_iso.py Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py Smoke tests completed. 81 look OK, 2 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_add_delete_kubernetes_supported_version | `Error` | 1807.38 | test_kubernetes_supported_versions.py ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py ContextSuite context=Test03SecuredVmMigration>:setup | `Error` | 0.00 | 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3952: vrouter: remove a POSTROUTING rule for port forwarding in VPC router
blueorangutan commented on pull request #3952: URL: https://github.com/apache/cloudstack/pull/3952#issuecomment-664857096 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4150: [HEALTH][4.14] Health Check Run
blueorangutan commented on pull request #4150: URL: https://github.com/apache/cloudstack/pull/4150#issuecomment-665425548 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3902: vrouter: Save PlaceHolder nic for VR if network does not have source nat
DaanHoogland commented on a change in pull request #3902:
URL: https://github.com/apache/cloudstack/pull/3902#discussion_r461593221
##
File path:
server/src/main/java/com/cloud/network/guru/ExternalGuestNetworkGuru.java
##
@@ -261,6 +265,17 @@ public NicProfile allocate(Network config, NicProfile nic,
VirtualMachineProfile
profile.setIPv4Netmask(null);
}
+if (config.getVpcId() == null && vm.getType() ==
VirtualMachine.Type.DomainRouter) {
+boolean isPublicNetwork =
_networkModel.isProviderSupportServiceInNetwork(config.getId(),
Service.SourceNat, Provider.VirtualRouter);
+if (!isPublicNetwork) {
+Nic placeholderNic =
_networkModel.getPlaceholderNicForRouter(config, null);
+if (placeholderNic == null) {
+s_logger.debug("Saving placeholder nic with ip4 address "
+ profile.getIPv4Address() +
+" and ipv6 address " + profile.getIPv6Address() +
" for the network " + config);
+_networkMgr.savePlaceholderNic(config,
profile.getIPv4Address(), profile.getIPv6Address(),
VirtualMachine.Type.DomainRouter);
+}
+}
+}
Review comment:
`allocateRegularVrNic(...)`
##
File path: server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java
##
@@ -372,15 +373,25 @@ public NicProfile allocate(final Network network,
NicProfile nic, final VirtualM
if (isGateway) {
guestIp = network.getGateway();
-} else if (vm.getVirtualMachine().getType() ==
VirtualMachine.Type.DomainRouter) {
-guestIp =
_ipAddrMgr.acquireGuestIpAddressByPlacement(network, nic.getRequestedIPv4());
} else {
-guestIp = _ipAddrMgr.acquireGuestIpAddress(network,
nic.getRequestedIPv4());
-}
-
-if (!isGateway && guestIp == null && network.getGuestType() !=
GuestType.L2 &&
!_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty())
{
-throw new
InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP" + "
address for network " + network, DataCenter.class,
-dc.getId());
+if (network.getGuestType() != GuestType.L2 && vm.getType()
== VirtualMachine.Type.DomainRouter) {
+Nic placeholderNic =
_networkModel.getPlaceholderNicForRouter(network, null);
+if (placeholderNic != null) {
+s_logger.debug("Nic got an ip address " +
placeholderNic.getIPv4Address() + " stored in placeholder nic for the network "
+ network);
+guestIp = placeholderNic.getIPv4Address();
+}
+}
+if (guestIp == null) {
+if (vm.getVirtualMachine().getType() ==
VirtualMachine.Type.DomainRouter) {
+guestIp =
_ipAddrMgr.acquireGuestIpAddressByPlacement(network, nic.getRequestedIPv4());
+} else {
+guestIp =
_ipAddrMgr.acquireGuestIpAddress(network, nic.getRequestedIPv4());
+}
+}
+if (guestIp == null && network.getGuestType() !=
GuestType.L2 &&
!_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty())
{
+throw new
InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP" + "
address for network " + network, DataCenter.class,
+dc.getId());
+}
Review comment:
`allocateRegularVrNic(...)`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4226: Removed check on SSLEngine client mode
rhtyd merged pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4207: Human readable sizes in logs
blueorangutan commented on pull request #4207: URL: https://github.com/apache/cloudstack/pull/4207#issuecomment-664602670 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4222: Removed check on SSLEngine client mode
rhtyd commented on pull request #4222: URL: https://github.com/apache/cloudstack/pull/4222#issuecomment-664748427 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4149: set python explicit during transistion
DaanHoogland commented on pull request #4149: URL: https://github.com/apache/cloudstack/pull/4149#issuecomment-664840747 @GabrielBrascher then you have to change your version to the one you did install, or not set it at all. I have to switch between 2 and 3 as well. That is exactly the point of the change. after transistion it should read some 3 version or be deleted if we want to... I'm not sure if just 2 or just 3 works. if there is no interest in this, I'll just close. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4222: Removed check on SSLEngine client mode
Spaceman1984 commented on pull request #4222: URL: https://github.com/apache/cloudstack/pull/4222#issuecomment-665016249 > I had some issues on master, where the test checks for true `Assert.assertTrue(e.getUseClientMode()); `, not `assertFalse`. will this be forwarded to master as well? @GabrielBrascher , I will create a pull request for master. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4207: Human readable sizes in logs
Spaceman1984 commented on pull request #4207: URL: https://github.com/apache/cloudstack/pull/4207#issuecomment-664977855 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4032: Suspending the VM prior to deleting snapshots to avoid corruption, th…
DaanHoogland commented on a change in pull request #4032:
URL: https://github.com/apache/cloudstack/pull/4032#discussion_r461380678
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##
@@ -97,12 +108,26 @@ public Answer execute(final DeleteVMSnapshotCommand cmd,
final LibvirtComputingR
} else if (snapshot == null) {
s_logger.debug("Can not find vm snapshot " +
cmd.getTarget().getSnapshotName() + " on vm: " + vmName + ", return true");
return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
+} else if (didDelete) {
+s_logger.error("Failed to resume vm after delete snapshot " +
cmd.getTarget().getSnapshotName() + " on vm: " + vmName + " return true : " +
e);
+return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
}
s_logger.warn(msg, e);
return new DeleteVMSnapshotAnswer(cmd, false, msg);
} finally {
if (dm != null) {
+// Make sure if the VM is paused, then resume it, in case we
got an exception during our delete() and didn't have the chance before
+try {
+dm = libvirtComputingResource.getDomain(conn, vmName);
+state = dm.getInfo().state;
+if (state == DomainInfo.DomainState.VIR_DOMAIN_PAUSED) {
+dm.resume();
+}
+} catch (LibvirtException e) {
+s_logger.error("Failed to resume vm after delete snapshot
" + cmd.getTarget().getSnapshotName() + " on vm: " + vmName + " return true : "
+ e);
+}
+
Review comment:
can you extract this in a separate method please?
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##
@@ -97,12 +108,26 @@ public Answer execute(final DeleteVMSnapshotCommand cmd,
final LibvirtComputingR
} else if (snapshot == null) {
s_logger.debug("Can not find vm snapshot " +
cmd.getTarget().getSnapshotName() + " on vm: " + vmName + ", return true");
return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
+} else if (didDelete) {
+s_logger.error("Failed to resume vm after delete snapshot " +
cmd.getTarget().getSnapshotName() + " on vm: " + vmName + " return true : " +
e);
+return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());
Review comment:
here a var `didDelete` leads us to conclude that the current exception
thrown is caused by `dm.resume()`. this boolean should be named some thing like
`tryingResume` instead. better would be to investigate the nature of the
exception.
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##
@@ -1029,7 +1029,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
}
}
} catch (final Exception ex) {
-s_logger.debug("Failed to delete snapshots on primary",
ex);
+s_logger.error("Failed to delete snapshots on primary",
ex);
Review comment:
:+1:
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteVMSnapshotCommandWrapper.java
##
@@ -63,6 +66,14 @@ public Answer execute(final DeleteVMSnapshotCommand cmd,
final LibvirtComputingR
dm.suspend(); // suspend the vm to avoid image corruption
snapshot.delete(0); // only remove this snapshot, not children
+didDelete = true;
+
+// Resume the VM
+dm = libvirtComputingResource.getDomain(conn, vmName);
+state = dm.getInfo().state;
+if (state == DomainInfo.DomainState.VIR_DOMAIN_PAUSED) {
+dm.resume();
+}
Review comment:
resume action leads to an implicit assumption which must be guarded
during maintenance by future generations: failing resume is concluded by
examining a var named `didDelete`. dangerous assumption even with the current
logic computing. see below.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] rvalle commented on issue #4223: wrong bridge interfaces
rvalle commented on issue #4223: URL: https://github.com/apache/cloudstack/issues/4223#issuecomment-664593182 ok @GabrielBrascher let me check... I have configured - guest.network.device - public.network.device - private.network.device I have not configured private.bridge.name, you are right, I do remember the documentation mentioning it. here is my agent.properties ``` #Storage #Thu Jul 23 17:49:58 CEST 2020 guest.network.device=guest_netbr workers=5 private.network.device=host_netbr port=8250 resource=com.cloud.hypervisor.kvm.resource.LibvirtComputingResource pod=1 zone=1 hypervisor.type=kvm guid=ef861baa-8f7c-38fe-9b6a-981df48ac7ac public.network.device=host_netbr cluster=1 local.storage.uuid=**' keystore.passphrase=*** domr.scripts.dir=scripts/network/domr/kvm LibvirtComputingResource.id=1 host=10.71.0.254 router.aggregation.command.each.timeout=600 ``` I have confused cloud0 bridge with cloud0br bridge 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on a change in pull request #4194: enable update tags on disk offerings
RodrigoDLopez commented on a change in pull request #4194:
URL: https://github.com/apache/cloudstack/pull/4194#discussion_r461859143
##
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##
@@ -3183,7 +3184,7 @@ public DiskOffering updateDiskOffering(final
UpdateDiskOfferingCmd cmd) {
throw new InvalidParameterValueException(String.format("Unable to
update disk offering: %s by id user: %s because it is not root-admin or
domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
}
-final boolean updateNeeded = name != null || displayText != null ||
sortKey != null || displayDiskOffering != null;
+final boolean updateNeeded = name != null || displayText != null ||
sortKey != null || displayDiskOffering != null || tags != null;
Review comment:
I extract this to an method, and add documentation for this new method
##
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##
@@ -3207,30 +3208,13 @@ public DiskOffering updateDiskOffering(final
UpdateDiskOfferingCmd cmd) {
diskOffering.setDisplayOffering(displayDiskOffering);
}
-// Note: tag editing commented out for now;keeping the code intact,
-// might need to re-enable in next releases
-// if (tags != null)
-// {
-// if (tags.trim().isEmpty() && diskOfferingHandle.getTags() == null)
-// {
-// //no new tags; no existing tags
-// diskOffering.setTagsArray(csvTagsToList(null));
-// }
-// else if (!tags.trim().isEmpty() && diskOfferingHandle.getTags() !=
-// null)
-// {
-// //new tags + existing tags
-// List oldTags = csvTagsToList(diskOfferingHandle.getTags());
-// List newTags = csvTagsToList(tags);
-// oldTags.addAll(newTags);
-// diskOffering.setTagsArray(oldTags);
-// }
-// else if(!tags.trim().isEmpty())
-// {
-// //new tags; NO existing tags
-// diskOffering.setTagsArray(csvTagsToList(tags));
-// }
-// }
+if (tags != null){
+if(tags.isBlank()){
Review comment:
done... thanks for the hint
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4137: Adding VPN options for IKE version and IKE split connections.
blueorangutan commented on pull request #4137: URL: https://github.com/apache/cloudstack/pull/4137#issuecomment-665084642 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd opened a new pull request #4225: vmware: volume utilisation is always zero
rhtyd opened a new pull request #4225: URL: https://github.com/apache/cloudstack/pull/4225 - This fixes issues of virtual size to be twice in case the disk is a linked-clone root disk. The virtual size of root disk (first in chain) must be used. - Fixes to report the physical size for a volume whose stat is being collected ## Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on issue #4223: wrong bridge interfaces
GabrielBrascher commented on issue #4223: URL: https://github.com/apache/cloudstack/issues/4223#issuecomment-664597022 I thought that this was the case. All looking good then :-) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #3730: Update VR's scripts from python2 to python3
GabrielBrascher commented on pull request #3730: URL: https://github.com/apache/cloudstack/pull/3730#issuecomment-664590843 Sorry for the late reply, @rhtyd. I just fixed conflicts and ported to the current master. I am now performing tests and if all goes right I shall have this PR ready for review soon. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on a change in pull request #4213: Search vm snapshots using tags
RodrigoDLopez commented on a change in pull request #4213:
URL: https://github.com/apache/cloudstack/pull/4213#discussion_r461837337
##
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##
@@ -229,11 +238,32 @@ public boolean stop() {
sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
sb.and("display_name", sb.entity().getDisplayName(),
SearchCriteria.Op.EQ);
sb.and("account_id", sb.entity().getAccountId(), SearchCriteria.Op.EQ);
-sb.done();
+
+if (MapUtils.isNotEmpty(tags)) {
+SearchBuilder tagSearch =
_resourceTagDao.createSearchBuilder();
+for (int count = 0; count < tags.size(); count++) {
+tagSearch.or().op(ApiConstants.KEY + String.valueOf(count),
tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
+tagSearch.and(ApiConstants.VALUE + String.valueOf(count),
tagSearch.entity().getValue(), SearchCriteria.Op.EQ);
+tagSearch.cp();
+}
+tagSearch.and(ApiConstants.RESOURCE_TYPE,
tagSearch.entity().getResourceType(), SearchCriteria.Op.EQ);
+sb.groupBy(sb.entity().getId());
+sb.join("tagSearch", tagSearch, sb.entity().getId(),
tagSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER);
Review comment:
Could you please extract this to a method, add documentation to what
this method do and why.
Also can you please create unit tests to cover those method
##
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##
@@ -229,11 +238,32 @@ public boolean stop() {
sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
sb.and("display_name", sb.entity().getDisplayName(),
SearchCriteria.Op.EQ);
sb.and("account_id", sb.entity().getAccountId(), SearchCriteria.Op.EQ);
-sb.done();
+
+if (MapUtils.isNotEmpty(tags)) {
+SearchBuilder tagSearch =
_resourceTagDao.createSearchBuilder();
+for (int count = 0; count < tags.size(); count++) {
+tagSearch.or().op(ApiConstants.KEY + String.valueOf(count),
tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
+tagSearch.and(ApiConstants.VALUE + String.valueOf(count),
tagSearch.entity().getValue(), SearchCriteria.Op.EQ);
+tagSearch.cp();
+}
+tagSearch.and(ApiConstants.RESOURCE_TYPE,
tagSearch.entity().getResourceType(), SearchCriteria.Op.EQ);
+sb.groupBy(sb.entity().getId());
+sb.join("tagSearch", tagSearch, sb.entity().getId(),
tagSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER);
+}
SearchCriteria sc = sb.create();
_accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive,
permittedAccounts, listProjectResourcesCriteria);
+if (MapUtils.isNotEmpty(tags)) {
+int count = 0;
+sc.setJoinParameters("tagSearch", ApiConstants.RESOURCE_TYPE,
ResourceTag.ResourceObjectType.VMSnapshot.toString());
+for (String key : tags.keySet()) {
+sc.setJoinParameters("tagSearch", ApiConstants.KEY +
String.valueOf(count), key);
+sc.setJoinParameters("tagSearch", ApiConstants.VALUE +
String.valueOf(count), tags.get(key));
+count++;
+}
Review comment:
cloud you do the same here please,
extract to a new method, add documentation and unit 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4226: Removed check on SSLEngine client mode
Spaceman1984 commented on pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226#issuecomment-665051451 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4131: [HEALTH] Master/4.15 Health Check please don't merge this
blueorangutan commented on pull request #4131: URL: https://github.com/apache/cloudstack/pull/4131#issuecomment-665425488 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4226: Removed check on SSLEngine client mode
rhtyd commented on pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226#issuecomment-665412298 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4225: vmware: volume utilisation is always zero
blueorangutan commented on pull request #4225: URL: https://github.com/apache/cloudstack/pull/4225#issuecomment-665015464 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4222: Removed check on SSLEngine client mode
blueorangutan commented on pull request #4222: URL: https://github.com/apache/cloudstack/pull/4222#issuecomment-664728845 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4137: Adding VPN options for IKE version and IKE split connections.
DaanHoogland commented on pull request #4137: URL: https://github.com/apache/cloudstack/pull/4137#issuecomment-665083905 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4175: Redfish Client & Redfish OOBM Driver
GabrielBrascher commented on pull request #4175: URL: https://github.com/apache/cloudstack/pull/4175#issuecomment-665250531 @DaanHoogland you are right, docs should be updated indeed. I will work on that as well. > and can we create a template for nested testing Do you mean [@TestTemplate](https://junit.org/junit5/docs/5.0.2/api/org/junit/jupiter/api/TestTemplate.html) from JUnit5? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] borisstoyanov commented on pull request #4128: Role based users in Projects
borisstoyanov commented on pull request #4128: URL: https://github.com/apache/cloudstack/pull/4128#issuecomment-664910401 @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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4226: Removed check on SSLEngine client mode
RodrigoDLopez commented on pull request #4226: URL: https://github.com/apache/cloudstack/pull/4226#issuecomment-665270280 Code LGTM. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4068: Adding Centos8, Ubuntu 20.04, XCPNG8.1 Support
weizhouapache commented on pull request #4068: URL: https://github.com/apache/cloudstack/pull/4068#issuecomment-664626019 > @weizhouapache - related to the Ubuntu 20 changes (your original PR was closed, and Rohit cherry-picked the changes to this PR) - please feel free to test Ubuntu 20 specifically (i.e. for the moment, we are not testing Ubuntu 20 as part of this PR). Thx @andrijapanicsb ok. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] andrijapanicsb commented on issue #4224: test_vpc_redundant sometimes fails
andrijapanicsb commented on issue #4224: URL: https://github.com/apache/cloudstack/issues/4224#issuecomment-664882291 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] gaaray2k opened a new issue #4229: Creating a network offering
gaaray2k opened a new issue #4229: URL: https://github.com/apache/cloudstack/issues/4229 when I create a new network offering I give it a name and fill out all the require fields. I then enable the new offering but the new network offering doesnt show up when creating a new network. am I doing something wrong or is this a known bug? version: 4.14 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4214: Bug fixes for primate
rhtyd merged pull request #4214: URL: https://github.com/apache/cloudstack/pull/4214 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
