[GitHub] mrunalinikankariya commented on issue #2244: CLOUDSTACK-10054:Volume download times out in 3600 seconds
mrunalinikankariya commented on issue #2244: CLOUDSTACK-10054:Volume download times out in 3600 seconds URL: https://github.com/apache/cloudstack/pull/2244#issuecomment-362474082 @rhtyd All the outstanding issues has been addressed, can you please review and test. 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] bwsw commented on issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly
bwsw commented on issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly URL: https://github.com/apache/cloudstack/pull/2443#issuecomment-362467046 @rafaelweingartner Thank you commiting to this area of code. Proper accounting is very important, I commited in 4.11 regarding accounting too, but it seems there are a lot of possible bugs happen here. Nb. Please keep in mind to check that domain resources are accounted correctly too. Also, I strongly believe resources should be updated with increment/decrement merhods, but updateResourceCount is only should be used to do periodic update for top level domains when a/b/c/account is used and we would like to update for b and a... 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 #2433: Fix and enhance package script
rafaelweingartner commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362369782 Awesome, thanks for the explanation. 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 #2433: Fix and enhance package script
khos2ow commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362369459 @rafaelweingartner actually it will generate: `cloudstack---.<-r parameter || timestamp>..centos.x86_64.rpm` 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 #2433: Fix and enhance package script
blueorangutan commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362368203 Packaging result: ?centos6 ?centos7 ?debian. JID-1687 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 #2433: Fix and enhance package script
rafaelweingartner commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362367127 @khos2ow for the efforts here. I liked this since the beginning and now it is more flexible... great job +1. BTW: will it still generate packages following: `cloudstack---.<-r parameter>..x86_64.rpm` 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 a change in pull request #2433: Fix and enhance package script
khos2ow commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165448380
##
File path: packaging/package.sh
##
@@ -155,31 +181,51 @@ while [ $# -gt 0 ] ; do
usage
exit 1
fi
-shift
+shift 2
;;
+
-d | --distribution)
TARGETDISTRO=$2
if [ -z "$TARGETDISTRO" ] ; then
echo "Error: Missing target distribution"
usage
exit 1
fi
-shift
+shift 2
;;
+
-r | --release)
RELEASE=$2
-shift
+shift 2
;;
--)
-echo "Error: Unrecognized option"
-usage
-exit 1
+
+-b | --brand)
+BRANDING=$2
+shift 2
+;;
+
+-T | --use-timestamp)
+SNAPSHOT_TIMESTAMP="$(date +%s)"
+shift 1
;;
+
+-*)
+unrecognized_flags="${unrecognized_flags}$1 "
+shift 1
+;;
+
*)
-shift
+shift 1
;;
esac
done
-packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE"
+if [ -n "$unrecognized_flags" ]; then
+echo "Warning: Unrecognized option(s) found \" ${unrecognized_flags}\""
+echo " You're advised to fix your build job scripts and prevent
using these"
+echo " flags, as in the future release(s) they will break
packaging script."
+echo ""
+fi
+echo "Packaging CloudStack..."
+packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE" "$BRANDING"
Review comment:
not really. `SNAPSHOT_TIMESTAMP` is globally accessible. On the other words
we didn't need to pass all the rest of input params to `packaging` function too.
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 a change in pull request #2433: Fix and enhance package script
khos2ow commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165451333
##
File path: packaging/package.sh
##
@@ -67,22 +79,38 @@ function packaging() {
exit 2
fi
fi
+
VERSION=$(cd ../; $MVN
org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate
-Dexpression=project.version | grep --color=none '^[0-9]\.')
+
+if [ -n "$5" ]; then
+DEFBRN="-D_brand -$5"
+BRAND="${5}."
+else
+BASEVER=$(echo "$VERSION" | sed 's/-SNAPSHOT//g')
+REALVER=$(echo "$BASEVER" | cut -d '-' -f 1)
+BRAND=$(echo "$BASEVER" | cut -d '-' -f 2)
+
+if [ "$REALVER" != "$BRAND" ]; then
+DEFBRN="-D_brand -$BRAND"
+BRAND="${BRAND}."
+else
+BRAND=""
+fi
+fi
+
Review comment:
- If you specifically mention `--brand NAME` as input flag, it will use this
_NAME_ no matter what, otherwise it will try to extract any "branding" name
from POM version (hence the else block).
- If pass `$5` directly to `-D_rel` it will wipe the value of `-r, --release
integer` that's why it will be appended to `-D_rel` and not replace it.
- We still do need `-D_brand` for the cases in which POM version does have
branding string in them. The folder in `/dist/RPM/...` will have a folder name
as the full POM version but `_maventag` in spec wouldn't have it, so the
packaging will fail.
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 a change in pull request #2433: Fix and enhance package script
khos2ow commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165448380
##
File path: packaging/package.sh
##
@@ -155,31 +181,51 @@ while [ $# -gt 0 ] ; do
usage
exit 1
fi
-shift
+shift 2
;;
+
-d | --distribution)
TARGETDISTRO=$2
if [ -z "$TARGETDISTRO" ] ; then
echo "Error: Missing target distribution"
usage
exit 1
fi
-shift
+shift 2
;;
+
-r | --release)
RELEASE=$2
-shift
+shift 2
;;
--)
-echo "Error: Unrecognized option"
-usage
-exit 1
+
+-b | --brand)
+BRANDING=$2
+shift 2
+;;
+
+-T | --use-timestamp)
+SNAPSHOT_TIMESTAMP="$(date +%s)"
+shift 1
;;
+
+-*)
+unrecognized_flags="${unrecognized_flags}$1 "
+shift 1
+;;
+
*)
-shift
+shift 1
;;
esac
done
-packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE"
+if [ -n "$unrecognized_flags" ]; then
+echo "Warning: Unrecognized option(s) found \" ${unrecognized_flags}\""
+echo " You're advised to fix your build job scripts and prevent
using these"
+echo " flags, as in the future release(s) they will break
packaging script."
+echo ""
+fi
+echo "Packaging CloudStack..."
+packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE" "$BRANDING"
Review comment:
not really. `SNAPSHOT_TIMESTAMP` is globally accessible. On the other words
we didn't need to pass all the rest input params to `packaging` function too.
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 #2433: Fix and enhance package script
blueorangutan commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362360805 @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 #2433: Fix and enhance package script
rhtyd commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362360543 Thanks @khos2ow appreciate the effort, I think there are few minor issues left otherwise okay. Let me kick a test packaging job and see how the packages turn out. @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] rhtyd commented on a change in pull request #2433: Fix and enhance package script
rhtyd commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165447722
##
File path: packaging/package.sh
##
@@ -67,22 +79,38 @@ function packaging() {
exit 2
fi
fi
+
VERSION=$(cd ../; $MVN
org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate
-Dexpression=project.version | grep --color=none '^[0-9]\.')
+
+if [ -n "$5" ]; then
+DEFBRN="-D_brand -$5"
+BRAND="${5}."
+else
+BASEVER=$(echo "$VERSION" | sed 's/-SNAPSHOT//g')
+REALVER=$(echo "$BASEVER" | cut -d '-' -f 1)
+BRAND=$(echo "$BASEVER" | cut -d '-' -f 2)
+
+if [ "$REALVER" != "$BRAND" ]; then
+DEFBRN="-D_brand -$BRAND"
+BRAND="${BRAND}."
+else
+BRAND=""
+fi
+fi
+
Review comment:
If $5 or branding is passed, then the above code is not needed. `$5` may be
used as argument for `-D_rel`, why introduce a new `-D_brand`, that way changes
to the spec files may be removed as well.
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 #2433: Fix and enhance package script
rhtyd commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165447356
##
File path: packaging/package.sh
##
@@ -155,31 +181,51 @@ while [ $# -gt 0 ] ; do
usage
exit 1
fi
-shift
+shift 2
;;
+
-d | --distribution)
TARGETDISTRO=$2
if [ -z "$TARGETDISTRO" ] ; then
echo "Error: Missing target distribution"
usage
exit 1
fi
-shift
+shift 2
;;
+
-r | --release)
RELEASE=$2
-shift
+shift 2
;;
--)
-echo "Error: Unrecognized option"
-usage
-exit 1
+
+-b | --brand)
+BRANDING=$2
+shift 2
+;;
+
+-T | --use-timestamp)
+SNAPSHOT_TIMESTAMP="$(date +%s)"
+shift 1
;;
+
+-*)
+unrecognized_flags="${unrecognized_flags}$1 "
+shift 1
+;;
+
*)
-shift
+shift 1
;;
esac
done
-packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE"
+if [ -n "$unrecognized_flags" ]; then
+echo "Warning: Unrecognized option(s) found \" ${unrecognized_flags}\""
+echo " You're advised to fix your build job scripts and prevent
using these"
+echo " flags, as in the future release(s) they will break
packaging script."
+echo ""
+fi
+echo "Packaging CloudStack..."
+packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE" "$BRANDING"
Review comment:
Should you also pass `$SNAPSHOT_TIMESTAMP` as `$6` to `packaging`? Or, use
global setting in which case why pass `$BRANDING`?
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 #2433: Fix and enhance package script
rhtyd commented on a change in pull request #2433: Fix and enhance package
script
URL: https://github.com/apache/cloudstack/pull/2433#discussion_r165447356
##
File path: packaging/package.sh
##
@@ -155,31 +181,51 @@ while [ $# -gt 0 ] ; do
usage
exit 1
fi
-shift
+shift 2
;;
+
-d | --distribution)
TARGETDISTRO=$2
if [ -z "$TARGETDISTRO" ] ; then
echo "Error: Missing target distribution"
usage
exit 1
fi
-shift
+shift 2
;;
+
-r | --release)
RELEASE=$2
-shift
+shift 2
;;
--)
-echo "Error: Unrecognized option"
-usage
-exit 1
+
+-b | --brand)
+BRANDING=$2
+shift 2
+;;
+
+-T | --use-timestamp)
+SNAPSHOT_TIMESTAMP="$(date +%s)"
+shift 1
;;
+
+-*)
+unrecognized_flags="${unrecognized_flags}$1 "
+shift 1
+;;
+
*)
-shift
+shift 1
;;
esac
done
-packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE"
+if [ -n "$unrecognized_flags" ]; then
+echo "Warning: Unrecognized option(s) found \" ${unrecognized_flags}\""
+echo " You're advised to fix your build job scripts and prevent
using these"
+echo " flags, as in the future release(s) they will break
packaging script."
+echo ""
+fi
+echo "Packaging CloudStack..."
+packaging "$PACKAGEVAL" "$SIM" "$TARGETDISTRO" "$RELEASE" "$BRANDING"
Review comment:
Should you also pass `$SNAPSHOT_TIMESTAMP` as `$6` to `packaging`?
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 #2433: Fix and enhance package script
khos2ow commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362356870 @rhtyd @rafaelweingartner @DaanHoogland I changed the PR to be more backward-compatible as follow: - new `-T, --use-timestamp` flag to use `timestamp` instead of SNAPSHOT in final package name - silently ignore unknown flags (e.g. `-o`) and show a warning that in future building will be broken - new `-b, --brand string` flag which will override "branding" string found in POM version, or to add branding if POM doesn't have it at all 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 #2433: Fix and enhance package script
khos2ow commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362356870 @rhtyd @rafaelweingartner @DaanHoogland I changed the PR to be more backward-compatible as follow: - new `-T, --use-timestamp` flag to use `timestamp` instead of SNAPSHOT - silently ignore unknown flags (e.g. `-o`) and show a warning that in future building will be broken - new `-b, --brand string` flag. which will override "branding" string found in POM version, or to add branding if POM doesn't have 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] khos2ow commented on issue #2433: Fix and enhance package script
khos2ow commented on issue #2433: Fix and enhance package script URL: https://github.com/apache/cloudstack/pull/2433#issuecomment-362356870 @rhtyd @rafaelweingartner @DaanHoogland I changed the PR to be more backward-compatible as follow: - new `-T, --use-timestamp` flag to use `timestamp` instead of SNAPSHOT - silently ignore unknown flags (e.g. `-o`) and show a warning that in future building will be broken - new `-b, --brand string` flag. which will override "branding" string found in POM version, or to add branding if POM doesn't have 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 issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly
rafaelweingartner commented on issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly URL: https://github.com/apache/cloudstack/pull/2443#issuecomment-362290925 I can write one if you think it is worth it. I have never created one 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 issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly
DaanHoogland commented on issue #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly URL: https://github.com/apache/cloudstack/pull/2443#issuecomment-362289819 @rafaelweingartner your code looks good at first sight. Are you writing automation tests for this? I think the PR merrits 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 opened a new pull request #2443: [CLOUDSTACK-9338] ACS not accounting resources of VMs with custom service offering properly
rafaelweingartner opened a new pull request #2443: [CLOUDSTACK-9338] ACS not accounting resources of VMs with custom service offering properly URL: https://github.com/apache/cloudstack/pull/2443 ## Description ACS accounts the resources when deploying VMs with custom service offerings. However, there are other methods (such as updateResourceCount) that do not execute the resource accounting properly, and these methods update the resource count for an account in the database. Therefore, if a user deploys VMs with custom service offerings, and later this user calls the ?updateResourceCount? method, it (the method) will only account for VMs with normal service offerings, and update this as the number of resources used by the account. This will result in a smaller number of resources to be accounted for the given account than the real used value. The problem becomes worse because if the user starts to delete these VMs, it is possible to reach negative values of resources allocated (breaking all of the resource limiting for accounts). This is a very serious attack vector for public cloud providers! ## Steps to reproduce: 1. Create a new account and dedicate some resources to it 2. Instantiate a VM using a custom service offering 3. Check the CPU and memory resource count for the given VM (the count must be correct at this point): select * from resource_count where account_id = ? 4. Go to the details of the account and click on ?Update resource count? 5. Execute the SQL again (select * from resource_count where account_id = ?), and check if the resource count is correct. The value of resource count will consider only VMs deployed with normal service offering; 6. Delete all VMs 7. Check the resource count table again. It will reach negative values. 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 issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's
DaanHoogland commented on issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362242075 So let's take it up with the ticket submitter, @houthuis 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 #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's
rafaelweingartner commented on issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362241268 I never used tags this way... but then you need to have tags on all of your service offerings, right? And then you need to remove the tags of hosts the you want to isolate. Or maybe custom service offerings to system administrators with tags that are not used by service offerings of end users... BTW: for me, this is indifferent, I am not an admin, I only wanted to lift this question to see your feedback. 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 issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's
DaanHoogland commented on issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362240521 but that's what tags are for aren't they? 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 #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's
rafaelweingartner commented on issue #2442: CLOUDSTACK-10147 Disabled Xenserver Cluster can still deploy VM's URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362240019 The first instinct is to say yes... However, can't this help administrators while debugging problems? I mean, otherwise they need to enable a host/cluster to test something, which will enable these resources for end users. 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 issue #2442: CLOUDSTACK-10147
DaanHoogland commented on issue #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362239425 @rafaelweingartner isn't the point of disabling that no new deployments happen on that host? 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 #2442: CLOUDSTACK-10147
rafaelweingartner commented on issue #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362237523 I think I did not express my self properly... I meant, do we want to block root admins to deploy VMs on disabled clusters/hosts? It feels that they should be able to do so (for testing purposes for instance). 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] houthuis commented on issue #2442: CLOUDSTACK-10147
houthuis commented on issue #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362237163 @rafaelweingartner no not on disabled hosts, but i was able to reproduce on disabled clusters 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 #2442: CLOUDSTACK-10147
rafaelweingartner commented on issue #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362236381 @houthuis thanks for the contributions. However, I have a doubt here... Not even root administrators can deploy VMs on disabled clusters/hosts? BTW: can you update the title of the PR with your issue description as well? 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 #2406: CLOUDSTACK-9663 return role on updateRole
rhtyd commented on issue #2406: CLOUDSTACK-9663 return role on updateRole URL: https://github.com/apache/cloudstack/pull/2406#issuecomment-362236396 LGTM. @DaanHoogland yes we can accept this PR in 4.11 towards the .1 milestone, only after 4.11.0.0 releases. 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 #2442: CLOUDSTACK-10147
rafaelweingartner commented on issue #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442#issuecomment-362236381 @houthuis thanks for the contributions. However, I have a doubt here... Not even root administrators can deploy VMs on disabled clusters/hosts? 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] houthuis opened a new pull request #2442: CLOUDSTACK-10147
houthuis opened a new pull request #2442: CLOUDSTACK-10147 URL: https://github.com/apache/cloudstack/pull/2442 ENVIRONMENT = XenServer Version : 6.2 , 7 ISSUE == Disabled Xenserver Cluster can still deploy VM's , hosts in the cluster are still active Repro. steps followed == Disabled Cluster from UI. Deploy a new VM Expected Behavior === After disabling the cluster , the hosts should be disabled. and no VM's can be deployed Solution: Added a check to skip disabled clusters when selecting a host to deploy on. Deploying on a disabled cluster will now result in a InsufficientServerCapacityException, if no enabled clusters are found. 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] houthuis closed pull request #2382: CLOUDSTACK-4045 IP address acquired with associateIpAddress is marked as source NAT
houthuis closed pull request #2382: CLOUDSTACK-4045 IP address acquired with
associateIpAddress is marked as source NAT
URL: https://github.com/apache/cloudstack/pull/2382
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/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
index c00359c92f0..d0b3098c3d3 100644
--- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
@@ -1370,16 +1370,7 @@ public IPAddressVO associateIPToGuestNetwork(long ipId,
long networkId, boolean
}
}
-NetworkOffering offering =
_networkOfferingDao.findById(network.getNetworkOfferingId());
-boolean sharedSourceNat = offering.getSharedSourceNat();
-boolean isSourceNat = false;
-if (!sharedSourceNat) {
-if (getExistingSourceNatInNetwork(owner.getId(), networkId) ==
null) {
-if (network.getGuestType() == GuestType.Isolated &&
network.getVpcId() == null && !ipToAssoc.isPortable()) {
-isSourceNat = true;
-}
-}
-}
+boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc,
network);
s_logger.debug("Associating ip " + ipToAssoc + " to network " +
network);
@@ -1417,6 +1408,25 @@ public IPAddressVO associateIPToGuestNetwork(long ipId,
long networkId, boolean
}
}
+protected boolean isSourceNatAvailableForNetwork(Account owner,
IPAddressVO ipToAssoc, Network network) {
+NetworkOffering offering =
_networkOfferingDao.findById(network.getNetworkOfferingId());
+boolean sharedSourceNat = offering.getSharedSourceNat();
+boolean isSourceNat = false;
+if (!sharedSourceNat) {
+if (getExistingSourceNatInNetwork(owner.getId(), network.getId())
== null) {
+if (network.getGuestType() == GuestType.Isolated &&
network.getVpcId() == null && !ipToAssoc.isPortable()) {
+if (network.getState() == Network.State.Allocated) {
+//prevent associating an ip address to an allocated
(unimplemented network).
+//it will cause the ip to become source nat, and it
can't be disassociated later on.
+throw new InvalidParameterValueException("Network is
in allocated state, implement network first before acquiring an IP address");
+}
+isSourceNat = true;
+}
+}
+}
+return isSourceNat;
+}
+
protected boolean isSharedNetworkOfferingWithServices(long
networkOfferingId) {
NetworkOfferingVO networkOffering =
_networkOfferingDao.findById(networkOfferingId);
if ((networkOffering.getGuestType() == Network.GuestType.Shared)
diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
index 0bf92ee2f69..fe11292e826 100644
--- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
+++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
@@ -17,38 +17,74 @@
package com.cloud.network;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.IPAddressVO;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.network.rules.StaticNat;
+import com.cloud.network.rules.StaticNatImpl;
+import com.cloud.offerings.NetworkOfferingVO;
+import com.cloud.offerings.dao.NetworkOfferingDao;
+import com.cloud.user.AccountVO;
+import com.cloud.utils.net.Ip;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
-
-import com.cloud.network.dao.IPAddressDao;
-import com.cloud.network.dao.IPAddressVO;
-import com.cloud.network.rules.StaticNat;
-import com.cloud.network.rules.StaticNatImpl;
-import com.cloud.utils.net.Ip;
-
-import static org.mockito.Mockito.when;
+import org.mockito.Spy;
import java.util.Collections;
import java.util.List;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.anyLong;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
public class IpAddressManagerTest {
@Mock
IPAddressDao _ipAddrDao;
+@Mock
+NetworkDao
[GitHub] houthuis opened a new pull request #2382: CLOUDSTACK-4045 IP address acquired with associateIpAddress is marked as source NAT
houthuis opened a new pull request #2382: CLOUDSTACK-4045 IP address acquired with associateIpAddress is marked as source NAT URL: https://github.com/apache/cloudstack/pull/2382 added a check for network state when determining whether a new IP should be source NAT. this prevents associated IP's to be marked as source NAT when the network is in allocated state, causing disassociateIpAddress to fail later Code will now throw a InvalidParameterValueException in the above scenario. 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] fmaximus opened a new pull request #2441: CLOUDSTACK-10261: Libvirt metadata: only create one nuage-extension tag
fmaximus opened a new pull request #2441: CLOUDSTACK-10261: Libvirt metadata: only create one nuage-extension tag URL: https://github.com/apache/cloudstack/pull/2441 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 #2382: CLOUDSTACK-4045 IP address acquired with associateIpAddress is marked as source NAT
blueorangutan commented on issue #2382: CLOUDSTACK-4045 IP address acquired with associateIpAddress is marked as source NAT URL: https://github.com/apache/cloudstack/pull/2382#issuecomment-362204779 Packaging result: ?centos6 ?centos7 ?debian. JID-1686 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 #2382: CLOUDSTACK-4045
blueorangutan commented on issue #2382: CLOUDSTACK-4045 URL: https://github.com/apache/cloudstack/pull/2382#issuecomment-362198232 @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 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 issue #2382: CLOUDSTACK-4045
DaanHoogland commented on issue #2382: CLOUDSTACK-4045 URL: https://github.com/apache/cloudstack/pull/2382#issuecomment-362198015 @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] niteshsarda opened a new pull request #2440: CLOUDSTACK-10260 : Introducing a new API to collect and list CPU Sockets Metrics information for all Hypervisors.
niteshsarda opened a new pull request #2440: CLOUDSTACK-10260 : Introducing a new API to collect and list CPU Sockets Metrics information for all Hypervisors. URL: https://github.com/apache/cloudstack/pull/2440 **Requirement :** Introduce a new API to collect and list CPU Sockets Metrics information for each hypervisor. Currently, listHosts API is fired for each hypervisor seprately to retrieve the count of CPU sockets. So, with this fix instead of multiple API execution, single API query will be executed to reterieve information for all the hypervisors. **Implementation :** 1. Introduced a new API namely "listCpuSocketsMetrics", which will fetch details of CPU Sockets for all the hypervisors in a single run. 2. In this API, a query will be fired on a host table to fetch details based on grouping of Hypervisor type and Hypervisor version. 3. This API will prevent multiple calls of listHosts API and thus will improve performance of the infrastructure tab. Api Response Screenshot :  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
