[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554198132 @anuragaw a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554198020 @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 With regards, Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346628250
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
##
@@ -1019,21 +1021,26 @@ public String toString() {
private String _dpdkExtraLines;
private String _interfaceMode;
-public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model) {
-defBridgeNet(brName, targetBrName, macAddr, model, 0);
+/**
+ * Bridge Net
+ */
+public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model, Integer mtu) {
+defBridgeNet(brName, targetBrName, macAddr, model, 0, mtu);
}
-public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model, Integer networkRateKBps) {
+public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model, Integer networkRateKBps, Integer mtu) {
_netType = GuestNetType.BRIDGE;
_sourceName = brName;
_networkName = targetBrName;
_macAddr = macAddr;
_model = model;
_networkRateKBps = networkRateKBps;
+_mtu = mtu;
}
-
-public void defDpdkNet(String dpdkSourcePath, String dpdkPort, String
macAddress, NicModel model,
- Integer networkRateKBps, String extra, String
interfaceMode) {
+ /**
+ * Dpdk Net
Review comment:
Same here, a bit more details would be nice. Otherwise, I think that the
Javadoc block could be removed.
PS: thumbs up for the Javadoc initiative it is always a plus :+1:
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
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346597181
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
##
@@ -1019,21 +1021,26 @@ public String toString() {
private String _dpdkExtraLines;
private String _interfaceMode;
-public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model) {
-defBridgeNet(brName, targetBrName, macAddr, model, 0);
+/**
+ * Bridge Net
Review comment:
Javadoc is always very welcome; however, this one is not aggregating too
much info. Could you please provide a bit more details? Thanks!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346624532 ## File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java ## @@ -1054,7 +1061,7 @@ public void validateCustomParameters(ServiceOfferingVO serviceOffering, Map
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346625196
##
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##
@@ -2083,31 +2090,41 @@ public boolean configure(String name, Map params) throws Configu
Map configs =
_configDao.getConfiguration("AgentManager", params);
-_kvmMtuSize = NumbersUtil.parseInt(_configDao.getValue(KVM_MTU_KEY),
0);
+final List clusterVOs =
_clusterDao.listByHypervisorAndCluster(HypervisorType.KVM,
Cluster.ClusterType.CloudManaged);
+
+if ((clusterVOs != null) && !clusterVOs.isEmpty()) {
Review comment:
CollectionUtils.isEmpty(clusterVOs) would be nice here
:slightly_smiling_face: :+1:
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
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346473236
##
File path:
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##
@@ -247,6 +262,10 @@
@Inject
ConfigurationManager _configMgr;
@Inject
+ClusterDao _clusterDao;
+@Inject
+ClusterDetailsDao _clusterDetailsDao;
Review comment:
I really don't want to sound picky or negative here, just adding a few minor
comments :-)
- those variables could be set to private;
- additionally, could you please remove the underscore ('_') at the
beginning of variables names?
I know that we see this a lot around the CloudStack codebase but this is not
really the best approach. While it's technically legal to begin your variable's
name with "_" or "$", this practice is discouraged by the Java variable naming
convention.
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
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346597181
##
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
##
@@ -1019,21 +1021,26 @@ public String toString() {
private String _dpdkExtraLines;
private String _interfaceMode;
-public void defBridgeNet(String brName, String targetBrName, String
macAddr, NicModel model) {
-defBridgeNet(brName, targetBrName, macAddr, model, 0);
+/**
+ * Bridge Net
Review comment:
Javadoc is always very welcome, I am always in favor; however, this one is
not aggregating too much info. Could you please provide a bit more details?
Thanks!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on a change in pull request #3186: Add possibility to
set KVM MTU size for all NIC
URL: https://github.com/apache/cloudstack/pull/3186#discussion_r346622474
##
File path:
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##
@@ -578,7 +583,17 @@ public void doInTransactionWithoutResult(final
TransactionStatus status) {
_networkOfferingDao.persistDefaultL2NetworkOfferings();
-_kvmMtuSize =
NumbersUtil.parseInt(_configDao.getValue(KvmMtuSize.key()), 0);
+final List clusterVOs =
_clusterDao.listByHypervisorAndCluster(HypervisorType.KVM,
Cluster.ClusterType.CloudManaged);
+
+if ((clusterVOs != null) && !clusterVOs.isEmpty()) {
Review comment:
Can you please update this if conditional to `if
(!CollectionUtils.isEmpty(clusterVOs)) { ... }`?
The "isEmpty" is a Null-safe check; thus, it is a great option for checking
if a collection is empty (returning true if null or empty).
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
With regards,
Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554047905 ping for review - @rhtyd , @nvazquez , @shwstppr , @DaanHoogland , @borisstoyanov 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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554047384 @anuragaw a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554046870 @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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554046454 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-358 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 With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on a change in pull request #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on a change in pull request #3425: [WIP DO NOT MERGE] Better
tracking host maintanence success and failure
URL: https://github.com/apache/cloudstack/pull/3425#discussion_r346508319
##
File path: test/integration/smoke/test_host_maintenance.py
##
@@ -254,25 +301,101 @@ def
test_02_cancel_host_maintenace_with_migration_jobs(self):
self.logger.debug("Creating vms = {}".format(no_vm_req))
self.vmlist = self.createVMs(listHost[0].id, no_vm_req)
-vm_migrating=False
+migrations_finished = True
try:
-
- vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[0].id,
listHost[1].id, self.checkVmMigratingOnHost)
-
- vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[1].id,
listHost[0].id, self.checkVmMigratingOnHost)
+migrations_finished =
self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id)
+
+if migrations_finished:
+migrations_finished =
self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id)
except Exception as e:
self.logger.debug("Exception {}".format(e))
self.fail("Cancel host maintenance failed {}".format(e[0]))
-
-if (vm_migrating == False):
-raise unittest.SkipTest("No VM is migrating and the test will not
be able to check the conditions the test is intended for");
-
-
+
+if (migrations_finished == False):
+raise unittest.SkipTest("VMs are still migrating and the test will
not be able to check the conditions the test is intended for");
+
+return
+
+@attr(
+tags=[
+"advanced",
+"advancedns",
+"smoke",
+"basic",
+"eip",
+"sg"],
+required_hardware="true")
+def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self):
+
+listHost = Host.list(
+self.apiclient,
+type='Routing',
+zoneid=self.zone.id,
+podid=self.pod.id,
+)
+for host in listHost:
+self.logger.debug('2 Hypervisor = {}'.format(host.id))
+
+if (len(listHost) != 2):
+raise unittest.SkipTest("Cancel host maintenance when VMs are
migrating can only be tested with 2 hosts");
+return
+
+target_host_id = listHost[0].id
+other_host_id = listHost[1].id
+
+no_of_vms = self.noOfVMsOnHost(target_host_id)
+
+# Need only 2 VMs for this case.
+if no_of_vms < 2:
+self.logger.debug("Create VMs as there are not enough vms to check
host maintenance")
+no_vm_req = 2 - no_of_vms
+if (no_vm_req > 0):
+self.logger.debug("Creating vms = {}".format(no_vm_req))
+self.vmlist = self.createVMs(listHost[0].id, no_vm_req)
+
+migrations_finished = True
+
+ssh_client = self.get_ssh_client(listHost[1].ipaddress)
+ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW
-m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'")
+ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW
-m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'")
Review comment:
Done
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
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554038613 @anuragaw a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554038431 @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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554033879 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-357 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 With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
anuragaw commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554025507 @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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure
blueorangutan commented on issue #3425: [WIP DO NOT MERGE] Better tracking host maintanence success and failure URL: https://github.com/apache/cloudstack/pull/3425#issuecomment-554025636 @anuragaw a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3659: Fix typo: the past tense of shutdown is shutdown, not shutdowned
GabrielBrascher commented on a change in pull request #3659: Fix typo: the past tense of shutdown is shutdown, not shutdowned URL: https://github.com/apache/cloudstack/pull/3659#discussion_r346482546 ## File path: api/src/main/java/com/cloud/vm/VirtualMachine.java ## @@ -52,7 +52,7 @@ Migrating(true, "VM is being migrated. host id holds to from host"), Error(false, "VM is in error"), Unknown(false, "VM state is unknown."), -Shutdowned(false, "VM is shutdowned from inside"); +Shutdown(false, "VM is shutdown from inside"); Review comment: @rhtyd does it look like a 4.13.1 fix? I had mapped it to 4.13.1.0, but I can target it to master/4.14.0.0; not really a bug fix, more likely an enhancement. 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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused
blueorangutan commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused URL: https://github.com/apache/cloudstack/pull/3617#issuecomment-554021164 Trillian test result (tid-476) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 31357 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3617-t476-kvm-centos7.zip Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py Smoke tests completed. 76 look OK, 1 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_redundant_vpc_site2site_vpn | `Failure` | 391.45 | test_vpc_vpn.py This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3632: add class cleanup method
blueorangutan commented on issue #3632: add class cleanup method URL: https://github.com/apache/cloudstack/pull/3632#issuecomment-553960255 Packaging result: ✖centos6 ✔centos7 ✔debian. JID-356 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 With regards, Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3635: server: acquire IPv4 address when add secondary IP to nic if IP is not specified
weizhouapache commented on a change in pull request #3635: server: acquire IPv4
address when add secondary IP to nic if IP is not specified
URL: https://github.com/apache/cloudstack/pull/3635#discussion_r346405230
##
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##
@@ -732,11 +732,10 @@ public NicSecondaryIp allocateSecondaryGuestIP(final
long nicId, IpAddresses req
}
try {
-if (ipv4Address != null) {
-ipaddr = _ipAddrMgr.allocatePublicIpForGuestNic(network,
podId, ipOwner, ipv4Address);
-}
if (ipv6Address != null) {
ip6addr =
ipv6AddrMgr.allocatePublicIp6ForGuestNic(network, podId, ipOwner, ipv6Address);
+} else {
Review comment:
@DaanHoogland
the method is called only when add a nic to vm.
https://github.com/apache/cloudstack/blob/master/api/src/main/java/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java#L159-L162
at most one ip (ipv4 or ipv6) is passed.
the idea of this PR is, if ipv6 is passed, check and assign an ipv6 address.
It ipv4 is passed or no ip is passed, then assign an ipv4 address.
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
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3669: server: Fix resource count of primary storage/volume because of Expunged volumes
DaanHoogland commented on issue #3669: server: Fix resource count of primary storage/volume because of Expunged volumes URL: https://github.com/apache/cloudstack/pull/3669#issuecomment-553954929 @rakgenius @weizhouapache makes perfect sense. Have you looked at the usage server and billing verification? This could be needed if events don't get logged for the volumes. 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 With regards, Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3179: Redundant VRouter guest network on wrong interface
weizhouapache commented on issue #3179: Redundant VRouter guest network on wrong interface URL: https://github.com/apache/cloudstack/issues/3179#issuecomment-553952656 @DennisKonrad I reverted #2304, and it works 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 With regards, Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3632: add class cleanup method
DaanHoogland commented on issue #3632: add class cleanup method URL: https://github.com/apache/cloudstack/pull/3632#issuecomment-553950690 @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 With regards, Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3632: add class cleanup method
blueorangutan commented on issue #3632: add class cleanup method URL: https://github.com/apache/cloudstack/pull/3632#issuecomment-553950773 @DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3635: server: acquire IPv4 address when add secondary IP to nic if IP is not specified
DaanHoogland commented on a change in pull request #3635: server: acquire IPv4
address when add secondary IP to nic if IP is not specified
URL: https://github.com/apache/cloudstack/pull/3635#discussion_r346391870
##
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##
@@ -732,11 +732,10 @@ public NicSecondaryIp allocateSecondaryGuestIP(final
long nicId, IpAddresses req
}
try {
-if (ipv4Address != null) {
-ipaddr = _ipAddrMgr.allocatePublicIpForGuestNic(network,
podId, ipOwner, ipv4Address);
-}
if (ipv6Address != null) {
ip6addr =
ipv6AddrMgr.allocatePublicIp6ForGuestNic(network, podId, ipOwner, ipv6Address);
+} else {
Review comment:
I'm a bit confused, the null check for ipv4address is gone and it is not
assigned (even when not null) when it isn't but ipv6Address is not null. This
means that if both are available ipv6 is the *only one* used. Is that what you
intend @ustcweizhou ?
I see how this fixes backwards compatibility btw, just wondering if we would
want to allow dualstack!?
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
With regards,
Apache Git Services
[GitHub] [cloudstack] DennisKonrad commented on issue #3179: Redundant VRouter guest network on wrong interface
DennisKonrad commented on issue #3179: Redundant VRouter guest network on wrong
interface
URL: https://github.com/apache/cloudstack/issues/3179#issuecomment-553940664
@weizhouapache I found that the json the router receives really does not
have the "nic_dev_id" set at all. Therefor the script fails because the value
has "null" asigned then.
**redundant VPC**
`{"ip_address":[
{"public_ip":"172.20.20.12","source_nat":true,"add":true,"one_to_one_nat":false,"first_i_p":true,"gateway":"172.20.0.1","netmask":"255.255.0.0","vif_mac_address":"1e:00:d5:00:00:c2","new_nic":false,"nw_type":"Public"}],"type":"ips"}
**non redundant VPC**
{"ip_address":[
{"public_ip":"172.20.20.13","source_nat":true,"add":true,"one_to_one_nat":false,"first_i_p":true,"gateway":"172.20.0.1","netmask":"255.255.0.0","vif_mac_address":"1e:00:f4:00:00:c3",**"nic_dev_id":1**,"new_nic":false,"nw_type":"Public"}],"type":"ips"}`
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
With regards,
Apache Git Services
[GitHub] [cloudstack] ustcweizhou opened a new pull request #3688: New feature: Add support to destroy/recover volumes
ustcweizhou opened a new pull request #3688: New feature: Add support to destroy/recover volumes URL: https://github.com/apache/cloudstack/pull/3688 ## Description cloudstack provides options to destroy and recover a vm. For volumes, we do not have similar operations. When we delete a volume, it will be removed from primary storage permanently. With this change, (1) we have option to destroy a volume (expunge=false) or delete a volume permanently (expunge=true) (2) we are be able to recover a Destroy volume to Ready state. (3) Destroy volume will be removed in storage clean up. (4) when reinstall a vm, the old root volume will be Destroy state. It can be recovered to Ready state and attached to other vm so we can restore some data. ## Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [X] New feature (non-breaking change which adds functionality) - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) ## Screenshots (if appropriate): ## How Has This Been Tested? destroy vm (expunge=false) recover a Destroy vm delete vm (same as destroy vm with expunge=True) More test cases can be found in the integration 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 With regards, Apache Git Services
[GitHub] [cloudstack] GabrielBrascher edited a comment on issue #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher edited a comment on issue #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#issuecomment-553909811 @svenvogel I am testing it right now; should have feedback soon (based on code and manual 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 With regards, Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on issue #3186: Add possibility to set KVM MTU size for all NIC
GabrielBrascher commented on issue #3186: Add possibility to set KVM MTU size for all NIC URL: https://github.com/apache/cloudstack/pull/3186#issuecomment-553909811 @svenvogel I am testing it right now; should have a feedback soon (based on code and manual 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 With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb merged pull request #3682: ui: fix migrate host form no host popup
andrijapanicsb merged pull request #3682: ui: fix migrate host form no host popup URL: https://github.com/apache/cloudstack/pull/3682 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 With regards, Apache Git Services
[cloudstack] branch master updated (96d98de -> c8681f5)
This is an automated email from the ASF dual-hosted git repository. andrijapanic pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from 96d98de Merge remote-tracking branch 'origin/4.13' add c8681f5 ui: fix migrate host form no host popup (#3682) No new revisions were added by this update. Summary of changes: ui/scripts/instances.js | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-)
[GitHub] [cloudstack] andrijapanicsb commented on issue #3682: ui: fix migrate host form no host popup
andrijapanicsb commented on issue #3682: ui: fix migrate host form no host popup URL: https://github.com/apache/cloudstack/pull/3682#issuecomment-553891135 I have done that Rohit check my previous comment. 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 With regards, Apache Git Services
[GitHub] [cloudstack] rhtyd commented on issue #3682: ui: fix migrate host form no host popup
rhtyd commented on issue #3682: ui: fix migrate host form no host popup URL: https://github.com/apache/cloudstack/pull/3682#issuecomment-553888766 This can be merged if someone can manually QA/test to confirm. cc @andrijapanicsb @borisstoyanov 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 With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov opened a new issue #3687: Domain admins not able to add LDAP users
borisstoyanov opened a new issue #3687: Domain admins not able to add LDAP users URL: https://github.com/apache/cloudstack/issues/3687 # ISSUE TYPE * Improvement Request # COMPONENT NAME ~~~ UI, API ~~~ # CLOUDSTACK VERSION ~~~ 4.14 ~~~ # SUMMARY Domain admins are not able to see the ‘Add LDAP Account' button at both “Account” tab and Domain - > Accounts tab. It would be really good if they can manage that as well, since they manage native users within the domain 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 With regards, Apache Git Services
[GitHub] [cloudstack] anuragaw commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router
anuragaw commented on issue #3575: [WIP DO NOT MERGE] Health check feature for virtual router URL: https://github.com/apache/cloudstack/pull/3575#issuecomment-553844262 ping for review - @rhtyd , @nvazquez , @shwstppr , @DaanHoogland. Thanks guys! 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 With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov commented on issue #3686: Domain level ldap server configuration is not displayed
borisstoyanov commented on issue #3686: Domain level ldap server configuration is not displayed URL: https://github.com/apache/cloudstack/issues/3686#issuecomment-553839977 I believe there's not such option: ``` (localcloud) SBCM5> > list ldapusers listtype= (localcloud) SBCM5> > list ldapusers listtype=all (localcloud) SBCM5> > list ldapusers domainid= filter= keyword=listtype= page= pagesize= userfilter= ``` 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 With regards, Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3179: Redundant VRouter guest network on wrong interface
weizhouapache commented on issue #3179: Redundant VRouter guest network on wrong interface URL: https://github.com/apache/cloudstack/issues/3179#issuecomment-553839855 @DennisKonrad I will test it as well 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 With regards, Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3686: Domain level ldap server configuration is not displayed
weizhouapache commented on issue #3686: Domain level ldap server configuration is not displayed URL: https://github.com/apache/cloudstack/issues/3686#issuecomment-553839109 @borisstoyanov listall=true ? 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 With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov opened a new issue #3686: Domain level ldap server configuration is not displayed
borisstoyanov opened a new issue #3686: Domain level ldap server configuration
is not displayed
URL: https://github.com/apache/cloudstack/issues/3686
# ISSUE TYPE
* Bug Report
# COMPONENT NAME
~~~
UI
~~~
# CLOUDSTACK VERSION
~~~
4.14
~~~
# SUMMARY
Configure LDAP at a domain level and you won't be able to see this
configuration anywhere in the UI, at Global Setting -> LDAP it's being
displayed only root domain level configurations
Looks like list api is dependent on the domainid, and if no domain id is
passed it ignores the results.
```
(localcloud) SBCM5> > add ldapconfiguration hostname=10.1.2.5 port=389
domainid=7cc39a93-a6a7-423c-b150-50f63b8765d6
{
"LdapAddConfiguration": {
"domainid": "7cc39a93-a6a7-423c-b150-50f63b8765d6",
"hostname": "10.1.2.5",
"port": 389
}
}
(localcloud) SBCM5> > list ldapconfigurations
(localcloud) SBCM5> > list ldapconfigurations
domainid=7cc39a93-a6a7-423c-b150-50f63b8765d6
{
"LdapConfiguration": [
{
"domainid": "7cc39a93-a6a7-423c-b150-50f63b8765d6",
"hostname": "10.1.2.5",
"port": 389
}
],
"count": 1
}
```
![Uploading Screenshot 2019-11-14 at 12.52.50.png…]()
# EXPECTED RESULTS
~~~
As an admin I would like to see all level of configurations
~~~
# ACTUAL RESULTS
~~~
Not being listed
~~~
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
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused
blueorangutan commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused URL: https://github.com/apache/cloudstack/pull/3617#issuecomment-553801212 @andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack] andrijapanicsb commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused
andrijapanicsb commented on issue #3617: [KVM] Agent LB Fix: Connections from disabled KVM host agents are refused URL: https://github.com/apache/cloudstack/pull/3617#issuecomment-553800862 @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 With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov opened a new issue #3685: no way to unlink account/domain from ldap
borisstoyanov opened a new issue #3685: no way to unlink account/domain from ldap URL: https://github.com/apache/cloudstack/issues/3685 ``` (localcloud) SBCM5> > link domaintoldap domainid=5a155ee3-70dd-4549-9d29-4c926530a8f0 type=group ldapdomain=cn=admins,ou=groups,dc=acs,dc=echt,dc=net accounttype=2 Error: (HTTP 530, error code ) Entity already exists: ``` User is able to link domain/account to LDAP but there's no way to remove this link if not deleting it in DB. 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 With regards, Apache Git Services
[GitHub] [cloudstack] borisstoyanov opened a new issue #3684: With Chrome you cannot edit domain level settings
borisstoyanov opened a new issue #3684: With Chrome you cannot edit domain level settings URL: https://github.com/apache/cloudstack/issues/3684 Chrome version: Version 78.0.3904.97 (Official Build) (64-bit) Automatically update Chrome for all users Learn more  Probably a scroller is 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 With regards, Apache Git Services
