You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by koushik-das <gi...@git.apache.org> on 2017/01/10 10:45:24 UTC
[GitHub] cloudstack pull request #1899: CLOUDSTACK-9650: Allow starting VMs regardles...
GitHub user koushik-das opened a pull request:
https://github.com/apache/cloudstack/pull/1899
CLOUDSTACK-9650: Allow starting VMs regardless of cpu/memory cluster.\u2026
\u2026disablethreshold setting
Fixed the scope of configuration flag 'cluster.threshold.enabled' introduced as part of PR#1812 to global
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9650
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1899.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1899
----
commit 44a59652489f4129fd09d4c8de0bb6f0247d8fd7
Author: Koushik Das <ko...@apache.org>
Date: 2017-01-10T10:38:59Z
CLOUDSTACK-9650: Allow starting VMs regardless of cpu/memory cluster.disablethreshold setting
Fixed the scope of configuration flag 'cluster.threshold.enabled' introduced as part of PR#1812 to global
----
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@koushik-das there are no test results on this PR, also can you explain why you've removed the option to override this setting at the zone level?
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by serg38 <gi...@git.apache.org>.
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1899
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:
https://github.com/apache/cloudstack/pull/1899
LTGM
smoke test is not necessary
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@rhtyd If you had checked #1812 you wouldn't have asked these questions :)
This config was introduced in #1812 and the scope was incorrectly put as zone, the config is meant to be a global 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1899
Merging this
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@koushik-das there are no tests, Java related code changes ought to require regression tests, even if it may appear to not need them. Let's not work towards degrading the branch health.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@ustcweizhou please 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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@koushik-das as a RM I want to ensure that all changes, irrespective of their size, are merged after a QA process.
For UI/translation/docs changes tests are not necessary. Please understand that it's difficult to review and keep track of each and every PR in detail, and help and support from the community is appreciated to make this work. We have agreed to gate the branches, and you've went ahead with merging without following proper guidelines; testing against one hypervisor which have errors that may/may-not be related to a PR are not good enough, given we've systems like Trillian to test against at least three major hypervisors.
Lastly, I disagree, I think the setting should be available per zone for admins to override this on a per-zone basis.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1899: CLOUDSTACK-9650: Allow starting VMs regardless of cp...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1899
@rhtyd Tests are present in #1812
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1899: CLOUDSTACK-9650: Allow starting VMs regardles...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1899
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1899: CLOUDSTACK-9650: Allow starting VMs regardles...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1899#discussion_r95524178
--- Diff: api/src/com/cloud/deploy/DeploymentClusterPlanner.java ---
@@ -55,7 +55,7 @@
"true",
"Enable/Disable cluster thresholds. If disabled, an instance can start in a cluster even though the threshold may be crossed.",
false,
- ConfigKey.Scope.Zone);
+ ConfigKey.Scope.Global);
--- End diff --
This refers to cluster level setting, admins should be allowed to override this at the cluster level. Please change this to ConfigKey.Scope.Cluster.
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1899: CLOUDSTACK-9650: Allow starting VMs regardles...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1899#discussion_r95524526
--- Diff: api/src/com/cloud/deploy/DeploymentClusterPlanner.java ---
@@ -55,7 +55,7 @@
"true",
"Enable/Disable cluster thresholds. If disabled, an instance can start in a cluster even though the threshold may be crossed.",
false,
- ConfigKey.Scope.Zone);
+ ConfigKey.Scope.Global);
--- End diff --
@koushik-das Ignore, the above comment. But, I think making this global takes away the option to override this at a zone level? Why remove this?
---
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 is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---