Add guard rails against manipulating mirrors

Snapshot mirrors should normally be promoted using "mirror promote", and
not started manually. This adds guard rails against that to the "start",
"stop", and "disable" state commands to prevent changing mirror states
without an explicit "--force" option.
This commit is contained in:
Joshua Boniface 2024-10-08 23:51:48 -04:00
parent 3ea7421f09
commit 1d90b066bc
5 changed files with 59 additions and 21 deletions

View File

@ -2645,7 +2645,7 @@ class API_VM_State(Resource):
- in: query - in: query
name: force name: force
type: boolean type: boolean
description: Whether to force stop instead of shutdown VM during disable description: For "disable", force stop instead of shutdown and/or force mirror state; for "start" or "stop", force mirror state.
- in: query - in: query
name: wait name: wait
type: boolean type: boolean
@ -2667,15 +2667,15 @@ class API_VM_State(Resource):
wait = bool(strtobool(reqargs.get("wait", "false"))) wait = bool(strtobool(reqargs.get("wait", "false")))
if state == "start": if state == "start":
return api_helper.vm_start(vm) return api_helper.vm_start(vm, force=force)
if state == "shutdown": if state == "shutdown":
return api_helper.vm_shutdown(vm, wait) return api_helper.vm_shutdown(vm, wait)
if state == "stop": if state == "stop":
return api_helper.vm_stop(vm) return api_helper.vm_stop(vm, force=force)
if state == "restart": if state == "restart":
return api_helper.vm_restart(vm, wait) return api_helper.vm_restart(vm, wait)
if state == "disable": if state == "disable":
return api_helper.vm_disable(vm, force) return api_helper.vm_disable(vm, force=force)
abort(400) abort(400)

View File

@ -1153,11 +1153,11 @@ def vm_remove(zkhandler, name):
@ZKConnection(config) @ZKConnection(config)
def vm_start(zkhandler, name): def vm_start(zkhandler, name, force=False):
""" """
Start a VM in the PVC cluster. Start a VM in the PVC cluster.
""" """
retflag, retdata = pvc_vm.start_vm(zkhandler, name) retflag, retdata = pvc_vm.start_vm(zkhandler, name, force=force)
if retflag: if retflag:
retcode = 200 retcode = 200
@ -1201,11 +1201,11 @@ def vm_shutdown(zkhandler, name, wait):
@ZKConnection(config) @ZKConnection(config)
def vm_stop(zkhandler, name): def vm_stop(zkhandler, name, force=False):
""" """
Forcibly stop a VM in the PVC cluster. Forcibly stop a VM in the PVC cluster.
""" """
retflag, retdata = pvc_vm.stop_vm(zkhandler, name) retflag, retdata = pvc_vm.stop_vm(zkhandler, name, force=force)
if retflag: if retflag:
retcode = 200 retcode = 200
@ -1219,7 +1219,7 @@ def vm_stop(zkhandler, name):
@ZKConnection(config) @ZKConnection(config)
def vm_disable(zkhandler, name, force=False): def vm_disable(zkhandler, name, force=False):
""" """
Disable (shutdown or force stop if required)a VM in the PVC cluster. Disable (shutdown or force stop if required) a VM in the PVC cluster.
""" """
retflag, retdata = pvc_vm.disable_vm(zkhandler, name, force=force) retflag, retdata = pvc_vm.disable_vm(zkhandler, name, force=force)

View File

@ -1517,12 +1517,21 @@ def cli_vm_remove(domain):
@click.command(name="start", short_help="Start up a defined virtual machine.") @click.command(name="start", short_help="Start up a defined virtual machine.")
@connection_req @connection_req
@click.argument("domain") @click.argument("domain")
def cli_vm_start(domain): @click.option(
"--force",
"force_flag",
is_flag=True,
default=False,
help="Force a snapshot mirror state change.",
)
def cli_vm_start(domain, force_flag):
""" """
Start virtual machine DOMAIN on its configured node. DOMAIN may be a UUID or name. Start virtual machine DOMAIN on its configured node. DOMAIN may be a UUID or name.
If the VM is a snapshot mirror, "--force" allows a manual state change to the mirror.
""" """
retcode, retmsg = pvc.lib.vm.vm_state(CLI_CONFIG, domain, "start") retcode, retmsg = pvc.lib.vm.vm_state(CLI_CONFIG, domain, "start", force=force_flag)
finish(retcode, retmsg) finish(retcode, retmsg)
@ -1582,13 +1591,22 @@ def cli_vm_shutdown(domain, wait):
@click.command(name="stop", short_help="Forcibly halt a running virtual machine.") @click.command(name="stop", short_help="Forcibly halt a running virtual machine.")
@connection_req @connection_req
@click.argument("domain") @click.argument("domain")
@click.option(
"--force",
"force_flag",
is_flag=True,
default=False,
help="Force a snapshot mirror state change.",
)
@confirm_opt("Forcibly stop virtual machine {domain}") @confirm_opt("Forcibly stop virtual machine {domain}")
def cli_vm_stop(domain): def cli_vm_stop(domain, force_flag):
""" """
Forcibly halt (destroy) running virtual machine DOMAIN. DOMAIN may be a UUID or name. Forcibly halt (destroy) running virtual machine DOMAIN. DOMAIN may be a UUID or name.
If the VM is a snapshot mirror, "--force" allows a manual state change to the mirror.
""" """
retcode, retmsg = pvc.lib.vm.vm_state(CLI_CONFIG, domain, "stop") retcode, retmsg = pvc.lib.vm.vm_state(CLI_CONFIG, domain, "stop", force=force_flag)
finish(retcode, retmsg) finish(retcode, retmsg)
@ -1603,14 +1621,14 @@ def cli_vm_stop(domain):
"force_flag", "force_flag",
is_flag=True, is_flag=True,
default=False, default=False,
help="Forcibly stop the VM instead of waiting for shutdown.", help="Forcibly stop VM without shutdown and/or force a snapshot mirror state change.",
) )
@confirm_opt("Shut down and disable virtual machine {domain}") @confirm_opt("Shut down and disable virtual machine {domain}")
def cli_vm_disable(domain, force_flag): def cli_vm_disable(domain, force_flag):
""" """
Shut down virtual machine DOMAIN and mark it as disabled. DOMAIN may be a UUID or name. Shut down virtual machine DOMAIN and mark it as disabled. DOMAIN may be a UUID or name.
Disabled VMs will not be counted towards a degraded cluster health status, unlike stopped VMs. Use this option for a VM that will remain off for an extended period. If "--force" is specified, and the VM is running, it will be forcibly stopped instead of waiting for a graceful ACPI shutdown. If the VM is a snapshot mirror, "--force" allows a manual state change to the mirror.
""" """
retcode, retmsg = pvc.lib.vm.vm_state( retcode, retmsg = pvc.lib.vm.vm_state(

View File

@ -383,8 +383,8 @@ def vm_state(config, vm, target_state, force=False, wait=False):
""" """
params = { params = {
"state": target_state, "state": target_state,
"force": str(force).lower(), "force": force,
"wait": str(wait).lower(), "wait": wait,
} }
response = call_api(config, "post", "/vm/{vm}/state".format(vm=vm), params=params) response = call_api(config, "post", "/vm/{vm}/state".format(vm=vm), params=params)

View File

@ -696,12 +696,19 @@ def remove_vm(zkhandler, domain):
return True, 'Removed VM "{}" and its disks from the cluster.'.format(domain) return True, 'Removed VM "{}" and its disks from the cluster.'.format(domain)
def start_vm(zkhandler, domain): def start_vm(zkhandler, domain, force=False):
# Validate that VM exists in cluster # Validate that VM exists in cluster
dom_uuid = getDomainUUID(zkhandler, domain) dom_uuid = getDomainUUID(zkhandler, domain)
if not dom_uuid: if not dom_uuid:
return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain) return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain)
current_vm_state = zkhandler.read(("domain.state", dom_uuid))
if current_vm_state in ["mirror"] and not force:
return (
False,
'ERROR: VM "{}" is a snapshot mirror and start not forced!'.format(domain),
)
# Set the VM to start # Set the VM to start
change_state(zkhandler, dom_uuid, "start") change_state(zkhandler, dom_uuid, "start")
@ -756,12 +763,18 @@ def shutdown_vm(zkhandler, domain, wait=False):
return True, retmsg return True, retmsg
def stop_vm(zkhandler, domain): def stop_vm(zkhandler, domain, force=False):
# Validate that VM exists in cluster # Validate that VM exists in cluster
dom_uuid = getDomainUUID(zkhandler, domain) dom_uuid = getDomainUUID(zkhandler, domain)
if not dom_uuid: if not dom_uuid:
return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain) return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain)
current_vm_state = zkhandler.read(("domain.state", dom_uuid))
if current_vm_state in ["mirror"] and not force:
return False, 'ERROR: VM "{}" is a snapshot mirror and stop not forced!'.format(
domain
)
# Set the VM to stop # Set the VM to stop
change_state(zkhandler, dom_uuid, "stop") change_state(zkhandler, dom_uuid, "stop")
@ -775,8 +788,15 @@ def disable_vm(zkhandler, domain, force=False):
return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain) return False, 'ERROR: Could not find VM "{}" in the cluster!'.format(domain)
# Get state and perform a shutdown/stop if VM is online # Get state and perform a shutdown/stop if VM is online
current_state = zkhandler.read(("domain.state", dom_uuid)) current_vm_state = zkhandler.read(("domain.state", dom_uuid))
if current_state in ["start"]: if current_vm_state in ["mirror"] and not force:
return (
False,
'ERROR: VM "{}" is a snapshot mirror and disable not forced!'.format(
domain
),
)
elif current_vm_state in ["start"]:
if force: if force:
change_state(zkhandler, dom_uuid, "stop") change_state(zkhandler, dom_uuid, "stop")
# Wait for the command to be registered by the node # Wait for the command to be registered by the node