api_server: restructured the Boot Source

We don't plan to add a network boot source, so we can simplify
the BootSource request body:
* removed the boot source id as we only support one boot source
* removed the boot source type (only local image is supported)
* removed the local image configuration.
Now the boot source has 2 fields: kernel_image_path and boot_args.
The kernel_image_path is required.

Signed-off-by: Andreea Florescu <[email protected]>

Author: andreeaflorescu

CHANGELOG.md

@@ -11,6 +11,8 @@
 
 ### Changed
 
+- The boot source is specified only with the `kernel_image_path` and
+  the optional parameter `boot_args`. All other fields are removed.
 - The `path_on_host` property in the drive specification is now marked as
   *mandatory*.
 - PATCH drive only allows patching/changing the `path_on_host` property.

README.md

@@ -179,12 +179,8 @@ curl --unix-socket ${socket} -i  \
      -H "accept: application/json" \
      -H "Content-Type: application/json" \
      -d "{
-           \"boot_source_id\": \"linux_kernel\",
-           \"source_type\": \"LocalImage\",
-           \"local_image\":
-                {
-                    \"kernel_image_path\": \"${kernel_path}\"
-                }
+            \"kernel_image_path\": \"${kernel_path}\",
+            \"boot_args\": \"reboot=k panic=1 pci=off nomodules console=ttyS0\"
         }"
 ```
 

api_server/src/http_service.rs

@@ -14,7 +14,7 @@ use data_model::mmds::Mmds;
 use data_model::vm::{BlockDeviceConfig, MachineConfiguration};
 use logger::{Metric, METRICS};
 use request::actions::ActionBody;
-use request::boot_source::BootSourceBody;
+use request::boot_source::BootSourceConfig;
 use request::drive::PatchDrivePayload;
 use request::instance_info::InstanceInfo;
 use request::logger::APILoggerDescription;
@@ -154,7 +154,7 @@ fn parse_boot_source_req<'a>(
 
         0 if method == Method::Put => {
             METRICS.put_api_requests.boot_source_count.inc();
-            Ok(serde_json::from_slice::<BootSourceBody>(body)
+            Ok(serde_json::from_slice::<BootSourceConfig>(body)
                 .map_err(|e| {
                     METRICS.put_api_requests.boot_source_fails.inc();
                     Error::SerdeJson(e)
@@ -782,12 +782,10 @@ mod tests {
     fn test_parse_boot_source_req() {
         let path = "/foo";
         let path_tokens: Vec<&str> = path[1..].split_terminator('/').collect();
-        let json = "{
-                \"boot_source_id\": \"bar\",
-                \"source_type\": \"LocalImage\",
-                \"local_image\": {\"kernel_image_path\": \"/foo/bar\"},
-                \"boot_args\": \"baz\"
-              }";
+        let json = r#"{
+                "kernel_image_path": "/foo/bar",
+                "boot_args": "baz"
+              }"#;
         let body: Chunk = Chunk::from(json);
 
         // GET
@@ -800,7 +798,7 @@ mod tests {
         // Falling back to json deserialization for constructing the "correct" request because not
         // all of BootSourceBody's members are accessible. Rather than making them all public just
         // for the purpose of unit tests, it's preferable to trust the deserialization.
-        let res_bsb = serde_json::from_slice::<BootSourceBody>(&body);
+        let res_bsb = serde_json::from_slice::<BootSourceConfig>(&body);
         match res_bsb {
             Ok(boot_source_body) => {
                 match parse_boot_source_req(&path_tokens, &path, Method::Put, &body) {

api_server/src/request/boot_source.rs

@@ -6,33 +6,16 @@ use hyper::{Response, StatusCode};
 use http_service::{json_fault_message, json_response};
 use request::{GenerateResponse, ParsedRequest, VmmAction};
 
-#[derive(Debug, Deserialize, PartialEq, Serialize)]
-pub enum BootSourceType {
-    LocalImage,
-}
-
 #[derive(Debug, Deserialize, PartialEq, Serialize)]
 #[serde(deny_unknown_fields)]
-pub struct LocalImage {
+pub struct BootSourceConfig {
     pub kernel_image_path: String,
-}
-
-#[derive(Debug, Deserialize, PartialEq, Serialize)]
-#[serde(deny_unknown_fields)]
-pub struct BootSourceBody {
-    boot_source_id: String,
-    source_type: BootSourceType,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub local_image: Option<LocalImage>,
-    // drive_boot to be added later
-    // network_boot to be added later
     #[serde(skip_serializing_if = "Option::is_none")]
     pub boot_args: Option<String>,
 }
 
 #[derive(Debug)]
 pub enum BootSourceConfigError {
-    EmptyKernelPath,
     InvalidKernelPath,
     InvalidKernelCommandLine,
     UpdateNotAllowedPostBoot,
@@ -42,10 +25,6 @@ impl GenerateResponse for BootSourceConfigError {
     fn generate_response(&self) -> Response {
         use self::BootSourceConfigError::*;
         match *self {
-            EmptyKernelPath => json_response(
-                StatusCode::BadRequest,
-                json_fault_message("No kernel path is specified."),
-            ),
             InvalidKernelPath => json_response(
                 StatusCode::BadRequest,
                 json_fault_message(
@@ -65,7 +44,7 @@ impl GenerateResponse for BootSourceConfigError {
     }
 }
 
-impl BootSourceBody {
+impl BootSourceConfig {
     pub fn into_parsed_request(self) -> result::Result<ParsedRequest, String> {
         let (sender, receiver) = oneshot::channel();
         Ok(ParsedRequest::Sync(
@@ -97,20 +76,12 @@ mod tests {
 
     #[test]
     fn test_into_parsed_request() {
-        let body = BootSourceBody {
-            boot_source_id: String::from("/foo/bar"),
-            source_type: BootSourceType::LocalImage,
-            local_image: Some(LocalImage {
-                kernel_image_path: String::from("/foo/bar"),
-            }),
+        let body = BootSourceConfig {
+            kernel_image_path: String::from("/foo/bar"),
             boot_args: Some(String::from("foobar")),
         };
-        let same_body = BootSourceBody {
-            boot_source_id: String::from("/foo/bar"),
-            source_type: BootSourceType::LocalImage,
-            local_image: Some(LocalImage {
-                kernel_image_path: String::from("/foo/bar"),
-            }),
+        let same_body = BootSourceConfig {
+            kernel_image_path: String::from("/foo/bar"),
             boot_args: Some(String::from("foobar")),
         };
         let (sender, receiver) = oneshot::channel();

api_server/src/request/mod.rs

@@ -16,7 +16,7 @@ use hyper::{Method, StatusCode};
 
 use data_model::vm::{BlockDeviceConfig, DriveError, MachineConfiguration};
 
-use self::boot_source::BootSourceBody;
+use self::boot_source::BootSourceConfig;
 use self::logger::APILoggerDescription;
 use self::net::NetworkInterfaceBody;
 
@@ -68,7 +68,7 @@ impl GenerateResponse for () {
 // This enum contains messages for the VMM which represent sync requests. They each contain various
 // bits of information (ids, paths, etc.), together with an OutcomeSender, which is always present.
 pub enum VmmAction {
-    ConfigureBootSource(BootSourceBody, OutcomeSender),
+    ConfigureBootSource(BootSourceConfig, OutcomeSender),
     ConfigureLogger(APILoggerDescription, OutcomeSender),
     GetMachineConfiguration(OutcomeSender),
     InsertBlockDevice(BlockDeviceConfig, OutcomeSender),

api_server/swagger/firecracker.yaml

@@ -303,20 +303,14 @@ paths:
 definitions:
   BootSource:
     type: object
+    required:
+      - kernel_image_path
     description:
-      Boot source descriptor. The only supported boot source type is the Local Image.
-      Network Boot and Drive Boot will be added in future releases.
+      Boot source descriptor.
     properties:
-      boot_source_id:
-        type: string
-        description: Unique identifier for the boot source
-      source_type:
-        type: string
-        description: Type of boot source
-        enum:
-        - LocalImage
-      local_image:
-        $ref: "#/definitions/LocalImage"
+    kernel_image_path:
+            type: string
+            description: Host level path to the kernel image used to boot the guest
       boot_args:
         type: string
         description: Kernel boot arguments
@@ -402,15 +396,6 @@ definitions:
           - Halting
           - Halted
 
-  LocalImage:
-    type: object
-    description:
-      Locations for local kernel image.
-    properties:
-      kernel_image_path:
-        type: string
-        description: Host level path to the kernel image used to boot the guest
-
   Logger:
     type: object
     description:

tests/framework/microvm.py

@@ -390,11 +390,7 @@ def basic_config(
 
         # Add a kernel to start booting from.
         response = self.boot.put(
-            boot_source_id='1',
-            source_type='LocalImage',
-            local_image={'kernel_image_path': self.kernel_api_path(
-                create=True
-            )}
+            kernel_image_path=self.kernel_api_path(create=True)
         )
         assert self.api_session.is_good_response(response.status_code)
 

tests/framework/resources.py

@@ -85,19 +85,13 @@ def get(cls):
 
     @staticmethod
     def create_json(
-            boot_source_id=None,
-            source_type=None,
-            local_image=None,
-            boot_args=None
+            boot_args=None,
+            kernel_image_path=None
     ):
         """Compose the json associated to this type of API request."""
         datax = {}
-        if boot_source_id is not None:
-            datax['boot_source_id'] = boot_source_id
-        if source_type is not None:
-            datax['source_type'] = source_type
-        if local_image is not None:
-            datax['local_image'] = local_image
+        if kernel_image_path is not None:
+            datax['kernel_image_path'] = kernel_image_path
         if boot_args is not None:
             datax['boot_args'] = boot_args
         return datax

tests/integration_tests/functional/test_api.py

@@ -44,19 +44,15 @@ def test_api_put_update_pre_boot(test_microvm_with_api):
 
     # Updates to `kernel_image_path` with an invalid path are not allowed.
     response = test_microvm.boot.put(
-        boot_source_id='1',
-        source_type='LocalImage',
-        local_image={'kernel_image_path': 'foo.bar'}
+        kernel_image_path='foo.bar'
     )
     assert not test_microvm.api_session.is_good_response(response.status_code)
     assert "The kernel file cannot be opened due to invalid kernel path or " \
            "invalid permissions" in response.text
 
     # Updates to `kernel_image_path` with a valid path are allowed.
     response = test_microvm.boot.put(
-        boot_source_id='1',
-        source_type='LocalImage',
-        local_image={'kernel_image_path': test_microvm.kernel_api_path()}
+        kernel_image_path=test_microvm.kernel_api_path()
     )
     assert test_microvm.api_session.is_good_response(response.status_code)
 
@@ -255,9 +251,7 @@ def test_api_put_update_post_boot(test_microvm_with_api):
 
     # Valid updates to `kernel_image_path` are not allowed after boot.
     response = test_microvm.boot.put(
-        boot_source_id='1',
-        source_type='LocalImage',
-        local_image={'kernel_image_path': test_microvm.kernel_api_path()}
+        kernel_image_path=test_microvm.kernel_api_path()
     )
     assert not test_microvm.api_session.is_good_response(response.status_code)
     assert "The update operation is not allowed after boot" in response.text
@@ -493,8 +487,7 @@ def test_api_patch_pre_boot(test_microvm_with_api):
 
     # Partial updates to the boot source are not allowed.
     response = test_microvm.boot.patch(
-        boot_source_id='1',
-        local_image={'kernel_image_path': 'otherfile'}
+        kernel_image_path='otherfile'
     )
     assert not test_microvm.api_session.is_good_response(response.status_code)
     assert "Invalid request method" in response.text
@@ -561,8 +554,7 @@ def test_api_patch_post_boot(test_microvm_with_api):
 
     # Partial updates to the boot source are not allowed.
     response = test_microvm.boot.patch(
-        boot_source_id='1',
-        local_image={'kernel_image_path': 'otherfile'}
+        kernel_image_path='otherfile'
     )
     assert not test_microvm.api_session.is_good_response(response.status_code)
     assert "Invalid request method" in response.text

vmm/src/lib.rs

@@ -1361,14 +1361,10 @@ impl Vmm {
 
         match request {
             VmmAction::ConfigureBootSource(boot_source_body, sender) => {
-                let boxed_response = match boot_source_body.local_image {
-                    // Check that the kernel path exists and it is valid.
-                    Some(local_image) => Box::new(self.configure_boot_source(
-                        local_image.kernel_image_path,
-                        boot_source_body.boot_args,
-                    )),
-                    None => Box::new(Err(BootSourceConfigError::EmptyKernelPath)),
-                };
+                let boxed_response = Box::new(self.configure_boot_source(
+                    boot_source_body.kernel_image_path,
+                    boot_source_body.boot_args
+                ));
                 Vmm::send_response(boxed_response, sender);
             }
             VmmAction::ConfigureLogger(logger_description, sender) => {