Add PATCH MachineConfig API Call

1. Make all machine_config parameters mandatory
   except CPU Template for a PUT request.
2. Replace current PUT MachineConfig with PATCH.
3. Add PUT that requires all fields.

Signed-off-by: YLyu <[email protected]>

Author: 0x00A5

CHANGELOG.md

@@ -10,6 +10,8 @@
 - New `devtool` command: `tag`. This creates a new git tag for the specified
   release number, based on the changelog contents.
 - New doc section about building with glibc.
+- New API call: `PATCH /machine-config/`, used to update VM configuration, 
+  before the microVM boots.
 
 ### Changed
 
@@ -21,6 +23,8 @@
   than `String`, `Array`, `Object` will return status code 400.
 - Improved multiple error messages.
 - Removed all kernel modules from the recommended kernel config.
+- `vcpu_count`, `mem_size_mib` and `ht_enabled` have been changed to be mandatory
+  for `PUT` requests on `/machine-config/`.
 
 ### Fixed
 

api_server/src/http_service.rs

@@ -305,6 +305,20 @@ fn parse_machine_config_req<'a>(
                     Error::Generic(StatusCode::BadRequest, s)
                 })?)
         }
+
+        0 if method == Method::Patch => {
+            METRICS.patch_api_requests.machine_cfg_count.inc();
+            Ok(serde_json::from_slice::<VmConfig>(body)
+                .map_err(|e| {
+                    METRICS.patch_api_requests.machine_cfg_fails.inc();
+                    Error::SerdeJson(e)
+                })?
+                .into_parsed_request(None, method)
+                .map_err(|s| {
+                    METRICS.patch_api_requests.machine_cfg_fails.inc();
+                    Error::Generic(StatusCode::BadRequest, s)
+                })?)
+        }
         _ => Err(Error::InvalidPathMethod(path, method)),
     }
 }
@@ -1158,6 +1172,26 @@ mod tests {
             _ => assert!(false),
         }
 
+        // PATCH
+        let vm_config = VmConfig {
+            vcpu_count: Some(32),
+            mem_size_mib: None,
+            ht_enabled: None,
+            cpu_template: None,
+        };
+        let body = r#"{
+            "vcpu_count": 32
+        }"#;
+        match vm_config.into_parsed_request(None, Method::Patch) {
+            Ok(parsed_req) => {
+                match parse_machine_config_req(&path, Method::Patch, &Chunk::from(body)) {
+                    Ok(other_parsed_req) => assert!(parsed_req.eq(&other_parsed_req)),
+                    _ => assert!(false),
+                }
+            }
+            _ => assert!(false),
+        }
+
         // Error cases
         // Error Case: Invalid payload (cannot deserialize the body into a VmConfig object).
         assert!(
@@ -1168,9 +1202,9 @@ mod tests {
         // Error Case: Invalid payload (payload is empty).
         let expected_err = Err(Error::Generic(
             StatusCode::BadRequest,
-            String::from("Empty request."),
+            String::from("Empty PATCH request."),
         ));
-        assert!(parse_machine_config_req(path, Method::Put, &Chunk::from("{}")) == expected_err);
+        assert!(parse_machine_config_req(path, Method::Patch, &Chunk::from("{}")) == expected_err);
 
         // Error Case: cpu count exceeds limitation
         let json = "{
@@ -1185,6 +1219,19 @@ mod tests {
         } else {
             assert!(false);
         }
+
+        // Error Case: PUT request with missing parameter
+        let json = "{
+                \"mem_size_mib\": 1025,
+                \"ht_enabled\": true,
+                \"cpu_template\": \"T2\"
+              }";
+        let body: Chunk = Chunk::from(json);
+        let expected_err = Err(Error::Generic(
+            StatusCode::BadRequest,
+            String::from("Missing mandatory fields."),
+        ));
+        assert!(parse_machine_config_req(path, Method::Put, &body) == expected_err);
     }
 
     #[test]

api_server/src/request/machine_configuration.rs

@@ -42,13 +42,25 @@ impl IntoParsedRequest for VmConfig {
                 VmmAction::GetVmConfiguration(sender),
                 receiver,
             )),
-            Method::Put => {
+            Method::Patch => {
                 if self.vcpu_count.is_none()
                     && self.mem_size_mib.is_none()
                     && self.cpu_template.is_none()
                     && self.ht_enabled.is_none()
                 {
-                    return Err(String::from("Empty request."));
+                    return Err(String::from("Empty PATCH request."));
+                }
+                Ok(ParsedRequest::Sync(
+                    VmmAction::SetVmConfiguration(self, sender),
+                    receiver,
+                ))
+            }
+            Method::Put => {
+                if self.vcpu_count.is_none()
+                    || self.mem_size_mib.is_none()
+                    || self.ht_enabled.is_none()
+                {
+                    return Err(String::from("Missing mandatory fields."));
                 }
                 Ok(ParsedRequest::Sync(
                     VmmAction::SetVmConfiguration(self, sender),
@@ -81,6 +93,7 @@ mod tests {
                 VmmAction::SetVmConfiguration(body, sender),
                 receiver
             ))));
+
         let uninitialized = VmConfig {
             vcpu_count: None,
             mem_size_mib: None,
@@ -91,14 +104,23 @@ mod tests {
             .clone()
             .into_parsed_request(None, Method::Get)
             .is_ok());
+
+        // Empty PATCH
         assert!(uninitialized
             .clone()
             .into_parsed_request(None, Method::Patch)
             .is_err());
 
-        match uninitialized.into_parsed_request(None, Method::Put) {
+        // Incomplete PUT payload
+        let body = VmConfig {
+            vcpu_count: Some(8),
+            mem_size_mib: Some(1024),
+            ht_enabled: None,
+            cpu_template: Some(CpuFeaturesTemplate::T2),
+        };
+        match body.into_parsed_request(None, Method::Put) {
             Ok(_) => assert!(false),
-            Err(e) => assert_eq!(e, String::from("Empty request.")),
+            Err(e) => assert_eq!(e, String::from("Missing mandatory fields.")),
         };
     }
 }

api_server/swagger/firecracker.yaml

@@ -215,6 +215,30 @@ paths:
           schema:
             $ref: "#/definitions/Error"
 
+    patch:
+      summary: Partially updates the Machine Configuration of the VM.
+      description:
+        Partially updates the Virtual Machine Configuration with the specified input.
+        If any of the parameters has an incorrect value, the whole update fails.
+      operationId: patchMachineConfiguration
+      parameters:
+      - name: body
+        in: body
+        description: A subset of Machine Configuration Parameters
+        schema:
+          $ref: "#/definitions/MachineConfiguration"
+      responses:
+        204:
+          description: Machine Configuration created/updated
+        400:
+          description: Machine Configuration cannot be updated due to bad input
+          schema:
+            $ref: "#/definitions/Error"
+        default:
+          description: Internal server error
+          schema:
+            $ref: "#/definitions/Error"
+
   /mmds:
     put:
       summary: Creates a MMDS (Microvm Metadata Service) data store.

logger/src/metrics.rs

@@ -186,6 +186,10 @@ pub struct PatchRequestsMetrics {
     pub network_count: SharedMetric,
     /// Number of failures in PATCHing a net device.
     pub network_fails: SharedMetric,
+    /// Number of PATCHs for configuring the machine.
+    pub machine_cfg_count: SharedMetric,
+    /// Number of failures in configuring the machine.
+    pub machine_cfg_fails: SharedMetric,
 }
 
 /// Block Device associated metrics.

tests/integration_tests/functional/test_api.py

@@ -254,12 +254,20 @@ def test_api_put_update_post_boot(test_microvm_with_api):
     assert "The update operation is not allowed after boot" in response.text
 
     # Valid updates to the machine configuration are not allowed after boot.
-    response = test_microvm.machine_cfg.put(
+    response = test_microvm.machine_cfg.patch(
         vcpu_count=4
     )
     assert test_microvm.api_session.is_status_bad_request(response.status_code)
     assert "The update operation is not allowed after boot" in response.text
 
+    response = test_microvm.machine_cfg.put(
+        vcpu_count=4,
+        ht_enabled=False,
+        mem_size_mib=128
+    )
+    assert test_microvm.api_session.is_status_bad_request(response.status_code)
+    assert "The update operation is not allowed after boot" in response.text
+
     # Network interface update is not allowed after boot.
     response = test_microvm.network.put(
         iface_id='1',
@@ -472,10 +480,11 @@ def test_api_patch_pre_boot(test_microvm_with_api):
     assert test_microvm.api_session.is_status_bad_request(response.status_code)
     assert "Invalid request method" in response.text
 
-    # Partial updates to the machine configuration are not allowed.
+    # Partial updates to the machine configuration are allowed before boot.
     response = test_microvm.machine_cfg.patch(vcpu_count=4)
-    assert test_microvm.api_session.is_status_bad_request(response.status_code)
-    assert "Invalid request method" in response.text
+    assert test_microvm.api_session.is_status_no_content(response.status_code)
+    response_json = test_microvm.machine_cfg.get().json()
+    assert response_json['vcpu_count'] == 4
 
     # Partial updates to the logger configuration are not allowed.
     response = test_microvm.logger.patch(level='Error')
@@ -534,10 +543,10 @@ def test_api_patch_post_boot(test_microvm_with_api):
     assert test_microvm.api_session.is_status_bad_request(response.status_code)
     assert "Invalid request method" in response.text
 
-    # Partial updates to the machine configuration are not allowed.
+    # Partial updates to the machine configuration are not allowed after boot.
     response = test_microvm.machine_cfg.patch(vcpu_count=4)
     assert test_microvm.api_session.is_status_bad_request(response.status_code)
-    assert "Invalid request method" in response.text
+    assert "The update operation is not allowed after boot." in response.text
 
     # Partial updates to the logger configuration are not allowed.
     response = test_microvm.logger.patch(level='Error')

tests/integration_tests/functional/test_logging.py

@@ -191,11 +191,23 @@ def test_api_requests_logs(test_microvm_with_api):
 
     expected_log_strings = []
 
-    # Check that a Put request on /machine-config is logged.
-    response = microvm.machine_cfg.put(vcpu_count=4)
+    # Check that a Patch request on /machine-config is logged.
+    response = microvm.machine_cfg.patch(vcpu_count=4)
     assert microvm.api_session.is_status_no_content(response.status_code)
     # We are not interested in the actual body. Just check that the log
     # message also has the string "body" in it.
+    expected_log_strings.append(
+        "The API server received a synchronous Patch request "
+        "on \"/machine-config\" with body"
+    )
+
+    # Check that a Put request on /machine-config is logged.
+    response = microvm.machine_cfg.put(
+        vcpu_count=4,
+        ht_enabled=False,
+        mem_size_mib=128
+    )
+    assert microvm.api_session.is_status_no_content(response.status_code)
     expected_log_strings.append(
         "The API server received a synchronous Put request "
         "on \"/machine-config\" with body"