Skip to content

Commit

Permalink
[nexus] allow reconfiguring auto-restart policies
Browse files Browse the repository at this point in the history
This commit extends the `instance-reconfigure` API endpoint added in
#6585 to also allow setting instance auto-restart policies (as added in
#6503).

I've also added the actual auto-restart policy to the external API
instance view, along with the boolean `auto_restart_enabled` added in
#6503. This way, it's possible to change just the boot disk by providing
the current auto-restart policy in an instance POST.
  • Loading branch information
hawkw committed Oct 2, 2024
1 parent 5ef07cf commit d0864a0
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 12 deletions.
14 changes: 14 additions & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,20 @@ pub struct InstanceAutoRestartStatus {
#[serde(rename = "auto_restart_enabled")]
pub enabled: bool,

/// The auto-restart policy configured for this instance, or `None` if no
/// explicit policy is configured.
///
/// If this is not present, then this instance uses the default auto-restart
/// policy, which may or may not allow it to be restarted. The
/// `auto_restart_enabled` field indicates whether the instance will be
/// automatically restarted.
//
// Rename this field, as the struct is `#[serde(flatten)]`ed into the
// `Instance` type, and we would like the field to be prefixed with
// `auto_restart`.
#[serde(rename = "auto_restart_policy")]
pub policy: Option<InstanceAutoRestartPolicy>,

/// The time at which the auto-restart cooldown period for this instance
/// completes, permitting it to be automatically restarted again. If the
/// instance enters the `Failed` state, it will not be restarted until after
Expand Down
21 changes: 14 additions & 7 deletions nexus/db-queries/src/db/datastore/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,23 @@ impl From<InstanceAndActiveVmm> for external::Instance {
},
);

let policy = value
.instance
.auto_restart
.policy
.unwrap_or(InstanceAutoRestart::DEFAULT_POLICY);
let enabled = match policy {
let policy = value.instance.auto_restart.policy;
// The active policy for this instance --- either its configured
// policy or the default. We report the configured policy as the
// instance's policy, but we must use this to determine whether it
// will be auto-restarted, since it may have no configured policy.
let active_policy =
policy.unwrap_or(InstanceAutoRestart::DEFAULT_POLICY);

let enabled = match active_policy {
InstanceAutoRestartPolicy::Never => false,
InstanceAutoRestartPolicy::BestEffort => true,
};
external::InstanceAutoRestartStatus { enabled, cooldown_expiration }
external::InstanceAutoRestartStatus {
enabled,
policy: policy.map(Into::into),
cooldown_expiration,
}
};

Self {
Expand Down
4 changes: 3 additions & 1 deletion nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,9 @@ impl super::Nexus {
None => None,
};

let update = InstanceUpdate { boot_disk_id };
let auto_restart_policy = params.auto_restart_policy.map(Into::into);

let update = InstanceUpdate { boot_disk_id, auto_restart_policy };
self.datastore()
.instance_reconfigure(opctx, &authz_instance, update)
.await
Expand Down
5 changes: 4 additions & 1 deletion nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,10 @@ pub static DEMO_INSTANCE_CREATE: Lazy<params::InstanceCreate> =
auto_restart_policy: Default::default(),
});
pub static DEMO_INSTANCE_UPDATE: Lazy<params::InstanceUpdate> =
Lazy::new(|| params::InstanceUpdate { boot_disk: None });
Lazy::new(|| params::InstanceUpdate {
boot_disk: None,
auto_restart_policy: None,
});

// The instance needs a network interface, too.
pub static DEMO_INSTANCE_NIC_NAME: Lazy<Name> =
Expand Down
18 changes: 15 additions & 3 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4161,7 +4161,10 @@ async fn test_cannot_detach_boot_disk(cptestctx: &ControlPlaneTestContext) {

let builder =
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate { boot_disk: None }))
.body(Some(&params::InstanceUpdate {
boot_disk: None,
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::OK));
let response = NexusRequest::new(builder)
.authn_as(AuthnMode::PrivilegedUser)
Expand Down Expand Up @@ -4238,7 +4241,10 @@ async fn test_updating_running_instance_is_conflict(

let builder =
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate { boot_disk: None }))
.body(Some(&params::InstanceUpdate {
boot_disk: None,
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::CONFLICT));

let response = NexusRequest::new(builder)
Expand All @@ -4264,7 +4270,10 @@ async fn test_updating_missing_instance_is_not_found(

let builder =
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate { boot_disk: None }))
.body(Some(&params::InstanceUpdate {
boot_disk: None,
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::NOT_FOUND));

let response = NexusRequest::new(builder)
Expand Down Expand Up @@ -4353,6 +4362,7 @@ async fn test_boot_disk_can_be_changed(cptestctx: &ControlPlaneTestContext) {
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate {
boot_disk: Some(disks[1].identity.id.into()),
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::OK));

Expand Down Expand Up @@ -4424,6 +4434,7 @@ async fn test_boot_disk_must_be_attached(cptestctx: &ControlPlaneTestContext) {
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate {
boot_disk: Some(disks[0].identity.id.into()),
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::CONFLICT));
let response = NexusRequest::new(builder)
Expand Down Expand Up @@ -4459,6 +4470,7 @@ async fn test_boot_disk_must_be_attached(cptestctx: &ControlPlaneTestContext) {
RequestBuilder::new(client, http::Method::PUT, &url_instance_update)
.body(Some(&params::InstanceUpdate {
boot_disk: Some(disks[0].identity.id.into()),
auto_restart_policy: None,
}))
.expect_status(Some(http::StatusCode::OK));
let response = NexusRequest::new(builder)
Expand Down
28 changes: 28 additions & 0 deletions openapi/nexus-internal.json
Original file line number Diff line number Diff line change
Expand Up @@ -3382,6 +3382,15 @@
"description": "`true` if this instance's auto-restart policy will permit the control plane to automatically restart it if it enters the `Failed` state.",
"type": "boolean"
},
"auto_restart_policy": {
"nullable": true,
"description": "The auto-restart policy configured for this instance, or `None` if no explicit policy is configured.\n\nIf this is not present, then this instance uses the default auto-restart policy, which may or may not allow it to be restarted. The `auto_restart_enabled` field indicates whether the instance will be automatically restarted.",
"allOf": [
{
"$ref": "#/components/schemas/InstanceAutoRestartPolicy"
}
]
},
"boot_disk_id": {
"nullable": true,
"description": "the ID of the disk used to boot this Instance, if a specific one is assigned.",
Expand Down Expand Up @@ -3469,6 +3478,25 @@
"time_run_state_updated"
]
},
"InstanceAutoRestartPolicy": {
"description": "A policy determining when an instance should be automatically restarted by the control plane.",
"oneOf": [
{
"description": "The instance should not be automatically restarted by the control plane if it fails.",
"type": "string",
"enum": [
"never"
]
},
{
"description": "If this instance is running and unexpectedly fails (e.g. due to a host software crash or unexpected host reboot), the control plane will make a best-effort attempt to restart it. The control plane may choose not to restart the instance to preserve the overall availability of the system.",
"type": "string",
"enum": [
"best_effort"
]
}
]
},
"InstanceCpuCount": {
"description": "The number of CPUs in an Instance",
"type": "integer",
Expand Down
18 changes: 18 additions & 0 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -15156,6 +15156,15 @@
"description": "`true` if this instance's auto-restart policy will permit the control plane to automatically restart it if it enters the `Failed` state.",
"type": "boolean"
},
"auto_restart_policy": {
"nullable": true,
"description": "The auto-restart policy configured for this instance, or `None` if no explicit policy is configured.\n\nIf this is not present, then this instance uses the default auto-restart policy, which may or may not allow it to be restarted. The `auto_restart_enabled` field indicates whether the instance will be automatically restarted.",
"allOf": [
{
"$ref": "#/components/schemas/InstanceAutoRestartPolicy"
}
]
},
"boot_disk_id": {
"nullable": true,
"description": "the ID of the disk used to boot this Instance, if a specific one is assigned.",
Expand Down Expand Up @@ -15807,6 +15816,15 @@
"description": "Parameters of an `Instance` that can be reconfigured after creation.",
"type": "object",
"properties": {
"auto_restart_policy": {
"nullable": true,
"description": "The auto-restart policy for this instance.\n\nIf not provided, unset the instance's auto-restart policy.",
"allOf": [
{
"$ref": "#/components/schemas/InstanceAutoRestartPolicy"
}
]
},
"boot_disk": {
"nullable": true,
"description": "Name or ID of the disk the instance should be instructed to boot from.\n\nIf not provided, unset the instance's boot disk.",
Expand Down

0 comments on commit d0864a0

Please sign in to comment.