wido commented on a change in pull request #5862:
URL: https://github.com/apache/cloudstack/pull/5862#discussion_r811591901
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java
##########
@@ -71,6 +73,29 @@ public Boolean checkingHeartBeat() {
}
}
+ for (RbdStoragePool pool : rbdStoragePools) {
+ Script cmd = new Script(s_heartBeatPathRbd,
heartBeatCheckerTimeout, s_logger);
+ cmd.add("-i", pool._poolSourceHost);
+ cmd.add("-p", pool._poolMountSourcePath);
+ cmd.add("-n", pool._poolAuthUserName);
+ cmd.add("-s", pool._poolAuthSecret);
+ cmd.add("-h", hostIp);
Review comment:
I know we use this with NFS, but isn't it better to use the UUID of the
host here?
Each host has a unique UUID and that seems safer to use then the Private
IPv4 address of the host.
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java
##########
@@ -71,6 +73,29 @@ public Boolean checkingHeartBeat() {
}
}
+ for (RbdStoragePool pool : rbdStoragePools) {
+ Script cmd = new Script(s_heartBeatPathRbd,
heartBeatCheckerTimeout, s_logger);
+ cmd.add("-i", pool._poolSourceHost);
+ cmd.add("-p", pool._poolMountSourcePath);
+ cmd.add("-n", pool._poolAuthUserName);
+ cmd.add("-s", pool._poolAuthSecret);
+ cmd.add("-h", hostIp);
+ cmd.add("-r");
+ OutputInterpreter.OneLineParser parser = new
OutputInterpreter.OneLineParser();
+ String result = cmd.execute(parser);
+ String parsedLine = parser.getLine();
+
+ s_logger.debug(String.format("Checking heart beat with
KVMHAChecker [{command=\"%s\", result: \"%s\", log: \"%s\", pool: \"%s\"}].",
cmd.toString(), result, parsedLine,
+ pool._poolIp));
+
+ if (result == null && parsedLine.contains("DEAD")) {
+ s_logger.warn(String.format("Checking heart beat with
KVMHAChecker command [%s] returned [%s]. [%s]. It may cause a shutdown of host
IP [%s].", cmd.toString(),
Review comment:
Then also use the UUID here.
##########
File path: scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh
##########
@@ -0,0 +1,136 @@
+#!/bin/bash
+# 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.
+
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
Review comment:
Again, I suggest using the UUID of the Host
##########
File path: scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh
##########
@@ -0,0 +1,136 @@
+#!/bin/bash
+# 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.
+
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
+ -r cretae/read hb watcher
+ -c cleanup"
+ exit 1
+}
+#set -x
+PoolName=
+PoolAuthUserName=
+PoolAuthSecret=
+HostIP=
+SourceHostIP=
+rflag=0
+cflag=0
+
+while getopts 'p:n:s:h:i:rc' OPTION
+do
+ case $OPTION in
+ p)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ PoolAuthUserName="$OPTARG"
+ ;;
+ s)
+ PoolAuthSecret="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ i)
+ SourceHostIP="$OPTARG"
+ ;;
+ r)
+ rflag=1
+ ;;
+ c)
+ cflag=1
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$PoolName" ]; then
+ exit 2
+fi
+
+keyringFile="/etc/cloudstack/agent/keyring"
Review comment:
This will break if multiple Ceph clusters are attached to the same host.
This is technically possible and also done (we do this for example).
At least you need a unique keyfile per Ceph cluster, but I would even say
that we should not write this credentials to a file anyway.
If we do so at least write it to a temporary directory in /run somewhere and
make sure the chmod permissions are set properly so no other user can read it.
##########
File path: scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh
##########
@@ -0,0 +1,107 @@
+#!/bin/bash
+# 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.
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
+ -u volume uuid list"
+ exit 1
+}
+#set -x
+PoolName=
+PoolAuthUserName=
+PoolAuthSecret=
+HostIP=
+SourceHostIP=
+UUIDList=
+
+while getopts 'p:n:s:h:i:u:d:' OPTION
+do
+ case $OPTION in
+ p)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ PoolAuthUserName="$OPTARG"
+ ;;
+ s)
+ PoolAuthSecret="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ i)
+ SourceHostIP="$OPTARG"
+ ;;
+ u)
+ UUIDList="$OPTARG"
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$PoolName" ]; then
+ exit 2
+fi
+
+#Creating Ceph keyring for executing rbd commands
+keyringFile="/etc/cloudstack/agent/keyring.bin"
Review comment:
See my earlier comment about these keyring files.
##########
File path: scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh
##########
@@ -0,0 +1,136 @@
+#!/bin/bash
+# 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.
+
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
+ -r cretae/read hb watcher
Review comment:
Typo
##########
File path:
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -139,7 +139,13 @@ public DataStore initialize(Map<String, Object> dsInfos) {
PrimaryDataStoreParameters parameters = new
PrimaryDataStoreParameters();
URI uri = null;
+ boolean multi = false;
try {
+ String urlType = url.substring(0, 3);
Review comment:
Should we not do this split after we have used the URI class and then
just split the 'host' part?
Now you are doing manual string parsing on a string which we will then run
through a library.
##########
File path: scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh
##########
@@ -0,0 +1,151 @@
+#!/bin/bash
+# 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.
+
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
+ -r write/read hb log
+ -c cleanup
+ -t interval between read hb log\n"
+ exit 1
+}
+#set -x
+PoolName=
+PoolAuthUserName=
+PoolAuthSecret=
+HostIP=
+SourceHostIP=
+interval=
+rflag=0
+cflag=0
+
+while getopts 'p:n:s:h:i:t:rc' OPTION
+do
+ case $OPTION in
+ p)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ PoolAuthUserName="$OPTARG"
+ ;;
+ s)
+ PoolAuthSecret="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ i)
+ SourceHostIP="$OPTARG"
+ ;;
+ t)
+ interval="$OPTARG"
+ ;;
+ r)
+ rflag=1
+ ;;
+ c)
+ cflag=1
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$PoolName" ]; then
+ exit 2
+fi
+
+keyringFile="/etc/ceph/keyring"
+confFile="/etc/ceph/ceph.conf"
+
+create_cephConf() {
+#Creating Ceph keyring and conf for executing rados commands
+ if [ ! -f $keyringFile ]; then
+ echo -e "[client.$PoolAuthUserName]\n key=$PoolAuthSecret" > $keyringFile
Review comment:
This still stands. We should not write to /etc, never.
IF we need such a file, use /run/cloudstack/agent (for example) to make sure
the files are gone after a reboot.
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java
##########
@@ -57,6 +58,28 @@ public NfsStoragePool(String poolUUID, String poolIp, String
poolSourcePath, Str
}
}
+ public static class RbdStoragePool {
+ String _poolUUID;
+ String _poolIp;
+ String _poolMountSourcePath;
+ String _mountDestPath;
+ PoolType _type;
+ String _poolAuthUserName;
+ String _poolAuthSecret;
+ String _poolSourceHost;
+
+ public RbdStoragePool(String poolUUID, String poolIp, String
poolSourcePath, String mountDestPath, PoolType type, String poolAuthUserName,
String poolAuthSecret, String poolSourceHost) {
+ _poolUUID = poolUUID;
+ _poolIp = poolIp;
Review comment:
I would not call this 'poolIP'. The correct naming is the mon_host as it
can contain an IP-address of a hostname.
##########
File path:
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -162,10 +168,18 @@ public DataStore initialize(Map<String, Object> dsInfos) {
throw new InvalidParameterValueException("host or path is
null, should be sharedmountpoint://localhost/path");
}
} else if (uri.getScheme().equalsIgnoreCase("rbd")) {
+ String uriHost = uri.getHost();
String uriPath = uri.getPath();
if (uriPath == null) {
throw new InvalidParameterValueException("host or path is
null, should be rbd://hostname/pool");
}
+ if (multi) {
Review comment:
We can probably just split uriHost here.
##########
File path: scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh
##########
@@ -0,0 +1,136 @@
+#!/bin/bash
+# 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.
+
+help() {
+ printf "Usage: $0
+ -p rbd pool name
+ -n pool auth username
+ -s pool auth secret
+ -h host
+ -i source host ip
+ -r cretae/read hb watcher
+ -c cleanup"
+ exit 1
+}
+#set -x
+PoolName=
+PoolAuthUserName=
+PoolAuthSecret=
+HostIP=
+SourceHostIP=
+rflag=0
+cflag=0
+
+while getopts 'p:n:s:h:i:rc' OPTION
+do
+ case $OPTION in
+ p)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ PoolAuthUserName="$OPTARG"
+ ;;
+ s)
+ PoolAuthSecret="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ i)
+ SourceHostIP="$OPTARG"
+ ;;
+ r)
+ rflag=1
+ ;;
+ c)
+ cflag=1
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$PoolName" ]; then
+ exit 2
+fi
+
+keyringFile="/etc/cloudstack/agent/keyring"
+
+create_cephKeyring() {
+#Creating Ceph keyring for executing rbd commands
+ if [ ! -f $keyringFile ]; then
+ echo -e "[client.$PoolAuthUserName]\n key=$PoolAuthSecret" > $keyringFile
+ fi
+}
+
+delete_cephKeyring() {
+#Deleting Ceph keyring
+ if [ -f $keyringFile ]; then
+ rm -rf $keyringFile
+ fi
+}
+
+cretae_hbWatcher() {
+#Create HB RBD Image and watcher
+ status=$(rbd status hb-$HostIP --pool $PoolName -m $SourceHostIP -k
$keyringFile)
+ if [ $? == 2 ]; then
+ rbd create hb-$HostIP --size 1 --pool $PoolName
+ setsid sh -c 'exec rbd watch hb-'$HostIP' --pool $PoolName -m
'$SourceHostIP' -k '$keyringFile' <> /dev/tty20 >&0 2>&1'
+ fi
+
+ if [ "$status" == "Watchers: none" ]; then
+ setsid sh -c 'exec rbd watch hb-'$HostIP' --pool $PoolName -m
'$SourceHostIP' -k '$keyringFile' <> /dev/tty20 >&0 2>&1'
+ fi
+
+ return 0
+}
+
+check_hbWatcher() {
+#check the heart beat watcher
+ hb=$(rbd status hb-$HostIP --pool $PoolName -m $SourceHostIP -k $keyringFile)
+ if [ "$hb" == "Watchers: none" ]; then
+ return 2
+ else
+ return 0
+ fi
+}
+
+if [ "$rflag" == "1" ]; then
+ create_cephKeyring
+ check_hbWatcher
+ hb=$?
+ if [ "$hb" != "Watchers: none" ]; then
+ echo "=====> ALIVE <====="
+ else
+ echo "=====> Considering host as DEAD due to [hb-$HostIP] watcher that the
host is seeing is not running. <======"
+ fi
+ delete_cephKeyring
+ exit 0
+elif [ "$cflag" == "1" ]; then
+ /usr/bin/logger -t heartbeat "kvmheartbeat_rbd.sh reboots the system because
there is no heartbeat watcher."
+ sync &
+ sleep 5
+ echo b > /proc/sysrq-trigger
Review comment:
Do we really need to kill the host? Since Ceph/RBD lives in userspace we
could also 'just' kill all the VMs which are using this storage pool.
No need to reboot the host completely.
With NFS you have something hanging in kernel space and causes I/O to lock
up. This is not the case with RBD.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
