[GitHub] blueorangutan commented on issue #2613: Cleanup POMs
blueorangutan commented on issue #2613: Cleanup POMs URL: https://github.com/apache/cloudstack/pull/2613#issuecomment-404912938 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2172 This is an automated message from the Apache Git Service. To respond to the message, please log on 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] blueorangutan commented on issue #2613: Cleanup POMs
blueorangutan commented on issue #2613: Cleanup POMs URL: https://github.com/apache/cloudstack/pull/2613#issuecomment-404904385 @rhtyd 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 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] rhtyd commented on issue #2613: Cleanup POMs
rhtyd commented on issue #2613: Cleanup POMs URL: https://github.com/apache/cloudstack/pull/2613#issuecomment-404904342 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on 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] smeetsr opened a new pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
smeetsr opened a new pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset URL: https://github.com/apache/cloudstack/pull/2743 ## Description * When password reset is called, and VR accepts it, set vm updateParameters to false. * When password reset is called, and SSH key is set, make sure the password isn't removed again. * Fix Marvin tests * Make sure saveDetails keeps the display value of the existing details. ## 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) - [x] Cleanup (Code refactoring and cleanup, that may add test cases) ## How Has This Been Tested? Manual + automated marvin nuagevsp tests + component config drive ## Checklist: - [x] I have read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document. - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. Testing - [x] I have added tests to cover my changes. - [ ] All relevant new and existing integration tests have passed. - [ ] A full integration testsuite with all test that can run on my environment has passed. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] khos2ow commented on issue #2613: Cleanup POMs
khos2ow commented on issue #2613: Cleanup POMs URL: https://github.com/apache/cloudstack/pull/2613#issuecomment-404858588 @rhtyd rebased. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202358111
##
File path: test/integration/smoke/test_async_job.py
##
@@ -0,0 +1,136 @@
+# 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.
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources
+from marvin.lib.base import ServiceOffering, DiskOffering, Account,
VirtualMachine,\
+queryAsyncJobResult, PASS
+from marvin.lib.common import get_domain, get_zone, get_test_template
+from pytz import timezone
+
+
+class TestAsyncJob(cloudstackTestCase):
+"""
+Test queryAsyncJobResult
+"""
+@classmethod
+def setUpClass(cls):
+cls.testClient = super(TestAsyncJob, cls).getClsTestClient()
+cls.api_client = cls.testClient.getApiClient()
+
+cls.testdata = cls.testClient.getParsedTestDataConfig()
+# Get Zone, Domain and templates
+cls.domain = get_domain(cls.api_client)
+cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+cls.hypervisor = cls.testClient.getHypervisorInfo()
+
+cls.template = get_test_template(
+cls.api_client,
+cls.zone.id,
+cls.hypervisor
+)
+
+# Create service, disk offerings etc
+cls.service_offering = ServiceOffering.create(
+cls.api_client,
+cls.testdata["service_offering"]
+)
+
+cls.disk_offering = DiskOffering.create(
+cls.api_client,
+cls.testdata["disk_offering"]
+)
+
+cls._cleanup = [
+cls.service_offering,
+cls.disk_offering
+]
+
+@classmethod
+def tearDownClass(cls):
+try:
+cleanup_resources(cls.api_client, cls._cleanup)
+except Exception as exception:
+raise Exception("Warning: Exception during cleanup : %s" %
exception)
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.testdata["virtual_machine"]["zoneid"] = self.zone.id
+self.testdata["virtual_machine"]["template"] = self.template.id
+self.testdata["iso"]["zoneid"] = self.zone.id
+self.account = Account.create(
+self.apiclient,
+self.testdata["account"],
+domainid=self.domain.id
+)
+self.cleanup = [self.account]
+
+def tearDown(self):
+try:
+self.debug("Cleaning up the resources")
+cleanup_resources(self.apiclient, self.cleanup)
+self.debug("Cleanup complete!")
+except Exception as exception:
+self.debug("Warning! Exception in tearDown: %s" % exception)
+
+@attr(tags=["advanced", "eip", "advancedns", "basic", "sg"],
required_hardware="false")
+def test_query_async_job_result(self):
+"""
+Test queryAsyncJobResult API for expected values
+"""
+self.debug("Deploying instance in the account: %s" %
+ self.account.name)
+virtual_machine = VirtualMachine.create(
+self.apiclient,
+self.testdata["virtual_machine"],
+accountid=self.account.name,
+domainid=self.account.domainid,
+serviceofferingid=self.service_offering.id,
+diskofferingid=self.disk_offering.id,
+hypervisor=self.hypervisor
+)
+
+response = virtual_machine.getState(
+self.apiclient,
+VirtualMachine.RUNNING)
+self.assertEqual(response[0], PASS, response[1])
+
+cmd = queryAsyncJobResult.queryAsyncJobResultCmd()
+cmd.jobid = virtual_machine.jobid
+cmd_response = self.apiclient.queryAsyncJobResult(cmd)
+
+db_result = self.dbclient.execute("select * from async_job where
uuid='%s'" %
+ virtual_machine.jobid)
+
+# verify that 'completed' value from api equals 'removed' db column
value
Review comment:
[GitHub] rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime
rafaelweingartner commented on a change in pull request #2739: Async jobs add
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202353309
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
That is exactly my point. I have seen this indiscriminate use of the `final`
keyword in some other places as if it would greatly improve our code base. This
is also not the first time I call attention regarding the use of final.
I really do not understand when I see people slapping `final` keywords in a
method that has 100+ lines that can be extracted into smaller blocks. Then, we
hear that it is a defensive programming style or some sort of best practice. I
know, those expressions sound nice and shiny, but simply adding the `final`
keyword makes no improvement what so ever. A poorly coded method will remain
poorly coded with or without `final` keywords in its variables.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2734: Fix invalid consoleproxy url after upgrade
rafaelweingartner commented on a change in pull request #2734: Fix invalid
consoleproxy url after upgrade
URL: https://github.com/apache/cloudstack/pull/2734#discussion_r202344815
##
File path: core/src/com/cloud/info/ConsoleProxyInfo.java
##
@@ -35,38 +35,40 @@ public ConsoleProxyInfo(int proxyUrlPort) {
public ConsoleProxyInfo(boolean sslEnabled, String proxyIpAddress, int
port, int proxyUrlPort, String consoleProxyUrlDomain) {
this.sslEnabled = sslEnabled;
+this.proxyPort = port;
+this.proxyUrlPort = proxyUrlPort;
+this.proxyAddress = this.formatProxyAddress(consoleProxyUrlDomain,
proxyIpAddress);
if (sslEnabled) {
-StringBuffer sb = new StringBuffer();
-if (consoleProxyUrlDomain.startsWith("*")) {
-sb.append(proxyIpAddress);
-for (int i = 0; i < proxyIpAddress.length(); i++)
-if (sb.charAt(i) == '.')
-sb.setCharAt(i, '-');
-sb.append(consoleProxyUrlDomain.substring(1));//skip the *
-} else {
-//LB address
-sb.append(consoleProxyUrlDomain);
-}
-proxyAddress = sb.toString();
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
-
proxyImageUrl = "https://; + proxyAddress;
-if (proxyUrlPort != 443)
+if (proxyUrlPort != 443) {
proxyImageUrl += ":" + this.proxyUrlPort;
-} else {
-proxyAddress = proxyIpAddress;
-if (StringUtils.isNotBlank(consoleProxyUrlDomain)) {
-proxyAddress = consoleProxyUrlDomain;
}
-proxyPort = port;
-this.proxyUrlPort = proxyUrlPort;
+} else {
proxyImageUrl = "http://; + proxyAddress;
-if (proxyUrlPort != 80)
+if (proxyUrlPort != 80) {
proxyImageUrl += ":" + proxyUrlPort;
+}
+}
+}
+
+private String formatProxyAddress(String consoleProxyUrlDomain, String
proxyIpAddress) {
Review comment:
@resmo you already did a lot! I would like to see a documentation (instead
of inline comments) and unit tests for this method though.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202342570
##
File path: test/integration/smoke/test_async_job.py
##
@@ -0,0 +1,136 @@
+# 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.
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources
+from marvin.lib.base import ServiceOffering, DiskOffering, Account,
VirtualMachine,\
+queryAsyncJobResult, PASS
+from marvin.lib.common import get_domain, get_zone, get_test_template
+from pytz import timezone
+
+
+class TestAsyncJob(cloudstackTestCase):
+"""
+Test queryAsyncJobResult
+"""
+@classmethod
+def setUpClass(cls):
+cls.testClient = super(TestAsyncJob, cls).getClsTestClient()
+cls.api_client = cls.testClient.getApiClient()
+
+cls.testdata = cls.testClient.getParsedTestDataConfig()
+# Get Zone, Domain and templates
+cls.domain = get_domain(cls.api_client)
+cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+cls.hypervisor = cls.testClient.getHypervisorInfo()
+
+cls.template = get_test_template(
+cls.api_client,
+cls.zone.id,
+cls.hypervisor
+)
+
+# Create service, disk offerings etc
+cls.service_offering = ServiceOffering.create(
+cls.api_client,
+cls.testdata["service_offering"]
+)
+
+cls.disk_offering = DiskOffering.create(
+cls.api_client,
+cls.testdata["disk_offering"]
+)
+
+cls._cleanup = [
+cls.service_offering,
+cls.disk_offering
+]
+
+@classmethod
+def tearDownClass(cls):
+try:
+cleanup_resources(cls.api_client, cls._cleanup)
+except Exception as exception:
+raise Exception("Warning: Exception during cleanup : %s" %
exception)
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.testdata["virtual_machine"]["zoneid"] = self.zone.id
+self.testdata["virtual_machine"]["template"] = self.template.id
+self.testdata["iso"]["zoneid"] = self.zone.id
+self.account = Account.create(
+self.apiclient,
+self.testdata["account"],
+domainid=self.domain.id
+)
+self.cleanup = [self.account]
+
+def tearDown(self):
+try:
+self.debug("Cleaning up the resources")
+cleanup_resources(self.apiclient, self.cleanup)
+self.debug("Cleanup complete!")
+except Exception as exception:
+self.debug("Warning! Exception in tearDown: %s" % exception)
+
+@attr(tags=["advanced", "eip", "advancedns", "basic", "sg"],
required_hardware="false")
+def test_query_async_job_result(self):
+"""
+Test queryAsyncJobResult API for expected values
+"""
+self.debug("Deploying instance in the account: %s" %
+ self.account.name)
+virtual_machine = VirtualMachine.create(
+self.apiclient,
+self.testdata["virtual_machine"],
+accountid=self.account.name,
+domainid=self.account.domainid,
+serviceofferingid=self.service_offering.id,
+diskofferingid=self.disk_offering.id,
+hypervisor=self.hypervisor
+)
+
+response = virtual_machine.getState(
+self.apiclient,
+VirtualMachine.RUNNING)
+self.assertEqual(response[0], PASS, response[1])
+
+cmd = queryAsyncJobResult.queryAsyncJobResultCmd()
+cmd.jobid = virtual_machine.jobid
+cmd_response = self.apiclient.queryAsyncJobResult(cmd)
+
+db_result = self.dbclient.execute("select * from async_job where
uuid='%s'" %
+ virtual_machine.jobid)
+
+# verify that 'completed' value from api equals 'removed' db column
value
Review
[GitHub] DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202339018
##
File path: server/src/test/java/com/cloud/storage/dao/AsyncJobJoinDaoTest.java
##
@@ -0,0 +1,89 @@
+/*
+ * 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.storage.dao;
+
+import com.cloud.api.query.dao.AsyncJobJoinDaoImpl;
+import com.cloud.api.query.vo.AsyncJobJoinVO;
+import org.apache.cloudstack.api.ApiCommandJobType;
+import org.apache.cloudstack.api.response.AsyncJobResponse;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import java.util.Date;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AsyncJobJoinDaoTest {
+
+@InjectMocks
+AsyncJobJoinDaoImpl dao;
+
+@Test
+public void testNewAsyncJobResponseValidValues() {
+final AsyncJobJoinVO job = new AsyncJobJoinVO();
+
ReflectionTestUtils.setField(job,"uuid","a2b22932-1b61-4406-8e89-4ae19968e8d3");
+
ReflectionTestUtils.setField(job,"accountUuid","4dea2836-72cc-11e8-b2de-107b4429825a");
+
ReflectionTestUtils.setField(job,"domainUuid","4dea136b-72cc-11e8-b2de-107b4429825a");
+
ReflectionTestUtils.setField(job,"userUuid","4decc724-72cc-11e8-b2de-107b4429825a");
+
ReflectionTestUtils.setField(job,"cmd","org.apache.cloudstack.api.command.admin.vm.StartVMCmdByAdmin");
+ReflectionTestUtils.setField(job,"status",0);
+ReflectionTestUtils.setField(job,"resultCode",0);
+ReflectionTestUtils.setField(job,"result",null);
+ReflectionTestUtils.setField(job,"created",new Date());
+ReflectionTestUtils.setField(job,"removed",new Date());
+
ReflectionTestUtils.setField(job,"instanceType",ApiCommandJobType.VirtualMachine);
+ReflectionTestUtils.setField(job,"instanceId",3L);
+final AsyncJobResponse response = dao.newAsyncJobResponse(job);
+Assert.assertEquals(job.getUuid(),response.getJobId());
+Assert.assertEquals(job.getAccountUuid(),
ReflectionTestUtils.getField(response, "accountId"));
+Assert.assertEquals(job.getUserUuid(),
ReflectionTestUtils.getField(response, "userId"));
+Assert.assertEquals(job.getCmd(),
ReflectionTestUtils.getField(response, "cmd"));
+Assert.assertEquals(job.getStatus(),
ReflectionTestUtils.getField(response, "jobStatus"));
+Assert.assertEquals(job.getStatus(),
ReflectionTestUtils.getField(response, "jobProcStatus"));
+Assert.assertEquals(job.getResultCode(),
ReflectionTestUtils.getField(response, "jobResultCode"));
+Assert.assertEquals(null, ReflectionTestUtils.getField(response,
"jobResultType"));
+Assert.assertEquals(job.getResult(),
ReflectionTestUtils.getField(response, "jobResult"));
+Assert.assertEquals(job.getInstanceType().toString(),
ReflectionTestUtils.getField(response, "jobInstanceType"));
+Assert.assertEquals(job.getInstanceUuid(),
ReflectionTestUtils.getField(response, "jobInstanceId"));
+Assert.assertEquals(job.getCreated(),
ReflectionTestUtils.getField(response, "created"));
+Assert.assertEquals(job.getRemoved(),
ReflectionTestUtils.getField(response, "removed"));
+}
+
+@Test
+public void testNewAsyncJobResponseNullValues() {
+final AsyncJobJoinVO job = new AsyncJobJoinVO();
+final AsyncJobResponse response = dao.newAsyncJobResponse(job);
+Assert.assertEquals(job.getUuid(),response.getJobId());
+Assert.assertEquals(job.getAccountUuid(),
ReflectionTestUtils.getField(response, "accountId"));
Review comment:
very good, not assuming the nulls ;)
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
DaanHoogland commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202337269
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Actually it is kind of kidding yourself in this case, as the object ref may
not be changed after setting but the state can still be changed. I would advice
using a final filter against this as a spam filter against spam; ignore, don't
read.
In this case it is best to follow @rafaelweingartner 's suggestion, to make
the new and/or changed code be in smaller methods and leave the existing alone.
Though refactoring the whole method is an option as well of course ;)
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202335074 ## File path: test/integration/smoke/test_async_job.py ## @@ -0,0 +1,139 @@ +""" +Integration Test +""" +# 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. + +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import cleanup_resources +from marvin.lib.base import ServiceOffering, DiskOffering, Account, VirtualMachine,\ +queryAsyncJobResult, PASS +from marvin.lib.common import get_domain, get_zone, get_test_template +from pytz import timezone + + +class TestAsyncJob(cloudstackTestCase): Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202329269
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Best practice is to make objects immutable and it makes the code safer and
more predictable. I agree that the methods should be much shorter in the first
place.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ernjvr commented on issue #2739: Async jobs add endtime
ernjvr commented on issue #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#issuecomment-404818065 @dhlaluku please review. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202329269
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Best practice is to make objects immutable and it makes the code safer and
more predictable.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202326838
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1025,21 +1027,24 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
// purge sync queue item running on this ms node
_queueMgr.cleanupActiveQueueItems(msid, true);
// reset job status for all jobs running on this ms node
-List jobs = _jobDao.getResetJobs(msid);
-for (AsyncJobVO job : jobs) {
+final List jobs = _jobDao.getResetJobs(msid);
+for (final AsyncJobVO job : jobs) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Cancel left-over job-" +
job.getId());
}
job.setStatus(JobInfo.Status.FAILED);
job.setResultCode(ApiErrorCode.INTERNAL_ERROR.getHttpCode());
job.setResult("job cancelled because of management
server restart or shutdown");
job.setCompleteMsid(msid);
+final Date currentGMTTime = DateUtil.currentGMTTime();
+job.setLastUpdated(currentGMTTime);
+job.setRemoved(currentGMTTime);
_jobDao.update(job.getId(), job);
if (s_logger.isDebugEnabled()) {
s_logger.debug("Purge queue item for cancelled
job-" + job.getId());
}
_queueMgr.purgeAsyncJobQueueItemId(job.getId());
-if
(job.getInstanceType().equals(ApiCommandJobType.Volume.toString())) {
+if (job.getInstanceType() != null &&
job.getInstanceType().equals(ApiCommandJobType.Volume.toString())) {
Review comment:
This is a better way. Thanks.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] ernjvr commented on a change in pull request #2739: Async jobs add endtime
ernjvr commented on a change in pull request #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202325643 ## File path: engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql ## @@ -32,4 +32,6 @@ ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255); -- We are only adding the permission to the default rules. Any custom rule must be configured by the root admin. INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 2, 'moveNetworkAclItem', 'ALLOW', 100) ON DUPLICATE KEY UPDATE rule=rule; INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 3, 'moveNetworkAclItem', 'ALLOW', 302) ON DUPLICATE KEY UPDATE rule=rule; -INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule; \ No newline at end of file +INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule; + +UPDATE `cloud`.`async_job` SET `removed` = now() WHERE `removed` IS NULL; Review comment: yes This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime
rafaelweingartner commented on a change in pull request #2739: Async jobs add
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202324106
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Sorry if I sound blunt, but yes I know this "excuse". My question is, what
are the cases that the use of these final keywords is helping us to avoid?
Is it the case that we have these monstrous methods (with 100+, or 500+
lines)? And then you are trying to help programmers to identify when he/she is
changing a variable that was previously assigned somewhere else in the middle
of a big chunk of code.
If that is the only reason for us to use this keyword, I do prefer other
options such as using shorter and concise methods, which can be unit tested.
Then, we can catch problems such as wrong assignments. Otherwise, we are just
adding code that is not needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2721: api: Introducing a new diagnostics API command for system VMs for CloudStack admins
rhtyd commented on issue #2721: api: Introducing a new diagnostics API command for system VMs for CloudStack admins URL: https://github.com/apache/cloudstack/pull/2721#issuecomment-404806007 Merging this based on code reviews and testing. The failures are not related to this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rhtyd commented on a change in pull request #2739: Async jobs add endtime
rhtyd commented on a change in pull request #2739: Async jobs add endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202321858
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
I think it's a good defensive programming style to keep assignment
restrictions unless needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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: diagnostics: new diagnostics admin API for system VMs (#2721)
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
The following commit(s) were added to refs/heads/master by this push:
new 40af32b diagnostics: new diagnostics admin API for system VMs (#2721)
40af32b is described below
commit 40af32b1b98e5ab6229531a151381b1f4f1f2910
Author: Dingane Hlaluku
AuthorDate: Fri Jul 13 13:28:45 2018 +0200
diagnostics: new diagnostics admin API for system VMs (#2721)
This is a new feature for CS that allows Admin users improved
troubleshooting of network issues in CloudStack hosted networks.
Description: For troubleshooting purposes, CloudStack administrators may
wish to execute network utility commands remotely on system VMs, or request
system VMs to ping/traceroute/arping to specific addresses over specific
interfaces. An API command to provide such functionalities is being developed
without altering any existing APIs. The targeted system VMs for this feature
are the Virtual Router (VR), Secondary Storage VM (SSVM) and the Console Proxy
VM (CPVM).
FS:
https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+Remote+Diagnostics+API
ML discussion:
https://markmail.org/message/xt7owmb2c6iw7tva
---
.travis.yml| 3 +-
.../org/apache/cloudstack/api/ApiConstants.java| 5 +
.../admin/diagnostics/RunDiagnosticsCmd.java | 136 ++
.../api/response/RunDiagnosticsResponse.java | 67 +++
.../cloudstack/diagnostics/DiagnosticsService.java | 29 ++
.../cloudstack/diagnostics/DiagnosticsType.java| 42 ++
.../agent/resource/virtualnetwork/VRScripts.java | 1 +
.../virtualnetwork/VirtualRoutingResource.java | 18 +-
.../cloudstack/diagnostics/DiagnosticsAnswer.java | 54 +++
.../cloudstack/diagnostics/DiagnosticsCommand.java | 44 ++
.../com/cloud/agent/manager/MockAgentManager.java | 3 +
.../cloud/agent/manager/MockAgentManagerImpl.java | 56 ++-
.../cloud/agent/manager/SimulatorManagerImpl.java | 5 +-
.../diagnostics/DiagnosticsServiceImpl.java| 131 +
.../core/spring-server-core-managers-context.xml | 2 +
.../diagnostics/DiagnosticsServiceImplTest.java| 196
systemvm/debian/opt/cloud/bin/diagnostics.py | 71 +++
test/integration/smoke/test_diagnostics.py | 539 +
tools/apidoc/gen_toc.py| 3 +-
19 files changed, 1377 insertions(+), 28 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 2d32324..130e907 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,7 +38,7 @@ env:
matrix:
# Keep the TESTS sorted by name and grouped by type
- TESTS="smoke/test_certauthority_root"
-
+
- TESTS="smoke/test_accounts
smoke/test_affinity_groups
smoke/test_affinity_groups_projects
@@ -47,6 +47,7 @@ env:
smoke/test_deploy_vm_root_resize
smoke/test_deploy_vm_with_userdata
smoke/test_deploy_vms_with_varied_deploymentplanners
+ smoke/test_diagnostics
smoke/test_disk_offerings
smoke/test_dynamicroles
smoke/test_global_settings
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index 504b214..1a57313 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -719,6 +719,11 @@ public class ApiConstants {
public static final String LAST_ANNOTATED = "lastannotated";
public static final String LDAP_DOMAIN = "ldapdomain";
+public static final String STDOUT = "stdout";
+public static final String STDERR = "stderr";
+public static final String EXITCODE = "exitcode";
+public static final String TARGET_ID = "targetid";
+
public enum HostDetails {
all, capacity, events, stats, min;
}
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
new file mode 100644
index 000..bb1ddf5
--- /dev/null
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
@@ -0,0 +1,136 @@
+// 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
[GitHub] rhtyd closed pull request #2721: api: Introducing a new diagnostics API command for system VMs for CloudStack admins
rhtyd closed pull request #2721: api: Introducing a new diagnostics API command
for system VMs for CloudStack admins
URL: https://github.com/apache/cloudstack/pull/2721
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/.travis.yml b/.travis.yml
index f4b52bae945..0474e7adb3d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -34,7 +34,7 @@ env:
matrix:
# Keep the TESTS sorted by name and grouped by type
- TESTS="smoke/test_certauthority_root"
-
+
- TESTS="smoke/test_accounts
smoke/test_affinity_groups
smoke/test_affinity_groups_projects
@@ -43,6 +43,7 @@ env:
smoke/test_deploy_vm_root_resize
smoke/test_deploy_vm_with_userdata
smoke/test_deploy_vms_with_varied_deploymentplanners
+ smoke/test_diagnostics
smoke/test_disk_offerings
smoke/test_dynamicroles
smoke/test_global_settings
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index 504b2149837..1a57313eaa7 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -719,6 +719,11 @@
public static final String LAST_ANNOTATED = "lastannotated";
public static final String LDAP_DOMAIN = "ldapdomain";
+public static final String STDOUT = "stdout";
+public static final String STDERR = "stderr";
+public static final String EXITCODE = "exitcode";
+public static final String TARGET_ID = "targetid";
+
public enum HostDetails {
all, capacity, events, stats, min;
}
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
new file mode 100644
index 000..bb1ddf57cb7
--- /dev/null
+++
b/api/src/main/java/org/apache/cloudstack/api/command/admin/diagnostics/RunDiagnosticsCmd.java
@@ -0,0 +1,136 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.diagnostics;
+
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
+import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.RunDiagnosticsResponse;
+import org.apache.cloudstack.api.response.SystemVmResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.diagnostics.DiagnosticsService;
+import org.apache.cloudstack.diagnostics.DiagnosticsType;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.log4j.Logger;
+
+import javax.inject.Inject;
+import java.util.Collections;
+import java.util.Map;
+
+@APICommand(name = RunDiagnosticsCmd.APINAME, responseObject =
RunDiagnosticsResponse.class, entityType = {VirtualMachine.class},
+responseHasSensitiveInfo = false,
+requestHasSensitiveInfo = false,
+description = "Execute network-utility command (ping/arping/tracert)
on system VMs remotely",
+authorized = {RoleType.Admin},
+since = "4.12.0.0")
+public class RunDiagnosticsCmd extends BaseCmd {
+private static final Logger LOGGER =
Logger.getLogger(RunDiagnosticsCmd.class);
+public static final String APINAME = "runDiagnostics";
+
+@Inject
+private DiagnosticsService diagnosticsService;
+
+/
+ API parameters
[GitHub] rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS)
rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS) URL: https://github.com/apache/cloudstack/pull/2524#issuecomment-404806324 @GabrielBrascher did you approve the PR? I think everything is fine to merge then This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rafaelweingartner commented on issue #2703: Fix concurrency problem when moving ACL rules with drag
rafaelweingartner commented on issue #2703: Fix concurrency problem when moving ACL rules with drag URL: https://github.com/apache/cloudstack/pull/2703#issuecomment-404805765 @borisstoyanov do you know why the packaging failed? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime
rafaelweingartner commented on a change in pull request #2739: Async jobs add
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202319599
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1025,21 +1027,24 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
// purge sync queue item running on this ms node
_queueMgr.cleanupActiveQueueItems(msid, true);
// reset job status for all jobs running on this ms node
-List jobs = _jobDao.getResetJobs(msid);
-for (AsyncJobVO job : jobs) {
+final List jobs = _jobDao.getResetJobs(msid);
+for (final AsyncJobVO job : jobs) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Cancel left-over job-" +
job.getId());
}
job.setStatus(JobInfo.Status.FAILED);
job.setResultCode(ApiErrorCode.INTERNAL_ERROR.getHttpCode());
job.setResult("job cancelled because of management
server restart or shutdown");
job.setCompleteMsid(msid);
+final Date currentGMTTime = DateUtil.currentGMTTime();
+job.setLastUpdated(currentGMTTime);
+job.setRemoved(currentGMTTime);
_jobDao.update(job.getId(), job);
if (s_logger.isDebugEnabled()) {
s_logger.debug("Purge queue item for cancelled
job-" + job.getId());
}
_queueMgr.purgeAsyncJobQueueItemId(job.getId());
-if
(job.getInstanceType().equals(ApiCommandJobType.Volume.toString())) {
+if (job.getInstanceType() != null &&
job.getInstanceType().equals(ApiCommandJobType.Volume.toString())) {
Review comment:
To avoid using this `job.getInstanceType() != null`, you can do the opposite
`ApiCommandJobType.Volume.toString().equals(job.getInstanceType())`
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime
rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202319051 ## File path: engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql ## @@ -32,4 +32,6 @@ ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255); -- We are only adding the permission to the default rules. Any custom rule must be configured by the root admin. INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 2, 'moveNetworkAclItem', 'ALLOW', 100) ON DUPLICATE KEY UPDATE rule=rule; INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 3, 'moveNetworkAclItem', 'ALLOW', 302) ON DUPLICATE KEY UPDATE rule=rule; -INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule; \ No newline at end of file +INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule; + +UPDATE `cloud`.`async_job` SET `removed` = now() WHERE `removed` IS NULL; Review comment: So, the column already existed but we were not using it? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rafaelweingartner commented on a change in pull request #2739: Async jobs add endtime
rafaelweingartner commented on a change in pull request #2739: Async jobs add
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202319718
##
File path:
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
##
@@ -1049,8 +1054,8 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
}
}
}
-List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
-for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+final List snapshotList =
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid),
false);
Review comment:
Why are using this final? There are other places you are adding them, what
is the real benefit? Besides adding more words for us to read.
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2709: check volumes for state when retrieving pool for configDrive creation
rhtyd commented on issue #2709: check volumes for state when retrieving pool for configDrive creation URL: https://github.com/apache/cloudstack/pull/2709#issuecomment-404805584 @borisstoyanov can you advise if it's passing manual config drive testing, wrt primary+secondary storage for hosting config drives? /cc @DaanHoogland - can you address any outstanding issues? This is an automated message from the Apache Git Service. To respond to the message, please log on 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: travis: Enhance Travis to do packaging jobs on different stage (#2640)
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
The following commit(s) were added to refs/heads/master by this push:
new 07042a6 travis: Enhance Travis to do packaging jobs on different
stage (#2640)
07042a6 is described below
commit 07042a67c69174c2ae1f49b3c1cdad2fa810fdc7
Author: Khosrow Moossavi
AuthorDate: Fri Jul 13 07:23:29 2018 -0400
travis: Enhance Travis to do packaging jobs on different stage (#2640)
Two stages are defined in travis job: test and package,
where test runs before package. On package stage we're
going to do the packaging of final artifacts based on
centos7, centos63, ubuntu1804, ubuntu1604 and ubuntu1404.
This is to validate that no PR will break packaging artifacts.
---
.travis.yml | 51 +--
1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index f4b52ba..2d32324 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,17 +17,21 @@
sudo: required
dist: trusty
group: edge
+
language: java
jdk:
-- oraclejdk8
+ - oraclejdk8
python:
- "2.7"
+
cache:
directories:
- - $HOME/.m2
+- $HOME/.m2
timeout: 500
+
notifications:
email: false
+
env:
global:
- PATH=$HOME/.local/bin:$PATH
@@ -171,3 +175,46 @@ script:
after_success: ./tools/travis/after_success.sh
after_failure: ./tools/travis/after_failure.sh
after_script: ./tools/travis/after_script.sh
+
+# Packaging job definition, will be reused
+.package_job: _job
+ before_install: docker pull ${IMAGE}
+ install: true
+ before_script: true
+ script: |
+docker run \
+ --volume ${TRAVIS_BUILD_DIR}:/mnt/build/cloudstack \
+ --volume $HOME/.m2:/root/.m2 \
+ --rm \
+ ${IMAGE} ${PARAMS}
+ after_script: true
+ after_success: true
+ after_failure: true
+
+jobs:
+ include:
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-rpm-builder:centos7 PARAMS="--distribution
centos7 --pack oss"
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-rpm-builder:centos6 PARAMS="--distribution
centos63 --pack oss"
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1804 PARAMS=""
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1604 PARAMS=""
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1404 PARAMS=""
+ <<: *package_job
+
[GitHub] rhtyd closed pull request #2640: Enhance Travis to do packaging job
rhtyd closed pull request #2640: Enhance Travis to do packaging job
URL: https://github.com/apache/cloudstack/pull/2640
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/.travis.yml b/.travis.yml
index f4b52bae945..2d323244f86 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,17 +17,21 @@
sudo: required
dist: trusty
group: edge
+
language: java
jdk:
-- oraclejdk8
+ - oraclejdk8
python:
- "2.7"
+
cache:
directories:
- - $HOME/.m2
+- $HOME/.m2
timeout: 500
+
notifications:
email: false
+
env:
global:
- PATH=$HOME/.local/bin:$PATH
@@ -171,3 +175,46 @@ script:
after_success: ./tools/travis/after_success.sh
after_failure: ./tools/travis/after_failure.sh
after_script: ./tools/travis/after_script.sh
+
+# Packaging job definition, will be reused
+.package_job: _job
+ before_install: docker pull ${IMAGE}
+ install: true
+ before_script: true
+ script: |
+docker run \
+ --volume ${TRAVIS_BUILD_DIR}:/mnt/build/cloudstack \
+ --volume $HOME/.m2:/root/.m2 \
+ --rm \
+ ${IMAGE} ${PARAMS}
+ after_script: true
+ after_success: true
+ after_failure: true
+
+jobs:
+ include:
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-rpm-builder:centos7 PARAMS="--distribution
centos7 --pack oss"
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-rpm-builder:centos6 PARAMS="--distribution
centos63 --pack oss"
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1804 PARAMS=""
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1604 PARAMS=""
+ <<: *package_job
+
+- stage: package
+ services: docker
+ env: IMAGE=khos2ow/cloudstack-deb-builder:ubuntu1404 PARAMS=""
+ <<: *package_job
+
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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] rhtyd commented on issue #2613: Cleanup POMs
rhtyd commented on issue #2613: Cleanup POMs URL: https://github.com/apache/cloudstack/pull/2613#issuecomment-404804666 @khos2ow can you rebase against latest master and fix conflicts? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rhtyd closed pull request #2649: Catch error in Debian packaging script and fail the build
rhtyd closed pull request #2649: Catch error in Debian packaging script and
fail the build
URL: https://github.com/apache/cloudstack/pull/2649
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/packaging/build-deb.sh b/packaging/build-deb.sh
index 52a168a95ba..e8c178350c7 100755
--- a/packaging/build-deb.sh
+++ b/packaging/build-deb.sh
@@ -16,7 +16,7 @@
# specific language governing permissions and limitations
# under the License.
-#set -e
+set -e
#
# This script builds Debian packages for CloudStack and does
@@ -100,8 +100,7 @@ while [ -n "$1" ]; do
esac
done
-DCH=$(which dch)
-if [ -z "$DCH" ] ; then
+if [ -z "$(which dch)" ] ; then
echo -e "dch not found, please install devscripts at first. \nDEB Build
Failed"
exit 1
fi
@@ -110,6 +109,17 @@ NOW="$(date +%s)"
PWD=$(cd $(dirname "$0") && pwd -P)
cd $PWD/../
+# Fail early if working directory is NOT clean and --use-timestamp was provided
+if [ "$USE_TIMESTAMP" == "true" ]; then
+if [ -n "$(cd $PWD; git status -s)" ]; then
+echo "Erro: You have uncommitted changes and asked for --use-timestamp
to be used."
+echo " --use-timestamp flag is going to temporarily change POM
versions and"
+echo " revert them at the end of build, and there's no way we
can do partial"
+echo " revert. Please commit your changes first or omit
--use-timestamp flag."
+exit 1
+fi
+fi
+
VERSION=$(head -n1 debian/changelog |awk -F [\(\)] '{print $2}')
DISTCODE=$(lsb_release -sc)
@@ -148,4 +158,6 @@ dpkg-buildpackage -uc -us -b
/bin/mv /tmp/changelog.orig debian/changelog
-(cd $PWD; git reset --hard)
+if [ "$USE_TIMESTAMP" == "true" ]; then
+(cd $PWD; git reset --hard)
+fi
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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: packaging: Catch error in packaging script and fail the build (#2649)
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
The following commit(s) were added to refs/heads/master by this push:
new 4a5fab0 packaging: Catch error in packaging script and fail the build
(#2649)
4a5fab0 is described below
commit 4a5fab0573c6d99a79ef3c6cbcf30918c2465ff8
Author: Khosrow Moossavi
AuthorDate: Fri Jul 13 07:17:34 2018 -0400
packaging: Catch error in packaging script and fail the build (#2649)
This is to fix the regression added in #2433. In this fix
we're going to fail the build early if --use-timestamp is
provided and working directory is *not* clean. And also
fail the build if something in the script has returned
non-zero value.
---
packaging/build-deb.sh | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/packaging/build-deb.sh b/packaging/build-deb.sh
index 52a168a..e8c1783 100755
--- a/packaging/build-deb.sh
+++ b/packaging/build-deb.sh
@@ -16,7 +16,7 @@
# specific language governing permissions and limitations
# under the License.
-#set -e
+set -e
#
# This script builds Debian packages for CloudStack and does
@@ -100,8 +100,7 @@ while [ -n "$1" ]; do
esac
done
-DCH=$(which dch)
-if [ -z "$DCH" ] ; then
+if [ -z "$(which dch)" ] ; then
echo -e "dch not found, please install devscripts at first. \nDEB Build
Failed"
exit 1
fi
@@ -110,6 +109,17 @@ NOW="$(date +%s)"
PWD=$(cd $(dirname "$0") && pwd -P)
cd $PWD/../
+# Fail early if working directory is NOT clean and --use-timestamp was provided
+if [ "$USE_TIMESTAMP" == "true" ]; then
+if [ -n "$(cd $PWD; git status -s)" ]; then
+echo "Erro: You have uncommitted changes and asked for --use-timestamp
to be used."
+echo " --use-timestamp flag is going to temporarily change POM
versions and"
+echo " revert them at the end of build, and there's no way we
can do partial"
+echo " revert. Please commit your changes first or omit
--use-timestamp flag."
+exit 1
+fi
+fi
+
VERSION=$(head -n1 debian/changelog |awk -F [\(\)] '{print $2}')
DISTCODE=$(lsb_release -sc)
@@ -148,4 +158,6 @@ dpkg-buildpackage -uc -us -b
/bin/mv /tmp/changelog.orig debian/changelog
-(cd $PWD; git reset --hard)
+if [ "$USE_TIMESTAMP" == "true" ]; then
+(cd $PWD; git reset --hard)
+fi
[GitHub] rhtyd commented on issue #2734: Fix invalid consoleproxy url after upgrade
rhtyd commented on issue #2734: Fix invalid consoleproxy url after upgrade URL: https://github.com/apache/cloudstack/pull/2734#issuecomment-404803960 Regression test LGTM, I did not test it manually though. @borisstoyanov if you've time next week, can you test and advise? Thanks. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rhtyd commented on a change in pull request #2739: Async jobs add endtime
rhtyd commented on a change in pull request #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202318311 ## File path: test/integration/smoke/test_async_job.py ## @@ -0,0 +1,139 @@ +""" +Integration Test +""" +# 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. + +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import cleanup_resources +from marvin.lib.base import ServiceOffering, DiskOffering, Account, VirtualMachine,\ +queryAsyncJobResult, PASS +from marvin.lib.common import get_domain, get_zone, get_test_template +from pytz import timezone + + +class TestAsyncJob(cloudstackTestCase): Review comment: Can you add this to .travis.yml keeping alphabetical sorting order? This is an automated message from the Apache Git Service. To respond to the message, please log on 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] rhtyd commented on a change in pull request #2739: Async jobs add endtime
rhtyd commented on a change in pull request #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202318228 ## File path: test/integration/smoke/test_async_job.py ## @@ -0,0 +1,139 @@ +""" Review comment: Fix comment, remove it. This is an automated message from the Apache Git Service. To respond to the message, please log on 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] ernjvr commented on issue #2739: Async jobs add endtime
ernjvr commented on issue #2739: Async jobs add endtime URL: https://github.com/apache/cloudstack/pull/2739#issuecomment-404753768 @rhtyd @DaanHoogland please review. This is an automated message from the Apache Git Service. To respond to the message, please log on 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
