Skip to content

<feature>[kvm]: add kvmagent auto-restart window config #3863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 2 commits into
base: 5.5.16
Choose a base branch
from
Open

<feature>[kvm]: add kvmagent auto-restart window config #3863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7f332b
<feature>[kvm]: add kvmagent auto-restart window config
MaJin1996 Apr 27, 2026
3411af7
<feature>[kvm]: default kvmagent.autorestart.window to 02:00-04:00
MaJin1996 Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions conf/globalConfig/kvm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,12 @@
<defaultValue>10737418240</defaultValue>
<type>java.lang.Long</type>
</config>

<config>
<category>kvm</category>
<name>kvmagent.autorestart.window</name>
<description>Daily time window during which the automatic restart of zstack-kvmagent (triggered by physical memory hard limit alarm) is allowed. Format: HH:MM-HH:MM in 24-hour server local time, e.g. 02:00-04:00. Cross-midnight windows are supported, e.g. 22:00-02:00. Empty value means always allowed (no time restriction).</description>
<defaultValue>02:00-04:00</defaultValue>
<type>java.lang.String</type>
</config>
</globalConfig>
7 changes: 6 additions & 1 deletion plugin/kvm/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<artifactId>plugin</artifactId>
<groupId>org.zstack</groupId>
<version>5.5.0</version>
<version>5.5.0</version>
<relativePath>..</relativePath>
</parent>
<artifactId>kvm</artifactId>
Expand Down Expand Up @@ -34,6 +34,11 @@
<artifactId>maven-artifact</artifactId>
<version>3.0.5</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
3 changes: 3 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ public class KVMGlobalConfig {
@GlobalConfigValidation
public static GlobalConfig KVMAGENT_PHYSICAL_MEMORY_USAGE_HARD_LIMIT = new GlobalConfig(CATEGORY, "kvmagent.physicalmemory.usage.hardlimit");

@GlobalConfigValidation
public static GlobalConfig KVMAGENT_AUTO_RESTART_WINDOW = new GlobalConfig(CATEGORY, "kvmagent.autorestart.window");
Comment on lines +152 to +153
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

空值语义与当前注解默认校验存在冲突。

这里使用裸 @GlobalConfigValidation 会沿用默认 notNull=true/notEmpty=true，与“空值表示不限时段”的需求冲突；用户也无法把已配置窗口恢复为“始终允许”。

💡 建议修复 
-    `@GlobalConfigValidation`
-    public static GlobalConfig KVMAGENT_AUTO_RESTART_WINDOW = new GlobalConfig(CATEGORY, "kvmagent.autorestart.window");
+    `@GlobalConfigValidation`(notNull = false, notEmpty = false)
+    `@GlobalConfigDef`(
+            defaultValue = "",
+            description = "daily local-time window for kvmagent auto restart, format HH:MM-HH:MM; empty means always allowed"
+    )
+    public static GlobalConfig KVMAGENT_AUTO_RESTART_WINDOW = new GlobalConfig(CATEGORY, "kvmagent.autorestart.window");
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalConfig.java` around lines
152 - 153, The GlobalConfig KVMAGENT_AUTO_RESTART_WINDOW currently uses the bare
`@GlobalConfigValidation` which enforces notNull=true/notEmpty=true and thus
prevents using null/empty to mean "no time window"; update the annotation on
KVMAGENT_AUTO_RESTART_WINDOW to explicitly allow null/empty (e.g., set
notNull=false and notEmpty=false) so users can clear the configured window to
mean "always allowed", and ensure any downstream validation logic that reads
KVMAGENT_AUTO_RESTART_WINDOW honors null/empty as "no restriction".


@GlobalConfigDef(defaultValue = "0G", description = "minimum free memory size to start vm, size in GB")
@BindResourceConfig({HostVO.class, ClusterVO.class})
public static GlobalConfig MINIMUM_MEMORY_SIZE_BEFORE_START_VM = new GlobalConfig(CATEGORY, "min.free.memory.size");
Expand Down
47 changes: 47 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
import java.nio.channels.Selector;
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.time.LocalTime;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -459,6 +460,14 @@ private void processKvmagentPhysicalMemUsageAbnormal(HostProcessPhysicalMemoryUs
return;
}

String window = KVMGlobalConfig.KVMAGENT_AUTO_RESTART_WINDOW.value();
if (!isNowInAutoRestartWindow(window, LocalTime.now())) {
logger.info(String.format("zstack-kvmagent on host %s exceeded physical memory hard limit, " +
"but current time is outside auto-restart window [%s]; skip auto-restart, will retry on next alarm",
cmd.getHostUuid(), window));
return;
}

logger.debug("The zstack-kvmagent service has exceeded the hard limit for physical memory usage, " +
"and we will try restart it later");
RestartKvmAgentMsg restartKvmAgentMsg = new RestartKvmAgentMsg();
Expand All @@ -467,6 +476,19 @@ private void processKvmagentPhysicalMemUsageAbnormal(HostProcessPhysicalMemoryUs
bus.send(restartKvmAgentMsg);
}

static boolean isNowInAutoRestartWindow(String configValue, LocalTime now) {
if (configValue == null || configValue.trim().isEmpty()) {
return true;
}
String[] parts = configValue.trim().split("-");
LocalTime start = LocalTime.parse(parts[0]);
LocalTime end = LocalTime.parse(parts[1]);
if (start.isBefore(end)) {
return !now.isBefore(start) && now.isBefore(end);
}
return !now.isBefore(start) || now.isBefore(end);
}
Comment on lines +479 to +490
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 检查窗口解析实现是否存在无兜底解析 =="
rg -n -A20 -B5 'static boolean isNowInAutoRestartWindow|LocalTime\.parse|split\("-"\)' plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java

echo
echo "== 检查测试是否覆盖非法配置输入场景 =="
rg -n 'inWindow\(|assert(True|False)|Exception' plugin/kvm/src/test/java/org/zstack/kvm/TestKVMAutoRestartWindow.java
rg -n '25:00|aa:bb|invalid|malformed|02:00-02:00|02:00-' plugin/kvm/src/test/java/org/zstack/kvm/TestKVMAutoRestartWindow.java || true

Repository: MatheMatrix/zstack

Length of output: 5207

添加错误处理防止脏配置导致告警流程中断。

当前代码直接解析时间字符串而无任何错误处理；虽然全局配置验证器可阻止新的非法值写入数据库，但无法防护：

  • 验证器添加前已存在的历史脏数据
  • 绕过 API 直接修改数据库的数据
  • 迁移脚本或手工编辑引入的非法数据

若这些脏数据在生产环境触发 LocalTime.parse()，会抛出运行时异常并中断告警处理流程，影响稳定性。建议在解析失败时记录 warn 并按窗口外处理（返回 false）。

建议修复 
     static boolean isNowInAutoRestartWindow(String configValue, LocalTime now) {
         if (configValue == null || configValue.trim().isEmpty()) {
             return true;
         }
-        String[] parts = configValue.trim().split("-");
-        LocalTime start = LocalTime.parse(parts[0]);
-        LocalTime end = LocalTime.parse(parts[1]);
+        String[] parts = configValue.trim().split("-");
+        if (parts.length != 2) {
+            logger.warn(String.format("invalid auto-restart window config[%s], treat as out-of-window", configValue));
+            return false;
+        }
+
+        final LocalTime start;
+        final LocalTime end;
+        try {
+            start = LocalTime.parse(parts[0].trim());
+            end = LocalTime.parse(parts[1].trim());
+        } catch (Exception e) {
+            logger.warn(String.format("invalid auto-restart window config[%s], treat as out-of-window", configValue), e);
+            return false;
+        }
+
         if (start.isBefore(end)) {
             return !now.isBefore(start) && now.isBefore(end);
         }
         return !now.isBefore(start) || now.isBefore(end);
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java` around lines 479
- 490, The method isNowInAutoRestartWindow currently calls LocalTime.parse on
configValue parts without validation, which can throw and break alert flows for
dirty config; update isNowInAutoRestartWindow to validate parts.length == 2 and
wrap parsing in a try-catch that catches
DateTimeParseException/RuntimeException, emit a logger.warn including the
offending configValue and exception, and return false (treat as outside window)
on any parse/validation failure so malformed values do not throw.


private void initLibvirtTlsCA() {
if (CoreGlobalProperty.UNIT_TEST_ON) {
return;
Expand Down Expand Up @@ -558,6 +580,31 @@ public void validateGlobalConfig(String category, String name, String oldValue,
}
}
});
KVMGlobalConfig.KVMAGENT_AUTO_RESTART_WINDOW.installValidateExtension(new GlobalConfigValidatorExtensionPoint() {
@Override
public void validateGlobalConfig(String category, String name, String oldValue, String value) throws GlobalConfigException {
if (value == null || value.trim().isEmpty()) {
return;
}
String[] parts = value.trim().split("-");
if (parts.length != 2 || !parts[0].matches("\\d{2}:\\d{2}") || !parts[1].matches("\\d{2}:\\d{2}")) {
throw new GlobalConfigException(String.format("%s must be in format HH:MM-HH:MM, but got %s",
KVMGlobalConfig.KVMAGENT_AUTO_RESTART_WINDOW.getCanonicalName(), value));
}
int sh = Integer.parseInt(parts[0].substring(0, 2));
int sm = Integer.parseInt(parts[0].substring(3, 5));
int eh = Integer.parseInt(parts[1].substring(0, 2));
int em = Integer.parseInt(parts[1].substring(3, 5));
if (sh > 23 || eh > 23 || sm > 59 || em > 59) {
throw new GlobalConfigException(String.format("%s has out-of-range hour/minute, but got %s",
KVMGlobalConfig.KVMAGENT_AUTO_RESTART_WINDOW.getCanonicalName(), value));
}
if (sh == eh && sm == em) {
throw new GlobalConfigException(String.format("%s start equals end, but got %s",
KVMGlobalConfig.KVMAGENT_AUTO_RESTART_WINDOW.getCanonicalName(), value));
}
}
});
ResourceConfig resourceConfig = rcf.getResourceConfig(KVMGlobalConfig.VM_CPU_HYPERVISOR_FEATURE.getIdentity());
resourceConfig.installValidatorExtension((resourceUuid, oldValue, newValue) -> {
if (Boolean.TRUE.toString().equals(newValue)) {
Expand Down
67 changes: 67 additions & 0 deletions plugin/kvm/src/test/java/org/zstack/kvm/TestKVMAutoRestartWindow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package org.zstack.kvm;

import org.junit.Test;

import java.time.LocalTime;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class TestKVMAutoRestartWindow {

private boolean inWindow(String configValue, String now) {
return KVMHostFactory.isNowInAutoRestartWindow(configValue, LocalTime.parse(now));
}

@Test
public void emptyOrNullValueAlwaysAllowed() {
assertTrue(inWindow("", "00:00"));
assertTrue(inWindow("", "12:00"));
assertTrue(inWindow("", "23:59"));
assertTrue(inWindow(null, "12:00"));
assertTrue(inWindow(" ", "12:00"));
}

@Test
public void normalWindowMembership() {
assertTrue(inWindow("02:00-04:00", "02:30"));
assertTrue(inWindow("02:00-04:00", "03:59"));
assertFalse(inWindow("02:00-04:00", "00:00"));
assertFalse(inWindow("02:00-04:00", "14:00"));
}

@Test
public void normalWindowBoundary() {
// start is inclusive
assertTrue(inWindow("02:00-04:00", "02:00"));
// end is exclusive
assertFalse(inWindow("02:00-04:00", "04:00"));
// just before end is included
assertTrue(inWindow("02:00-04:00", "03:59"));
}

@Test
public void crossMidnightWindowMembership() {
// after start, before midnight
assertTrue(inWindow("22:00-02:00", "23:30"));
assertTrue(inWindow("22:00-02:00", "22:00"));
// after midnight, before end
assertTrue(inWindow("22:00-02:00", "00:30"));
assertTrue(inWindow("22:00-02:00", "01:59"));
// outside (afternoon)
assertFalse(inWindow("22:00-02:00", "14:00"));
// boundary: end is exclusive
assertFalse(inWindow("22:00-02:00", "02:00"));
// just before start is excluded
assertFalse(inWindow("22:00-02:00", "21:59"));
}

@Test
public void wholeDayMinusOneMinute() {
// 00:00-23:59 covers everything except 23:59 itself
assertTrue(inWindow("00:00-23:59", "00:00"));
assertTrue(inWindow("00:00-23:59", "12:00"));
assertTrue(inWindow("00:00-23:59", "23:58"));
assertFalse(inWindow("00:00-23:59", "23:59"));
}
}
8 changes: 8 additions & 0 deletions test/src/test/resources/globalConfig/kvm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,12 @@
<defaultValue>10737418240</defaultValue>
<type>java.lang.Long</type>
</config>

<config>
<category>kvm</category>
<name>kvmagent.autorestart.window</name>
<description>Daily time window during which the automatic restart of zstack-kvmagent (triggered by physical memory hard limit alarm) is allowed. Format: HH:MM-HH:MM in 24-hour server local time, e.g. 02:00-04:00. Cross-midnight windows are supported, e.g. 22:00-02:00. Empty value means always allowed (no time restriction).</description>
<defaultValue>02:00-04:00</defaultValue>
<type>java.lang.String</type>
</config>
</globalConfig>