You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/10/01 00:02:15 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4363: Ability to put a server in Down state to maintenance
ravening opened a new pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363
## Description
<!--- Describe your changes in detail -->
Adds a new global setting set.host.down.to.maintenance which when set
to true will allow setting a host in downstate into maintenance mode
so that when the host comes back up, it will be automatically set to
maintenance mode and the resource state will not be set to Enabled.
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
1, Power off one hypervisor so that it enters DOWN state
2. set the global setting `set.host.down.to.maintenance` to true
3. Enable maintenance mode on that host
4. Start the hypervisor
5. When it comes back up, it will be in maintenance mode
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717480423
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503709089
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
+ if (host.getResourceState() == ResourceState.Enabled) {
+ _hostDao.updateResourceState(ResourceState.Enabled, ResourceState.Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance, host);
+ _hostDao.updateResourceState(ResourceState.PrepareForMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ } else if (host.getResourceState() == ResourceState.ErrorInMaintenance) {
+ _hostDao.updateResourceState(ResourceState.ErrorInMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ }
Review comment:
@ravening any specific reason to skip "_ResourceState.ErrorInPrepareForMaintenance_", and consider "_ResourceState.ErrorInMaintenance_", to put the host in maintenance? What if the host is back Up when the resource state is "ErrorInPrepareForMaintenance" ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-702378372
@ravening what do you think PR https://github.com/apache/cloudstack/pull/4111? Does that PR fit the context of this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-714304067
@GabrielBrascher so whats your opinion on this pr? shall I close it and take yours further?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503710867
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
@ravening Currently, the config scope is Zone, do you want to mark that as Global scope?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717361236
Thanks, @ravening!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717361340
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-715511318
@ravening your PR covers a different scenario, very similar though. From what I understand, this PR avoids a problematic host to get back from down to Enabled.
I don't see any problem in merging this one, but here follows a small summary on what #4111 does:
In summary, it presents an API command to manually fence a problematic host. The command declares a host as degraded moving it from Alert or Disconnected to "Degraded" state. All VMs that were running and had HA enabled will be started on another host, VMs with HA disabled will triansit from "Running" state (obviously they are not running, but CloudStack does not know that) to Stopped.
In summary, this implementation is a simple way of avoiding a problematic host to get back and does not require admins to take action. The other PR requires admins to execute an API command when they know a Disconnected/Alert host is down and has no running VMs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503699026
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
+ if (host.getResourceState() == ResourceState.Enabled) {
+ _hostDao.updateResourceState(ResourceState.Enabled, ResourceState.Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance, host);
+ _hostDao.updateResourceState(ResourceState.PrepareForMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ } else if (host.getResourceState() == ResourceState.ErrorInMaintenance) {
+ _hostDao.updateResourceState(ResourceState.ErrorInMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ }
Review comment:
> @ravening can also consider state "_ResourceState.ErrorInPrepareForMaintenance_" ?
@sureshanaparti no. This is applicable only for hosts in maintenance state
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503698395
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
> @ravening consider the value in zone, as the config scope is specified as Zone.
>
>
>
> ```suggestion
>
> if (SET_HOST_DOWN_TO_MAINTENANCE.valueIn(host.getDataCenterId()) && (host.getStatus() == Status.Down)) {
>
> ```
@sureshanaparti i don't think we need that because the host can exist only in one zone at any time but I can change the setting to zone scope instead of global scope
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503712301
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
+ if (host.getResourceState() == ResourceState.Enabled) {
+ _hostDao.updateResourceState(ResourceState.Enabled, ResourceState.Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance, host);
+ _hostDao.updateResourceState(ResourceState.PrepareForMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ } else if (host.getResourceState() == ResourceState.ErrorInMaintenance) {
+ _hostDao.updateResourceState(ResourceState.ErrorInMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ }
Review comment:
> @ravening any specific reason to skip "_ResourceState.ErrorInPrepareForMaintenance_", and consider "_ResourceState.ErrorInMaintenance_", to put the host in maintenance? What if the host is back Up when the resource state is "ErrorInPrepareForMaintenance" ?
@sureshanaparti this code doesn't take care of that... This is only for hosts already in maintenance mode
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717383042
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2285
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r511082691
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
This observation makes sense. Can you please take a look at _org.apache.cloudstack.framework.config.ConfigKey.valueIn()_ @ravening?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-715511318
@ravening your PR covers a different scenario, very similar though. From what I understand, this PR avoids a problematic host to get back from down to Enabled.
I don't see any problem in merging this one, but here follows a small summary on what #4111 does:
In summary, it presents an API command to manually fence a problematic host. The command declares a host as degraded moving it from Alert or Disconnected to "Degraded" state. All VMs that were running and had HA enabled will be started on another host, VMs with HA disabled will triansit from "Running" state (obviously they are not running, but CloudStack does not know that) to Stopped.
In summary, this implementation (#4363) is a simple way of avoiding a problematic host to get back and does not require admins to take action. The other PR (#4111) requires admins to execute an API command when they know a Disconnected/Alert host is down and has no running VMs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-712732594
> @ravening small question. down means not enabled?
@svenvogel need not be. The host is still enabled but is in down state
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache closed pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503480390
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
@ravening what do you think of extracting lines 1308 - 1318 to a method? With that, it would be possible to cover these cases with JUnit test methods?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] svenvogel commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
svenvogel commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-712731580
@ravening small question. down means not enabled?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-712740157
@ravening can you look at the conflict?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717793239
@GabrielBrascher functionality makes sense and is guarded by a setting. 2 lgtms and test-report, so merging
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503716591
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
+ if (host.getResourceState() == ResourceState.Enabled) {
+ _hostDao.updateResourceState(ResourceState.Enabled, ResourceState.Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance, host);
+ _hostDao.updateResourceState(ResourceState.PrepareForMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ } else if (host.getResourceState() == ResourceState.ErrorInMaintenance) {
+ _hostDao.updateResourceState(ResourceState.ErrorInMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ }
Review comment:
ok @ravening
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503737321
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
@sureshanaparti I dont think I need to make changes here as config is at zone level..setting in one zone doesnt mess up setting in other zones
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] svenvogel edited a comment on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
svenvogel edited a comment on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-712731580
@ravening small question. maybe you can explain it a little more. down means not enabled? you mean if the host is disabled he can be put in the maintenance mode?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503661983
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
+ if (host.getResourceState() == ResourceState.Enabled) {
+ _hostDao.updateResourceState(ResourceState.Enabled, ResourceState.Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance, host);
+ _hostDao.updateResourceState(ResourceState.PrepareForMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ } else if (host.getResourceState() == ResourceState.ErrorInMaintenance) {
+ _hostDao.updateResourceState(ResourceState.ErrorInMaintenance, ResourceState.Event.InternalEnterMaintenance, ResourceState.Maintenance, host);
+ return _hostDao.findById(hostId);
+ }
Review comment:
@ravening can also consider state "_ResourceState.ErrorInPrepareForMaintenance_" ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-702393180
> @ravening what do you think PR https://github.com/apache/cloudstack/pull/4111? Does that PR fit the context of this PR?
@GabrielBrascher looked at it and have few questions
The declare degrade API works only if host is in disconnected state?
What if we set host to maintenance mode and after that for some reason it goes down, we need to execute the api.
But with global setting we don't need to execute another API.
Also to declare host as not dead, we need to call another API and then cancel maintenance mode but with global setting , you need to just cancel maintenance mode
Apart from these two extra steps, both are doing same functionality
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503804331
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
@ravening May be some confusion here. My point is, use the config value specified at the scope defined in the config parameter, else update the config scope accordingly.
The config "SET_HOST_DOWN_TO_MAINTENANCE" defined above uses Zone Scope. So, In order to pick the Zone specific value of that config, you have to use "_valueIn(<zoneId>)_", not _value()_ (this picks the global config value). If your use case is to set the config at Cloud level, then change Scope to Global.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717300323
> @ravening can you please fix the conflicts?
@GabrielBrascher have resolved the conflicts
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-715524721
@ravening can you please fix the conflicts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717479796
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717361760
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-717706581
<b>Trillian test result (tid-3089)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34311 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4363-t3089-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#issuecomment-703018771
@ravening the declare host as degraded API command would serve to put a problematic host on a special state that would allow to mark the VMs as stopped and trigger HA to start VMs (when service offering has _haenabled=true_) on another hosts.
Your implementation is nice and covers some interesting ground. The API command requires intervention from ADMIN with care. This is because if the host is Disconnected but there are VMs running in it and writing on storage it may cause corruption if VMs start on another host and write on same storage. Therefore that command should only be executed if the host is down.
API description: _Declare host as 'Degraded'. Host must be on 'Disconnected' or 'Alert' state. The ADMIN must be sure that there are no VMs running on the respective host otherwise this command might corrupted VMs that were running on the 'Degraded' host_.
Threfore, it cannot be automated. I think that both implementations cover different gaps on some corner cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4363: Ability to put a server in Down state to maintenance
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4363:
URL: https://github.com/apache/cloudstack/pull/4363#discussion_r503659931
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -1302,6 +1305,17 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) {
throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved.");
}
+ if (SET_HOST_DOWN_TO_MAINTENANCE.value() && (host.getStatus() == Status.Down)) {
Review comment:
@ravening consider the value in zone, as the config scope is specified as Zone.
```suggestion
if (SET_HOST_DOWN_TO_MAINTENANCE.valueIn(host.getDataCenterId()) && (host.getStatus() == Status.Down)) {
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org