Merge pull request #153 from cgwalters/status-cleanup

spec: Make status always required

Author: cgwalters

ci/run-kola.sh

@@ -25,6 +25,15 @@ storage:
     - path: /etc/ostree/auth.json
       contents:
         local: auth.json
+systemd:
+  units:
+    - name: zincati.service
+      dropins:
+        - name: disabled.conf
+          contents: |
+            [Unit]
+            ConditionPathExists=/enoent
+
 EOF
     butane -d . < pull-secret.bu > pull-secret.ign
     kola_args+=("--append-ignition" "pull-secret.ign")

lib/src/cli.rs

@@ -284,26 +284,31 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
     prepare_for_write().await?;
     let sysroot = &get_locked_sysroot().await?;
     let repo = &sysroot.repo();
-    let booted_deployment = &sysroot.require_booted_deployment()?;
-    let (_deployments, host) = crate::status::get_status(sysroot, Some(booted_deployment))?;
-    // SAFETY: There must be a status if we have a booted deployment
-    let status = host.status.unwrap();
+    let (booted_deployment, _deployments, host) =
+        crate::status::get_status_require_booted(sysroot)?;
     let imgref = host.spec.image.as_ref();
     // If there's no specified image, let's be nice and check if the booted system is using rpm-ostree
-    if imgref.is_none() && status.booted.as_ref().map_or(false, |b| b.incompatible) {
+    if imgref.is_none()
+        && host
+            .status
+            .booted
+            .as_ref()
+            .map_or(false, |b| b.incompatible)
+    {
         return Err(anyhow::anyhow!(
             "Booted deployment contains local rpm-ostree modifications; cannot upgrade via bootc"
         ));
     }
     let spec = RequiredHostSpec::from_spec(&host.spec)?;
-    let booted_image = status
+    let booted_image = host
+        .status
         .booted
         .map(|b| b.query_image(repo))
         .transpose()?
         .flatten();
     let imgref = imgref.ok_or_else(|| anyhow::anyhow!("No image source specified"))?;
     // Find the currently queued digest, if any before we pull
-    let staged = status.staged.as_ref();
+    let staged = host.status.staged.as_ref();
     let staged_image = staged.as_ref().and_then(|s| s.image.as_ref());
     let mut changed = false;
     if opts.check {
@@ -365,8 +370,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
 
     let sysroot = &get_locked_sysroot().await?;
     let repo = &sysroot.repo();
-    let booted_deployment = &sysroot.require_booted_deployment()?;
-    let (_deployments, host) = crate::status::get_status(sysroot, Some(booted_deployment))?;
+    let (booted_deployment, _deployments, host) =
+        crate::status::get_status_require_booted(sysroot)?;
 
     let transport = ostree_container::Transport::try_from(opts.transport.as_str())?;
     let imgref = ostree_container::ImageReference {
@@ -419,9 +424,8 @@ async fn edit(opts: EditOpts) -> Result<()> {
     prepare_for_write().await?;
     let sysroot = &get_locked_sysroot().await?;
     let repo = &sysroot.repo();
-    let booted_deployment = &sysroot.require_booted_deployment()?;
-    let (_deployments, host) = crate::status::get_status(sysroot, Some(booted_deployment))?;
-
+    let (booted_deployment, _deployments, host) =
+        crate::status::get_status_require_booted(sysroot)?;
     let new_host: Host = if opts.filename == "-" {
         let tmpf = tempfile::NamedTempFile::new()?;
         serde_yaml::to_writer(std::io::BufWriter::new(tmpf.as_file()), &host)?;

lib/src/privtests.rs

@@ -103,8 +103,7 @@ pub(crate) fn impl_run_container() -> Result<()> {
     assert!(ostree_ext::container_utils::is_ostree_container()?);
     let sh = Shell::new()?;
     let host: Host = serde_yaml::from_str(&cmd!(sh, "bootc status").read()?)?;
-    let status = host.status.unwrap();
-    assert!(status.is_container);
+    assert!(host.status.is_container);
     for c in ["upgrade", "update"] {
         let o = Command::new("bootc").arg(c).output()?;
         let st = o.status;

lib/src/spec.rs

@@ -19,7 +19,8 @@ pub struct Host {
     #[serde(default)]
     pub spec: HostSpec,
     /// The status
-    pub status: Option<HostStatus>,
+    #[serde(default)]
+    pub status: HostStatus,
 }
 
 #[derive(Serialize, Deserialize, Default, Debug, Clone, PartialEq, Eq)]
@@ -121,7 +122,7 @@ impl Host {
                 metadata,
             },
             spec,
-            status: None,
+            status: Default::default(),
         }
     }
 }

lib/src/status.rs

@@ -172,6 +172,15 @@ impl BootEntry {
     }
 }
 
+/// A variant of [`get_status`] that requires a booted deployment.
+pub(crate) fn get_status_require_booted(
+    sysroot: &SysrootLock,
+) -> Result<(ostree::Deployment, Deployments, Host)> {
+    let booted_deployment = sysroot.require_booted_deployment()?;
+    let (deployments, host) = get_status(sysroot, Some(&booted_deployment))?;
+    Ok((booted_deployment, deployments, host))
+}
+
 /// Gather the ostree deployment objects, but also extract metadata from them into
 /// a more native Rust structure.
 pub(crate) fn get_status(
@@ -227,12 +236,12 @@ pub(crate) fn get_status(
         })
         .unwrap_or_default();
     let mut host = Host::new(OBJECT_NAME, spec);
-    host.status = Some(HostStatus {
+    host.status = HostStatus {
         staged,
         booted,
         rollback,
         is_container,
-    });
+    };
     Ok((deployments, host))
 }
 
@@ -244,7 +253,7 @@ pub(crate) async fn status(opts: super::cli::StatusOpts) -> Result<()> {
             ..Default::default()
         };
         let mut r = Host::new(OBJECT_NAME, HostSpec { image: None });
-        r.status = Some(status);
+        r.status = status;
         r
     } else {
         let sysroot = super::cli::get_locked_sysroot().await?;