Adding Cached VM ID File for Provisioning Lifecycle Management

[peytonr18]

This PR adds a new status.rs module that helps manage the provisioning lifecycle for Azure Linux VMs.
The core idea is to create and check a .provisioned file named after the VM ID when provisioning is completed, ensuring we skip provisioning on reboot if the file already exists. Additionally, this file can serve as an indicator that provisioning has been completed.

---

Key Points

Provisioning File Logic

• We now store a file at \{provision_dir}/{vm_id}.provisioned.
• If the file exists, provisioning is considered complete; otherwise, we run provisioning.
• The provisioning directory is configurable through Config.provisioning_dir and defaults to /var/lib/azure-init/.

VM ID Retrieval

• Production retrieves the VM ID by reading /sys/class/dmi/id/product_uuid and byte-swapping if Gen1.
• is_vm_gen1() accepts a mock EFI path, allowing test scenarios to simulate different VM generations without relying on actual system files.

Integration with main.rs

• main.rs checks is_provisioning_complete() to see if it needs to run provisioning.
• After successful provisioning, mark_provisioning_complete() creates the .provisioned file.
• On subsequent boots, if the file is found, provisioning is skipped.
• Additionally, main.rs retrieves the vm_id once and passes it through all necessary functions.

Unit Test Setup

• Environment variables drive our partial mocking in tests:

  • TEST_PROVISION_DIR → points to a TempDir instead of /var/lib/azure-init/.
  • tests pass an explicit file path for VM ID retrieval.

• Tests verify that:

  • First run: no provisioning file → provisioning needed.
  • File is created: mark_provisioning_complete() actually writes the file.
  • Subsequent runs: the .provisioned file is detected, so provisioning is skipped (simulated reboot test).

• Linked Issues:

  • Resolves [RFE] azure-init should not attempt to provision the VM on reboot #120
  • Step 1 in integrating azure-init with WALinuxAgent

[anhvoms]

Since dmidecode is not used anymore, please fix the description to remove reference to dmidecode

[anhvoms]

I would recommend an additional cli feature to clear the provisioning directory. This might come in handy if we need to force azure-init to provision again. For any software that has a "cache", it should offer some feature to clear the cache

[cjp256]

TEST_PROVISION_DIR vs MOCK_VM_ID

if there are going to be env vars to support testing, maybe we need a scheme that's consistent.

e.g. AZURE_INIT_TEST_PROVISION_DIR AZURE_INIT_TEST_VM_ID

[cjp256]

it's possible to avoid this mock altogether by allowing an optional VM id param to

• is_provisioning_complete()
• mark_provisioning_complete()

but to get the best testing, it may be best to allow overriding "/sys/class/dmi/id/product_uuid" so we can put whatever id in there we want and test the scenarios.

is_vm_gen1() would some special care to mock or parameterized option for get_vm_id()

[cjp256]

it might be practical for testing if all paths derived from some configurable root path and any file structure is easily faked, but certainly beyond the scope of this PR

[peytonr18]

I can easily make an issue for the last idea and circle back to it!

For now, I updated is_provisioning_complete() and mark_provisioning_complete() to accept an optional vm_id, so we can pass in a test VM ID directly rather than relying on get_vm_id() or MOCK_VM_ID. In addition, I removed MOCK_VM_ID from get_vm_id() itself. Now it takes an optional path argument to override /sys/class/dmi/id/product_uuid, which avoids environment variables for testing.

I also updated is_vm_gen1() to accept an optional mock_efi_path, letting us simulate Gen1 or Gen2 in tests without touching real system files.

That was definitely a little more complicated than I anticipated, so please take a look and let me know what you think!

[peytonr18]

I amended the logic in main.rs, config.rs and status.rs so we use a configurable struct for the provisioining_dir instead of a hardcoded variable like this:

const DEFAULT_PROVISION_DIR: &str = "/var/lib/azure-init/";

With this change though, we have to pass the config into status.rs and that made it necessary to create a copy of the config struct in main (because Config by nature doesn't allow for Copy).

To me, it seemed better to do it as I show below because I anticipate needing a cloned version of config when I complete some of the issues related to toggling kvp telemetry on and off:

let clone_config: Config = config.clone();
match provision(config, opts).await {
mark_provisioning_complete(Some(&clone_config), None)

instead of doing this and cloning the config inline.

match provision(config.clone(), opts).await
mark_provisioning_complete(Some(&config), None)

This is one part of the PR that I would love feedback on if anyone else has a thought or a Rust standard we need to conform to!

[jeremycline]

Unfortunately, I think you're going to need to adjust the tests a bit to avoid writing to the environment 😿

[jeremycline]

You should be able to drop the byteorder dependency and avoid some bit shifting with something like the following:

fn swap_uuid_to_little_endian(mut bytes: [u8; 16]) -> Uuid {
    let (d1, remainder) = bytes.split_at(std::mem::size_of::<u32>());
    let d1 = d1.try_into().map(u32::from_be_bytes).unwrap_or(0).to_ne_bytes();
    
    let (d2, remainder) = remainder.split_at(std::mem::size_of::<u16>());
    let d2 = d2.try_into().map(u16::from_be_bytes).unwrap_or(0).to_ne_bytes();

    let (d3, _) = remainder.split_at(std::mem::size_of::<u16>());
    let d3 = d3.try_into().map(u16::from_be_bytes).unwrap_or(0).to_ne_bytes();

    let native_endian= d1
        .into_iter()
        .chain(d2.into_iter())
        .chain(d3.into_iter())
        .collect::<Vec<_>>();
    debug_assert_eq!(native_endian.len(), 8);
    bytes[..native_endian.len()].copy_from_slice(&native_endian);
    uuid::Uuid::from_bytes(bytes)
}

Frustratingly, I can't find a great way to avoid the fallible calls; there's not a stable API to split an array into a known length. split_at gives us a slice, which even though we know is the size of the u32, we have to use try_into() to attempt to convert it to [u8; 4]. I settled for using unwrap_or(0) although we should never hit the or case.

[jeremycline]

Ideally, I think, this function would be internal to the library and something it calls at the end of a successful provision. However, since the current Provision API has the user manually calling goalstate::report_health after it completes, perhaps we don't want this call to be done until after that?

We should consider moving the health reporting to be internal to the library as well and done for the user without additional calls, but if that's what we want to do, we can obviously do that separately.

[peytonr18]

I agree that having the library automatically call mark_provisioning_complete() once provisioning and health reporting are done would make the most sense! It'll probably be more intuitive to follow that logic too. I can file an issue so we can discuss there and decide if we want to wrap the entire flow (including report_health) inside the library?

In the meantime, if it's good with you, I'll just move the mark_provisioning_complete() call to occur after report_health() so we’re at least marking provisioning complete at the very end of the process (which I agree makes more sense).

[jeremycline]

Works for me 👍🏼

[jeremycline]

FWIW I don't think this is "odd" or breaking any Rust conventions I'm aware of so doing this or doing it inline are both fine

[anhvoms]

Config already has a default. If there's a possibility that the config module will fail to provide a default, this value (/var/lib/azure-init) should be declared a const that both modules can access. This way there's only one possible default

[peytonr18]

Should've caught that in my look-over -- will update, thank you!

[cjp256]

consider calling it something like [PERSISTENT_]STATE_DIR or simar. this path's scope is potentially wider than "provisioning" if azure-init wants non-provisioning state for some reason

[peytonr18]

Should we change the config struct too then to something similar?

pub const DEFAULT_PERSISTENT_STATE_DIR: &str = "/var/lib/azure-init/";

#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(default)]
pub struct PersistentStateDir{
    /// Specifies the path used for storing persistent status files.
    /// Defaults to `/var/lib/azure-init/`.
    pub path: PathBuf,
}

impl Default for PersistentStateDir {
    fn default() -> Self {
        Self {
            path: PathBuf::from(DEFAULT_PERSISTENT_STATE_DIR),
        }
    }
}

[peytonr18]

And, looking at the rest of the code too, are you okay with keeping the functions in status.rs get_provisioning_dir and check_provisioning_dir or does it make more sense to change them too?

TLDR --> get_persistent_state_dir and check_persistent_state_dir

[jeremycline]

It's looking pretty solid, and my notes are less relevant if the provisioning marking/checking APIs become private and part of the provision API down the road.

[jeremycline]

Since you're exposing this as a public API, I would recommend keeping the test parameters private. The easiest way would be to do something like this:
 

Suggested change

	/// Retrieves the VM ID by reading `/sys/class/dmi/id/product_uuid` and byte-swapping if Gen1.
	///
	/// The VM ID is a unique system identifier that persists across reboots but changes
	/// when a VM is cloned or redeployed.
	///
	/// # Returns
	/// - `Some(String)` containing the VM ID if retrieval is successful.
	/// - `None` if something fails or the output is empty.
	pub fn get_vm_id(
	custom_path: Option<&str>,
	mock_efi_path: Option<&str>,
	) -> Option<String> {
	/// Retrieves the VM ID by reading `/sys/class/dmi/id/product_uuid` and byte-swapping if Gen1.
	///
	/// The VM ID is a unique system identifier that persists across reboots but changes
	/// when a VM is cloned or redeployed.
	///
	/// # Returns
	/// - `Some(String)` containing the VM ID if retrieval is successful.
	/// - `None` if something fails or the output is empty.
	pub fn get_vm_id() -> Option<String> {
	private_get_vm_id(None, None)
	}
	fn private_get_vm_id(
	custom_path: Option<&str>,
	mock_efi_path: Option<&str>,
	) -> Option<String> {

[jeremycline]

Since this is a public function, it'd be good to expand the documentation slightly to describe what happens if either of the optional parameters isn't provided; if config is None, the default provisioning directory is used to check if things are complete, and if no VM id is provided, it'll be read.

If the parameters exist to make testing easier, you can use the same trick at above.

[peytonr18]

For this one, one of the parameters does actually matter beyond just testing --
config is how we determine the provisioning_dir (ie: where we put the vm_id.provisioned file).
main.rs sends the config when is_provisioning_competed() is called because it needs to know where to check for the file.

But, vm_id is just for testing - so I can make a private / public version here too. I'll also update the documentation to make everything clearer!

[peytonr18]

In the spirit of keeping this all consistent, I did the same for mark_provisioning_complete() and split it into a private and public function.

[cjp256]

This is a good example of where the optional parameter would be useful, desirable even. If the VM ID is known earlier it can be reused with this parameter without having to query it again.

The main entry point should query for VM ID and pass it to logging -> KVP.

And then it can re-use when querying for provisioning status.

This would avoid multiple calls to get_vm_id().

[peytonr18]

After some discussion offline, I've pushed a refactor to streamline how we handle vm_id. Instead of calling get_vm_id() in multiple places, we now retrieve it once in main.rs and pass it explicitly where needed (logging, provisioning, status etc.).

• vm_id is now passed from main.rs → setup_layers() → EmitKVPLayer, so EmitKVPLayer no longer fetches it itself.
• is_provisioning_complete() and mark_provisioning_complete() now take vm_id as a parameter from main.rs instead of each calling get_vm_id() internally.
• Since vm_id is always passed explicitly, we no longer need the separate private/public function split for is_provisioning_complete() and mark_provisioning_complete().

This removes redundant lookups with get_vm_id(). Let me know if anything looks off with that or anyone has any questions!

[cjp256]

While this works, I would suggest a mocking strategy that avoids new code paths for mocks. This inherently guarantees we can't get coverage over the actual potential code paths. While this function is relatively simple, it can be problematic for more complex cases.

Consider something like:

fn is_vm_gen1(sysfs_efi_path: Option<&str>, dev_efi_path: Option<&str>) -> bool {
    let sysfs_efi_path = sysfs_efi_path.unwrap_or("/sys/firmware/efi");
    let dev_efi_path = dev_efi_path.unwrap_or("/dev/efi");
    !Path::new(sysfs_efi_path).exists() && !Path::new(dev_efi_path).exists()
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::fs::{create_dir, remove_dir};

    #[test]
    fn test_gen1_vm() {
        // Mock paths that do not exist
        assert!(is_vm_gen1(Some("/nonexistent/efi"), Some("/nonexistent/dev_efi")));
    }

    #[test]
    fn test_gen2_vm_with_sysfs_efi() {
        let mock_path = "/tmp/mock_efi";
        create_dir(mock_path).ok(); // Create a mock directory
        assert!(!is_vm_gen1(Some(mock_path), Some("/nonexistent/dev_efi")));
        remove_dir(mock_path).ok(); // Clean up
    }

    #[test]
    fn test_gen2_vm_with_dev_efi() {
        let mock_path = "/tmp/mock_dev_efi";
        create_dir(mock_path).ok(); // Create a mock directory
        assert!(!is_vm_gen1(Some("/nonexistent/efi"), Some(mock_path)));
        remove_dir(mock_path).ok(); // Clean up
    }
}

This approach allows us to pretty much get 100% coverage.

[cjp256]

ah this is better. just a nit on name:

Suggested change

	custom_path: Option<&str>,
	product_uuid_path: Option<&str>,
	dev_efi_path: Option<&str>,

Opinionated, but instead of "mock_" just write a descriptive name which allow for overrides (either for tests or in some cases, callers that just want to specify some, usually default, parameter).

[jeremycline]

All my concerns have been addressed, thanks!

[anhvoms]

LGTM! Thanks for the work and all the testing for this PR