Copilot commented on code in PR #13090: URL: https://github.com/apache/cloudstack/pull/13090#discussion_r3987315325
########## scripts/vm/hypervisor/kvm/kvmha-fence.sh: ########## @@ -0,0 +1,85 @@ +#!/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. +# +# Shared fence-action helper for kvmheartbeat.sh and kvmspheartbeat.sh. +# Sourced by both scripts; do not invoke directly. +# +# Usage from caller: +# source "$(dirname "$0")/kvmha-fence.sh" +# fence_action "kvmheartbeat.sh" # script name passed for log tagging + +AGENT_PROPS="${AGENT_PROPS:-/etc/cloudstack/agent/agent.properties}" + +fence_action() { + local source_script="${1:-kvmha}" + local FENCE_ACTION="hard-reboot" + local CUSTOM_SCRIPT="/etc/cloudstack/agent/heartbeat-fence-custom.sh" + + if [ -r "$AGENT_PROPS" ]; then + local val + val=$(grep "^kvm.heartbeat.fence.action=" "$AGENT_PROPS" | tail -n 1 | cut -d= -f2- | tr -d '[:space:]') + [ -n "$val" ] && FENCE_ACTION="$val" + local cval + cval=$(grep "^kvm.heartbeat.fence.custom.script=" "$AGENT_PROPS" | tail -n 1 | cut -d= -f2- | tr -d '[:space:]') + [ -n "$cval" ] && CUSTOM_SCRIPT="$cval" + fi + + case "$FENCE_ACTION" in + log-only) + /usr/bin/logger -t heartbeat "${source_script}: heartbeat write to storage failed; fence action 'log-only' selected — taking no automatic action. Operator must investigate." Review Comment: This branch only writes a local syslog message and returns success. `KVMHAMonitor` logs locally and ignores the result of the `-c` command, so with `log-only` there is no management-plane alert or host-status transition for the controller to observe; the documented “log + alert” behavior is not implemented. Emit an explicit management alert, or change the documented semantics to local logging only. ########## agent/src/main/java/com/cloud/agent/properties/AgentProperties.java: ########## @@ -622,6 +622,44 @@ public class AgentProperties{ public static final Property<Boolean> REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT = new Property<>("reboot.host.and.alert.management.on.heartbeat.timeout", false); + /** + * Action taken by the KVM agent's storage heartbeat scripts (kvmheartbeat.sh / kvmspheartbeat.sh) + * when a heartbeat write fails persistently. Allowed values: + * <ul> + * <li>{@code hard-reboot} (default; {@code reboot} accepted as alias) — immediate + * sysrq-trigger reboot. Required default for setups where a stale NFSv3 mount can Review Comment: This Javadoc still says `reboot` is an accepted alias, but the documented action list and the prior agreed contract removed that alias; the shell only handles it indirectly through the catch-all unknown-value fallback. Please document only `hard-reboot` here so the Java discoverability metadata matches the supported values. ########## scripts/vm/hypervisor/kvm/kvmha-fence.sh: ########## @@ -0,0 +1,85 @@ +#!/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. +# +# Shared fence-action helper for kvmheartbeat.sh and kvmspheartbeat.sh. +# Sourced by both scripts; do not invoke directly. +# +# Usage from caller: +# source "$(dirname "$0")/kvmha-fence.sh" +# fence_action "kvmheartbeat.sh" # script name passed for log tagging + +AGENT_PROPS="${AGENT_PROPS:-/etc/cloudstack/agent/agent.properties}" + +fence_action() { + local source_script="${1:-kvmha}" + local FENCE_ACTION="hard-reboot" + local CUSTOM_SCRIPT="/etc/cloudstack/agent/heartbeat-fence-custom.sh" + + if [ -r "$AGENT_PROPS" ]; then + local val + val=$(grep "^kvm.heartbeat.fence.action=" "$AGENT_PROPS" | tail -n 1 | cut -d= -f2- | tr -d '[:space:]') + [ -n "$val" ] && FENCE_ACTION="$val" + local cval + cval=$(grep "^kvm.heartbeat.fence.custom.script=" "$AGENT_PROPS" | tail -n 1 | cut -d= -f2- | tr -d '[:space:]') + [ -n "$cval" ] && CUSTOM_SCRIPT="$cval" + fi + + case "$FENCE_ACTION" in + log-only) + /usr/bin/logger -t heartbeat "${source_script}: heartbeat write to storage failed; fence action 'log-only' selected — taking no automatic action. Operator must investigate." + exit 0 + ;; + restart-agent) + /usr/bin/logger -t heartbeat "${source_script}: heartbeat write to storage failed; fence action 'restart-agent' — restarting cloudstack-agent (running VMs preserved)." + sync & + sleep 2 + systemctl restart cloudstack-agent + exit $? + ;; + graceful-reboot) + /usr/bin/logger -t heartbeat "${source_script}: heartbeat write to storage failed; fence action 'graceful-reboot' — rebooting via systemctl (allows running VMs to stop cleanly)." + sync & + sleep 5 + systemctl reboot + exit $? + ;; + custom) + if [ -x "$CUSTOM_SCRIPT" ]; then + /usr/bin/logger -t heartbeat "${source_script}: heartbeat write to storage failed; fence action 'custom' — running ${CUSTOM_SCRIPT}." + sync & + sleep 2 + "$CUSTOM_SCRIPT" "$source_script" Review Comment: The PR description and linked issue list four actions, but this new branch adds a fifth `custom` mode that executes an operator-configured root script (and introduces a second property for its path). That materially expands the configuration and security/operational surface; please either keep it out of this PR or document the mode and its consequences in the PR description and release notes. -- 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]
