[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103890661
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/ha/DisableHAForClusterCmd.java 
---
@@ -0,0 +1,114 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.ha;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.org.Cluster;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.ha.HAConfigManager;
+
+import javax.inject.Inject;
+
+@APICommand(name = DisableHAForClusterCmd.APINAME, description = "Disables 
HA cluster-wide",
--- End diff --

If we decide to implement some kind of mirroring service for entire zones 
or clusters for high availability, these would then be disabled / enabled for a 
zone or cluster. In this case the enabling is done per host in the 
zone/cluster. That's why i consider the longer name I propose more appropriate. 
That said the shorter the name the better as long as it is unambiguous.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103872151
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java
 ---
@@ -108,4 +108,8 @@ public void execute() {
 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, 
"Failed to prepare host for maintenance");
 }
 }
+
+public void setHostId(final Long hostId) {
--- End diff --

the field name is id, so i don't understand your reply


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[rhtyd]

Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103397882
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java
 ---
@@ -108,4 +108,8 @@ public void execute() {
 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, 
"Failed to prepare host for maintenance");
 }
 }
+
+public void setHostId(final Long hostId) {
--- End diff --

I think the setters and getters are named per the variable, since the 
variable is hostId IDEs generate getHostId, setHostId named methods.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[rhtyd]

Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103397573
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/ha/DisableHAForClusterCmd.java 
---
@@ -0,0 +1,114 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.ha;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.org.Cluster;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.ha.HAConfigManager;
+
+import javax.inject.Inject;
+
+@APICommand(name = DisableHAForClusterCmd.APINAME, description = "Disables 
HA cluster-wide",
--- End diff --

@DaanHoogland can you illustrate an example where we may hit such a 
conflict? This API actually disables the feature (framework etc) for 
cluster/zone etc. While the Host specific APIs work on the host.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103221397
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/outofbandmanagement/ChangeOutOfBandManagementPasswordCmd.java
 ---
@@ -74,7 +74,7 @@ public void execute() throws 
ResourceUnavailableException, InsufficientCapacityE
 CallContext.current().setEventDetails("Host Id: " + host.getId() + 
" Password: " + getPassword().charAt(0) + "");
 CallContext.current().putContextParameter(Host.class, 
host.getUuid());
 
-final OutOfBandManagementResponse response = 
outOfBandManagementService.changeOutOfBandManagementPassword(host, 
getPassword());
+final OutOfBandManagementResponse response = 
outOfBandManagementService.changePassword(host, getPassword());
--- End diff --

:+1:


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103221221
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java
 ---
@@ -108,4 +108,8 @@ public void execute() {
 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, 
"Failed to prepare host for maintenance");
 }
 }
+
+public void setHostId(final Long hostId) {
--- End diff --

naming: consistency would dictate this be called setId()


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103207344
  
--- Diff: .travis.yml ---
@@ -42,13 +42,18 @@ env:
  smoke/test_dynamicroles
  smoke/test_global_settings
  smoke/test_guest_vlan_range
+ smoke/test_ha_for_host
--- End diff --

:+1: :)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103210828
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/ha/DisableHAForClusterCmd.java 
---
@@ -0,0 +1,114 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.ha;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.org.Cluster;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.ha.HAConfigManager;
+
+import javax.inject.Inject;
+
+@APICommand(name = DisableHAForClusterCmd.APINAME, description = "Disables 
HA cluster-wide",
--- End diff --

naming: looking at the code below this actually is 
DisableHAForHostsInClusterCmd. I don't think it is a biggy but might lead to 
conflicts at some point


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103212736
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/ha/DisableHAForZoneCmd.java ---
@@ -0,0 +1,115 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.ha;
+
+import com.cloud.dc.DataCenter;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.ha.HAConfigManager;
+
+import javax.inject.Inject;
+
+@APICommand(name = DisableHAForZoneCmd.APINAME, description = "Disables HA 
for a zone",
--- End diff --

naming: see disable cluster cmd


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1960#discussion_r103212914
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/ha/EnableHAForClusterCmd.java 
---
@@ -0,0 +1,114 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.ha;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.org.Cluster;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.ha.HAConfigManager;
+
+import javax.inject.Inject;
+
+@APICommand(name = EnableHAForClusterCmd.APINAME, description = "Enables 
HA cluster-wide",
+responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
+since = "4.11", authorized = {RoleType.Admin})
+public final class EnableHAForClusterCmd extends BaseAsyncCmd {
--- End diff --

naming: see disable cmd


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[rhtyd]

Github user rhtyd closed the pull request at:

https://github.com/apache/cloudstack/pull/1960


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KV...

[rhtyd]

GitHub user rhtyd reopened a pull request:

https://github.com/apache/cloudstack/pull/1960

[4.11/Future] CLOUDSTACK-9782: Host HA and KVM HA provider

Host-HA offers investigation, fencing and recovery mechanisms for host that 
for
any reason are malfunctioning. It uses Activity and Health checks to 
determine
current host state based on which it may degrade a host or try to recover 
it. On
failing to recover it, it may try to fence the host.

The core feature is implemented in a hypervisor agnostic way, with two 
separate
implementations of the driver/provider for Simulator and KVM hypervisors. 
The
framework also allows for implementation of other hypervisor specific 
provider
implementation in future.

The Host-HA provider implementation for KVM hypervisor uses the out-of-band
management sub-system to issue IPMI calls to reset (recover) or poweroff 
(fence)
a host.

The Host-HA provider implementation for Simulator provides a means of 
testing 
and validating the core framework implementation.

FS: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Host+HA

Signed-off-by: Abhinandan Prateek 
Signed-off-by: Rohit Yadav 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shapeblue/cloudstack host-ha-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1960.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 #1960


commit 24ec0dd70a97be738e18cc66a6ec0401edd7f32f
Author: Rohit Yadav 
Date:   2017-02-15T12:06:18Z

CLOUDSTACK-9782: Host HA and KVM HA provider

Host-HA offers investigation, fencing and recovery mechanisms for host that 
for
any reason are malfunctioning. It uses Activity and Health checks to 
determine
current host state based on which it may degrade a host or try to recover 
it. On
failing to recover it, it may try to fence the host.

The core feature is implemented in a hypervisor agnostic way, with two 
separate
implementations of the driver/provider for Simulator and KVM hypervisors. 
The
framework also allows for implementation of other hypervisor specific 
provider
implementation in future.

The Host-HA provider implementation for KVM hypervisor uses the out-of-band
management sub-system to issue IPMI calls to reset (recover) or poweroff 
(fence)
a host.

The Host-HA provider implementation for Simulator provides a means of 
testing
and validating the core framework implementation.

Signed-off-by: Abhinandan Prateek 
Signed-off-by: Rohit Yadav 




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---