GitHub user rhtyd opened a pull request:
https://github.com/apache/cloudstack/pull/1980
CLOUDSTACK-9805: Display VR list in network details
 can see this tab
too. I'll fix this asap.
---
If your
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1980
@PaulAngus @ustcweizhou fixed now, the tab is displayed only to admins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1923
LGTM. @karuturi not a blocker, but then we'll need to document the
workaround (manual fix) in the release notes. If RC1 has any blockers, let's
include this in RC2.
---
If your project is set
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1212
LGTM @karuturi this is ready for merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1907
LGTM @karuturi this is ready for merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1915
LGTM @karuturi this is ready for merge
Ping @abhinandanprateek in case you want to review?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1954
Thanks @dmabry
LGTM @karuturi this is ready for merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1954
@dmabry can we have this in 4.9 as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@nvazquez can you check if the failures in above test results are related
to your changes, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear
GitHub user rhtyd reopened a pull request:
https://github.com/apache/cloudstack/pull/1944
CLOUDSTACK-9783: Improve metrics view performance
This improves the metrics view feature by improving the rendering
performance
of metrics view tables, by re-implementing the logic
Github user rhtyd closed the pull request at:
https://github.com/apache/cloudstack/pull/1944
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1836
@blueorangutan test matrix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1843
LGTM.
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1784
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1784
Test looks good to me, @abhinandanprateek should we target/include this in
4.9 as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1957
LGTM (code review).
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1959
LGTM, code review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1950
LGTM. Tested manually on local Ubuntu installation. Fixed the permission
issue by fixing sudoers manually and JAVA_HOME issue by fixing the variable in
/etc/default/cloudstack
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1257
@sureshanaparti can check if failure in the above test results are due to
your changes, thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1836
/cc @karuturi -- I'm running health check for latest master/4.10 RC1 cut
from current HEAD
@blueorangutan package
---
If your project is set up for it, you can reply to this email and have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1958
/cc @karuturi ready for merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
@vincentbernat I made some changes, built packages and was able to install
and run agent and usage server on both Ubuntu 12.04, 14.04 using the same
packages.
@wido @vincentbernat can you
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1658
Great idea, LGTM (based on the api-changes urls). @swill can we also sort
the list of new APIs by name.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
@wido actually on a fresh system it would first attempt to find and install
openjdk8, on existing ones if openjdk7 is installed then it won't force
openjdk8 by default. Thanks.
---
If your
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76202771
--- Diff: debian/rules ---
@@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/etc"
DESTDIR
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76197541
--- Diff: debian/rules ---
@@ -5,18 +5,9 @@ PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f
1)
SYSCONFDIR = "/etc"
DESTDIR
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
RMs please help review /cc @jburwell @karuturi
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
Thanks @vincentbernat I'll update the dependencies, and in the
post-installation step we can modify the default JAVA_HOME path of the host in
the default/{cloudstack-usage, cloudstack-agent
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
@jburwell I've added an empty upgrade path. Tested the db upgrade locally.
LGTM.
When you fwd-merge on 4.9 use: `git merge -X ours 4.8` to avoid fixing
merge conflicts in pom.xm files by hand
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1662
@digitalgust please squash the changes, the changes look like bugfixes so
please edit the PR and change the base branch to 4.9 branch, and please open a
ticket on JIRA and edit the commit message
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
@wido @vincentbernat I've incorporate all the changes now, including
refactoring of systemd file and now we've both centos and ubuntu/debian
packaging consuming from the the same service/default
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76227609
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
@wido @vincentbernat I could not get rid of the postinst step as shell is
not executed when I move the code in the EnvironmentFile itself. I've reduced
its impact to only run when the default
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76226387
--- Diff: packaging/centos7/cloud.spec ---
@@ -482,7 +488,7 @@ pip install --upgrade
/usr/share/cloudstack-marvin/Marvin-*.tar.gz
%dir %attr(0770
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1647#discussion_r76227828
--- Diff: debian/cloudstack-agent.postinst ---
@@ -41,6 +41,12 @@ case "$1" in
mkdir /etc/libvirt/hooks
fi
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
Thanks @wido I'll see if I can fix the issue without including the changes
in postinst files.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
Packaging tests confirmed, PR is OK. Debian packages tested on Ubuntu
12.04/14.04/16.04, rpms tests on CentOS 7.2. Both usage, agent came up on
installation.
---
If your project is set up
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1647
Update: postinstall code has been removed now @wido @vincentbernat
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Lgtm.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
LGTM thanks @nvazquez for the change. Can you help with testing,
unfortunately I don't have the resources (some infra issues) to run tests
across vmware and xenserver.
---
If your project
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
Thanks @milamberspace
Based on the tests and LGTMs, I'll proceed with merging this on 4.9. On
fwd-merging, I've fixed conflicts and renamed a subsequent class to match the
change. Fwd-merged
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1578
The PR only contains test LGTM for PR/feature specific changes. Since the
PR brings with it several changes, a regression test LGTM is necessary i.e.
smoke test results against kvm/xen/vmware
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1670#discussion_r77133791
--- Diff: setup/db/db/schema-490to491.sql ---
@@ -23,3 +23,11 @@
UPDATE `cloud`.`roles` SET `description`='Default user role' WHERE `id`=4
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1670
LGTM, tested with a local installation, running it for past 3 days.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1664#discussion_r77139978
--- Diff:
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeFromVmSnapshotCmd.java
---
@@ -0,0 +1,105 @@
+// Licensed to the Apache
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Tests on 4.8 branch, all pass:
- Clean db setup works, last upgraded version to 4.8.2.0.
| 17 | 4.8.1 | 2016-09-01 09:56:21 | Complete |
| 18 | 4.8.2.0 | 2016
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Given Travis has passed with 2 LGTMs and test LGTM, I'll go ahead and merge
this after testing this across 4.8->4.9, 4.9->master.
---
If your project is set up for it, you can
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Tests on 4.9 branch (after fwd-merge), all pass:
- Upgrade from 4.8.2.0 to 4.9.1.0
- Clean fresh db deployment
---
If your project is set up for it, you can reply
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1664#discussion_r77139536
--- Diff:
api/src/org/apache/cloudstack/api/command/admin/template/SeedTemplateFromVmSnapshotCmd.java
---
@@ -0,0 +1,145 @@
+/*
+Â * Licensed
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1664#discussion_r77140682
--- Diff:
engine/schema/src/com/cloud/storage/dao/VMSnapshotTemplatePoolDaoImpl.java ---
@@ -0,0 +1,160 @@
+/*
+Â * Licensed to the Apache
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1664#discussion_r77140264
--- Diff:
engine/schema/src/com/cloud/storage/VMSnapshotTemplateStoragePoolVO.java ---
@@ -0,0 +1,228 @@
+/*
+Â * Licensed to the Apache Software
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1664
@sateesh-chodapuneedi please add unit tests to increase code coverage,
break large methods into smaller reviewable methods. Thanks.
---
If your project is set up for it, you can reply
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1624
LGTM. Thanks @ProjectMoon as soon as #1654 is merged, we'll get this merged
as well. Do we have any regression tests results, especially around the changed
code?
---
If your project is set up
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
@jburwell we'll need that upgrade path class, we have some alter table
changes we want for the 4.9.0->4.9.1.0 upgrade path, see the diff for changes
you've removed.
---
If your project is
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1636
LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1624
@ProjectMoon please rebase now, PR 1654 has been merged now. DB upgrades
should not cause errors.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
@jburwell this needs to be reworked now that #1654 is merged. Good to have
the changes but not a blocker for 4.8, 4.9 RCs.
---
If your project is set up for it, you can reply to this email
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Based on the tests and review. I'll proceed with merging this and fwd-merge
this across 4.9, master branches.
---
If your project is set up for it, you can reply to this email and have your
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
Tests on master branch (after fwd-merge), all pass:
- Upgrade from 4.8.2.0, upgrade from 4.9.1.0
- Clean db deployment
---
If your project is set up for it, you can reply
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1670
Thanks @borisstoyanov
Based on the reviews, db upgrade and local env tests, I'll merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
Tests performed:
- Upgraded from 4.9.0, upgrade path ran (whose sql file names were
changed)
- Clean db deployment
---
If your project is set up for it, you can reply
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1624#discussion_r77326112
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -761,6 +774,17 @@ protected boolean cleanupAccount(AccountVO account,
long callerUserId
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
Tests performed after fwd-merge, all pass:
- Upgraded from 4.9.1.0
- Clean db deployment
---
If your project is set up for it, you can reply to this email and have your
reply
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1624#discussion_r77325991
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -677,6 +679,17 @@ public boolean deleteAccount(AccountVO account, long
callerUserId
GitHub user rhtyd opened a pull request:
https://github.com/apache/cloudstack/pull/1669
Make CloudStack JSP-free
CLOUDSTACK-9842: Make UI JSP Free
We use some JSP file just for translation of strings in the UI. This is
achievable purely in JavaScript
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1663
Thanks @borisstoyanov
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1667
LGTM. @abhinandanprateek can you edit the base branch of this PR to 4.9.
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1663
Thanks all, based on the test results and reviews from this PR with the
marvin test and the original PR #1653 from where the main changes were taken,
I'll go ahead and merge this now
Github user rhtyd closed the pull request at:
https://github.com/apache/cloudstack/pull/1669
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
GitHub user rhtyd reopened a pull request:
https://github.com/apache/cloudstack/pull/1669
Make CloudStack JSP-free
CLOUDSTACK-9842: Make UI JSP Free
We use some JSP file just for translation of strings in the UI. This is
achievable purely in JavaScript
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1669
During build step, the l10n JS files are built:
> [INFO] --- exec-maven-plugin:1.2.1:exec (generate-sources) @
cloud-client-ui ---
> Exporting compiled dictionary:
/home/bhaisa
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1667
@abhinandanprateek thanks, can you close this one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Minor improvements suggested, LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1660#discussion_r76731318
--- Diff: utils/src/main/java/com/cloud/utils/ssh/SshHelper.java ---
@@ -196,6 +198,10 @@ public static void scpTo(String host, int port, String
user
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1593#discussion_r76730973
--- Diff: usage/conf/log4j-cloud_usage.xml.in ---
@@ -48,7 +48,7 @@ under the License
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1593#discussion_r76730939
--- Diff: debian/cloudstack-usage.postinst ---
@@ -43,6 +43,10 @@ case "$1" in
rm -rf /etc/cloudstack/usage/key
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1593#discussion_r76731031
--- Diff: usage/src/com/cloud/usage/UsageSanityChecker.java ---
@@ -43,9 +43,9 @@
protected static final int DEFAULT_AGGREGATION_RANGE = 1440
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1560
LGTM (code/logic seems alright, did not run any tests).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1621
Thanks @milamberspace for the PR, I'll merge it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1669
Thanks @milamberspace I've modified the build process to generate l10n
translation JS (dictionary) files when client/UI is built. I think if we use
json file, then also we will need some sort
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1665
With new db schema/versioning and upgrade changes in #1654 we can merge
this with some additional changes once #1654 is merged and fwd-merged.
---
If your project is set up for it, you can reply
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1667
@abhinandanprateek can you reopen this for 4.9 branch, since it's a bugfix.
LTS users should all be on innodb to avoid any replication issues, please also
move the changes suitably to 4.9.0
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1654
@jburwell I've fixed the issue failing db upgrade, it was a minor
comparison issue in the beginning of the upgrade process. On my local env the
upgrade worked as expected. LGTM.
---
If your
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1666
LGTM. @jburwell we should have this in 4.9.1.0-RC1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1696
I've manually kicked a trillian env (tid-84) to get an env against
xenserver 6.5sp1, as soon as it comes up I'll perform the test to launch a
Ubuntu 16.04 VM using templates from
http
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1619
LGTM. Since basic checks are all green and we've enough votes, I'll merge
this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1690
LGTM. Since it's a translation change, we've enough votes to merge this.
I'll proceed with merging this on master.
---
If your project is set up for it, you can reply to this email and have your
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1692
I'm investigating failures on this PR, likely most of them (esp related to
oobm) are env related as udp traffic seems to be blocked. There also seems to
be an error with the log upload script
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1696
With a manually deployed Trillian environment based on XenServer 6.5sp1, I
was able to register Ubuntu 16.04 and launch a VM. Screenshots attached:
