Skip to content

Commit

Permalink
vmm: Deprecate static CPU templates
Browse files Browse the repository at this point in the history
Deprecate the `cpu_template` field in the `/machine-config` API, which
is used to set static CPU templates. Users can use custom CPU templates
as an alternative.

To differentiate between explicitly specifying `None` and not specifying
anything, wrap the `cpu_template` of the `MachineConfig` with `Option`.
With this, it can notify the deprecation of only users using the field.

Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
  • Loading branch information
zulinx86 committed Sep 27, 2023
1 parent 915f56b commit 4ff2c99
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 13 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
- Added support for the /dev/userfaultfd device available on linux kernels >=
6.1. This is the default for creating UFFD handlers on these kernel versions.
If it is unavailable, Firecracker falls back to the userfaultfd syscall.
- Deprecated `cpu_template` field in `PUT` and `PATCH` request on `/machine-config`
API, which is used to set a static CPU template. Custom CPU templates added in
v1.4.0 are available as an improved iteration of the static CPU templates. For
more information about the transition from static CPU templates to custom CPU
templates, please refer [here](https://github.com/firecracker-microvm/firecracker/discussions/4135).

### Fixed

Expand Down
7 changes: 7 additions & 0 deletions docs/cpu_templates/cpu-templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ Firecracker supports two types of CPU templates:
- Custom CPU templates - users can create their own CPU templates in json
format and pass them to Firecracker

> **Note**
Staitc CPU templates are deprecated starting from v1.5.0 and will be removed in
accordance with our deprecation policy. Even after the removal, custom CPU
templates are available as an improved iteration of static CPU templates. For
more information about the transition from static CPU templates to custom CPU
templates, please refer [here](https://github.com/firecracker-microvm/firecracker/discussions/4135).

> **Note**
CPU templates for ARM (both static and custom) require the following patch
to be available in the host kernel: [Support writable CPU ID registers from userspace](https://lore.kernel.org/kvm/20230212215830.2975485-1-jingzhangos@google.com/#t).
Expand Down
109 changes: 100 additions & 9 deletions src/api_server/src/request/machine_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,25 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Err
err
})?;

// Check for the presence of deprecated `cpu_template` field.
let mut deprecation_message = None;
if config.cpu_template.is_some() {
// `cpu_template` field in request is deprecated.
METRICS.deprecated_api.deprecated_http_api_calls.inc();
deprecation_message = Some("PUT /machine-config: cpu_template field is deprecated.");
}

// Convert `MachineConfig` to `MachineConfigUpdate`.
let config_update = MachineConfigUpdate::from(config);

Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
config_update,
)))
// Construct the `ParsedRequest` object.
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
// If `cpu_template` was present, set the deprecation message in `parsing_info`.
if let Some(msg) = deprecation_message {
parsed_req.parsing_info().append_deprecation_message(msg);
}

Ok(parsed_req)
}

pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, Error> {
Expand All @@ -39,17 +53,30 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, E
return method_to_error(Method::Patch);
}

Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
config_update,
)))
// Check for the presence of deprecated `cpu_template` field.
let mut deprecation_message = None;
if config_update.cpu_template.is_some() {
// `cpu_template` field in request is deprecated.
METRICS.deprecated_api.deprecated_http_api_calls.inc();
deprecation_message = Some("PATCH /machine-config: cpu_template field is deprecated.");
}

// Construct the `ParsedRequest` object.
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
// If `cpu_template` was present, set the deprecation message in `parsing_info`.
if let Some(msg) = deprecation_message {
parsed_req.parsing_info().append_deprecation_message(msg);
}

Ok(parsed_req)
}

#[cfg(test)]
mod tests {
use vmm::cpu_config::templates::StaticCpuTemplate;

use super::*;
use crate::parsed_request::tests::vmm_action_from_request;
use crate::parsed_request::tests::{depr_action_from_req, vmm_action_from_request};

#[test]
fn test_parse_get_machine_config_request() {
Expand Down Expand Up @@ -79,6 +106,24 @@ mod tests {
"vcpu_count": 8,
"mem_size_mib": 1024
}"#;
let expected_config = MachineConfigUpdate {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(false),
cpu_template: None,
track_dirty_pages: Some(false),
};

match vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()) {
VmmAction::UpdateVmConfiguration(config) => assert_eq!(config, expected_config),
_ => panic!("Test failed."),
}

let body = r#"{
"vcpu_count": 8,
"mem_size_mib": 1024,
"cpu_template": "None"
}"#;
let expected_config = MachineConfigUpdate {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
Expand All @@ -102,7 +147,7 @@ mod tests {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(false),
cpu_template: Some(StaticCpuTemplate::None),
cpu_template: None,
track_dirty_pages: Some(true),
};

Expand Down Expand Up @@ -155,7 +200,7 @@ mod tests {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(true),
cpu_template: Some(StaticCpuTemplate::None),
cpu_template: None,
track_dirty_pages: Some(true),
};

Expand Down Expand Up @@ -209,4 +254,50 @@ mod tests {
let body = r#"{}"#;
assert!(parse_patch_machine_config(&Body::new(body)).is_err());
}

#[test]
fn test_depr_cpu_template_in_put_req() {
// Test that the deprecation message is shown when `cpu_template` is specified.
let body = r#"{
"vcpu_count": 8,
"mem_size_mib": 1024,
"cpu_template": "None"
}"#;
depr_action_from_req(
parse_put_machine_config(&Body::new(body)).unwrap(),
Some("PUT /machine-config: cpu_template field is deprecated.".to_string()),
);

// Test that the deprecation message is not shown when `cpu_template` is not specified.
let body = r#"{
"vcpu_count": 8,
"mem_size_mib": 1024
}"#;
let (_, mut parsing_info) = parse_put_machine_config(&Body::new(body))
.unwrap()
.into_parts();
assert!(parsing_info.take_deprecation_message().is_none());
}

#[test]
fn test_depr_cpu_template_in_patch_req() {
// Test that the deprecation message is shown when `cpu_template` is specified.
let body = r#"{
"vcpu_count": 8,
"cpu_template": "None"
}"#;
depr_action_from_req(
parse_patch_machine_config(&Body::new(body)).unwrap(),
Some("PATCH /machine-config: cpu_template field is deprecated.".to_string()),
);

// Test that the deprecation message is not shown when `cpu_template` is not specified.
let body = r#"{
"vcpu_count": 8
}"#;
let (_, mut parsing_info) = parse_patch_machine_config(&Body::new(body))
.unwrap()
.into_parts();
assert!(parsing_info.take_deprecation_message().is_none());
}
}
2 changes: 2 additions & 0 deletions src/api_server/swagger/firecracker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,8 @@ definitions:
description:
The CPU Template defines a set of flags to be disabled from the microvm so that
the features exposed to the guest are the same as in the selected instance type.
This parameter has been deprecated and it will be removed in accordance with our
policy.
enum:
- C3
- T2
Expand Down
9 changes: 9 additions & 0 deletions src/vmm/src/cpu_config/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ impl From<&Option<CpuTemplateType>> for StaticCpuTemplate {
}
}

impl From<&CpuTemplateType> for StaticCpuTemplate {
fn from(value: &CpuTemplateType) -> Self {
match value {
CpuTemplateType::Static(template) => *template,
CpuTemplateType::Custom(_) => StaticCpuTemplate::None,

Check warning on line 79 in src/vmm/src/cpu_config/templates.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/cpu_config/templates.rs#L79

Added line #L79 was not covered by tests
}
}
}

impl<'a> TryFrom<&'a [u8]> for CustomCpuTemplate {
type Error = serde_json::Error;

Expand Down
8 changes: 4 additions & 4 deletions src/vmm/src/vmm_config/machine_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ pub struct MachineConfig {
#[serde(default, deserialize_with = "deserialize_smt")]
pub smt: bool,
/// A CPU template that it is used to filter the CPU features exposed to the guest.
#[serde(default, skip_serializing_if = "StaticCpuTemplate::is_none")]
pub cpu_template: StaticCpuTemplate,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cpu_template: Option<StaticCpuTemplate>,
/// Enables or disables dirty page tracking. Enabling allows incremental snapshots.
#[serde(default)]
pub track_dirty_pages: bool,
Expand Down Expand Up @@ -122,7 +122,7 @@ impl From<MachineConfig> for MachineConfigUpdate {
vcpu_count: Some(cfg.vcpu_count),
mem_size_mib: Some(cfg.mem_size_mib),
smt: Some(cfg.smt),
cpu_template: Some(cfg.cpu_template),
cpu_template: cfg.cpu_template,
track_dirty_pages: Some(cfg.track_dirty_pages),
}
}
Expand Down Expand Up @@ -212,7 +212,7 @@ impl From<&VmConfig> for MachineConfig {
vcpu_count: value.vcpu_count,
mem_size_mib: value.mem_size_mib,
smt: value.smt,
cpu_template: (&value.cpu_template).into(),
cpu_template: value.cpu_template.as_ref().map(|template| template.into()),
track_dirty_pages: value.track_dirty_pages,
}
}
Expand Down
31 changes: 31 additions & 0 deletions tests/integration_tests/functional/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,37 @@ def test_api_machine_config(test_microvm_with_api):
assert json["machine-config"]["smt"] is False


def test_negative_machine_config_api(test_microvm_with_api):
"""
Test the deprecated `cpu_template` field in PUT and PATCH requests on
`/machine-config` API is handled correctly.
When using the `cpu_template` field (even if the value is "None"), the HTTP
response header should have "Deprecation: true".
"""
test_microvm = test_microvm_with_api
test_microvm.spawn()

# Use `cpu_template` field in PUT /machine-config
response = test_microvm.api.machine_config.put(
vcpu_count=2,
mem_size_mib=256,
cpu_template="None",
)
assert response.headers["deprecation"]
assert (
"PUT /machine-config: cpu_template field is deprecated."
in test_microvm.log_data
)

# Use `cpu_template` field in PATCH /machine-config
response = test_microvm.api.machine_config.patch(cpu_template="None")
assert (
"PATCH /machine-config: cpu_template field is deprecated."
in test_microvm.log_data
)


@nonci_on_arm
def test_api_cpu_config(test_microvm_with_api, custom_cpu_template):
"""
Expand Down

0 comments on commit 4ff2c99

Please sign in to comment.