cloud-init: Include `user.*` keys in `meta-data` file #14983

hamistao · 2025-02-14T04:39:51Z

We can't add make these values available through ds.config as we do when using the LXD datastore, so this is the next best thing.

This does raise the risk of adding duplicate keys since user.meta-data could include the same key as some user.*. Similarly to what is reported in #13853. But this won't be a concern after user.meta-data is removed.

Closes #14803

tomponline · 2025-02-14T07:44:33Z

We can't add make these values available through ds.config as we do when using the LXD datastore, so this is the next best thing.

Why is that?

stew3254 · 2025-02-14T12:15:43Z

We can't add make these values available through ds.config as we do when using the LXD datastore, so this is the next best thing.

Why is that?

Cloud-init limitations. The NoCloud DS doesn't support this so it would require changing upstream to my understanding. LXD is the only cloud I'm aware of not following the convention of ds.meta_data.

tomponline · 2025-02-14T12:17:41Z

We can't add make these values available through ds.config as we do when using the LXD datastore, so this is the next best thing.

Why is that?

Cloud-init limitations. The NoCloud DS doesn't support this so it would require changing upstream to my understanding. LXD is the only cloud I'm aware of not following the convention of ds.meta_data.

I wonder why cloud-init chose to expose the user vars from /dev/lxd under ds.config rather than ds.meta_data?

Its rather unfortunate that we cant offer a consistent experience here.

Any ideas @holmanb ?

hamistao · 2025-02-14T13:50:11Z

Its rather unfortunate that we cant offer a consistent experience here.

Indeed 😢

I wonder why cloud-init chose to expose the user vars from /dev/lxd under ds.config rather than ds.meta_data?

Just as additional info, and I am not entirely sure of this, but apparentely that accessing values through ds.config is only possible for the LXD datasource and no other.

Edit: Just saw this information was already stated by @stew3254 above

tomponline · 2025-02-14T14:19:23Z

OK I'm going to hold off on this until we've had confirmation from @holmanb that there's no way of doing this consistently. Thanks

tomponline · 2025-02-14T14:50:34Z

lxd/device/disk.go

+	// Add other user.* keys to meta-data so that cloud-init has access to this information.
+	for key, value := range instanceConfig {
+		if strings.HasPrefix(key, "user.") && !shared.ValueInSlice(key, reservedUserKeys) {
+			fmt.Println(key)


is this old debug ?

Rather than using fmt.Sprintf and string concatenation in this function, how about using strings.Builder to build up the contents of the file? As I understand strings.Builder is faster than concatenation and this file might end up being quite large depending on how many vars we're putting into it. It'll also likely be easier to read too :)

lxd/device/disk.go

hamistao · 2025-02-14T14:55:58Z

OK I'm going to hold off on this until we've had confirmation from @holmanb that there's no way of doing this consistently. Thanks

@tomponline We had confirmation from another member of the cloud-init team as mentioned in the issue (I am afraid I don't know his GH handle). You can check the cloud-init channel on Mattermost if you'd like 😉

tomponline · 2025-02-14T15:04:35Z

OK I'm going to hold off on this until we've had confirmation from @holmanb that there's no way of doing this consistently. Thanks

@tomponline We had confirmation from another member of the cloud-init team as mentioned in the issue (I am afraid I don't know his GH handle). You can check the cloud-init channel on Mattermost if you'd like 😉

Will you provide a link?

hamistao · 2025-02-14T15:08:55Z

Will you provide a link?

Here it is, I was on my phone and preferred to come to the computer to get the link: https://chat.canonical.com/canonical/pl/ieahsfitgjrd9p6okssof9r4yw

tomponline · 2025-02-14T15:50:24Z

lxd/device/disk.go

@@ -2597,11 +2597,21 @@ func (d *disk) generateVMConfigDrive() (string, error) {
 		}
 	}

-	// Append any custom meta-data to our predefined meta-data config.
+	reservedUserKeys := []string{"user.network-config", "user.user-data", "user.vendor-data", "user.meta-data"}


@hamistao @holmanb btw, does cloud-init itself have similar logic when generating the ds.config var when using the lxd datasource, so as not to request these keys from /dev/lxd?

It does not, every user.* key is added to ds.config. I have adjusted the implementation to match it.

Your other suggestion regarding the use of strings.Builder was also addressed.

tomponline

please can you use strings.Builder to build up the file data rather than fmt.Sprintf, it should work out clearer than having placeholders.

tomponline · 2025-02-18T12:48:51Z

lxd/device/disk.go


-	err = os.WriteFile(filepath.Join(scratchDir, "meta-data"), []byte(metaData), 0400)
+	// Append strings to the builder
+	metaDataBuilder.WriteString("instance-id: ")


Can we do 1 function call per line, e.g.

metaDataBuilder.WriteString("instance-id: "+d.inst.Name()+"\n")

The string build refacotring was done.

lxd/device/disk.go

tomponline · 2025-02-19T13:05:23Z

lxd/device/disk.go

+
+	// Add other user.* keys to meta-data so that cloud-init has access to this information.
+	for key, value := range instanceConfig {
+		if strings.HasPrefix(key, "user.") {


do we need to skip the legacy cloud-init keys here?

No, we do not since the LXD datasource doesn't do this when including user.* keys on ds.config.

Hrm that sounds like a bug though. It shouldnt serving up the legacy cloud-init config keys into the template right?

Sorry I should have been clearer. cloud-init just includes every defined config key that it has access to into ds.config. So this includes all user.* and cloud-init.* keys with the exception of cloud-init.ssh-keys.*. I included a duplicate logic here to derive if a hey can be exposed to cloud-init, but I will be moving it to the cloudinit package on SSH key injection improvement PR.

I am wondering if we should exclude user.meta-data though since its content is already included on ds.meta_data. But it could be used to distinguish the source of the meta-data and we are planning to remove it so this shouldn't be a big problem.

I dont think we should include user.user-data, user.vendor-data or user.meta-data as these are already included and are not custom user vars for accessing in the template.

cloud-init just includes every defined config key that it has access to into ds.config. So this includes all user.* and cloud-init.*

Yeah, this sounds like a bug, why would I want to reference user.user-data or cloud-init.user-data as a custom user template var?

We cannot do anything about it on the LXD DataSource, as we just show the defined keys and the logic for creating the datasource structure, such as ds.config, is part of cloud-init. But we can limit the keys we are including on meta_data on NoCloud datasource disk device.

@hamistao indeed.

Please can you bring this up with @holmanb to see if this needs also to be a bug report in cloud-init?

@holmanb Since we already called your attention, we are working on making user.* keys availlable for use in jinja templates when using the NoCloud datasource on LXD and we are discussing the fact that keys like cloud-init.user-data and user.network-config are included in ds.config, but this seems redundant since these keys are already used as a source of other data available to cloud-init, such as user-data or network-config. So the same information is included in more than one place. Do you think this could use some improvement?

lxd/device/disk.go

tomponline

One thought I had with this now.

Is it possible that by introducing this change we break an existing working instance that uses the NoCloud data store drive by including user.* keys that contain characters that render the meta-data unparseable by cloud-init?

hamistao · 2025-02-20T14:21:46Z

Is it possible that by introducing this change we break an existing working instance that uses the NoCloud data store drive by including user.* keys that contain characters that render the meta-data unparseable by cloud-init?

Good point. With the implementation you saw it is possible indeed, but single quoting added values prevents this. Single quotes included in the value itself can be escaped by using ''. I am pushing a new version in case you wish to see what it would look like.

We can't add make these values available through `ds.config` as we do when using the LXD datastore, so this is the next best thing. Signed-off-by: hamistao <[email protected]>

This fits the standard that we use for the LXD datasource, and there is no reason to use a different one in NoCloud. This also hepls to make changes to cloud-init configuration apply on every boot by updating the `instance-id` and allows a user to set a custom `instance-id` through `volatile.cloud-init.instance-id.` Signed-off-by: hamistao <[email protected]>

tomponline

Thanks!

github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Feb 14, 2025

hamistao force-pushed the nocloud_user_config branch from 2397234 to c21ceef Compare February 14, 2025 04:40

github-actions bot removed Documentation Documentation needs updating API Changes to the REST API labels Feb 14, 2025

hamistao changed the title ~~Nocloud_user_config~~ cloud-init: Include user.* keys on meta-data file Feb 14, 2025

hamistao mentioned this pull request Feb 14, 2025

user.* keys do not work with the cloud-init nocloud datasource provided by LXD #14803

Closed

hamistao changed the title ~~cloud-init: Include user.* keys on meta-data file~~ cloud-init: Include user.* keys in meta-data file Feb 14, 2025

tomponline reviewed Feb 14, 2025

View reviewed changes

lxd/device/disk.go Outdated Show resolved Hide resolved

hamistao force-pushed the nocloud_user_config branch from c21ceef to 9e13da3 Compare February 14, 2025 14:54

tomponline reviewed Feb 14, 2025

View reviewed changes

tomponline requested changes Feb 14, 2025

View reviewed changes

hamistao force-pushed the nocloud_user_config branch from 9e13da3 to c74036c Compare February 15, 2025 16:55

hamistao mentioned this pull request Feb 15, 2025

tests: Add tests for the NoCloud datasource canonical/lxd-ci#416

Draft

hamistao force-pushed the nocloud_user_config branch from c74036c to bacea4e Compare February 17, 2025 20:43

tomponline reviewed Feb 18, 2025

View reviewed changes

lxd/device/disk.go Outdated Show resolved Hide resolved

tomponline reviewed Feb 18, 2025

View reviewed changes

lxd/device/disk.go Outdated Show resolved Hide resolved

tomponline reviewed Feb 18, 2025

View reviewed changes

lxd/device/disk.go Outdated Show resolved Hide resolved

hamistao force-pushed the nocloud_user_config branch from bacea4e to aa6d6eb Compare February 19, 2025 12:52

tomponline reviewed Feb 19, 2025

View reviewed changes

lxd/device/disk.go Outdated Show resolved Hide resolved

tomponline reviewed Feb 19, 2025

View reviewed changes

hamistao force-pushed the nocloud_user_config branch 2 times, most recently from 621c19d to 64099c8 Compare February 19, 2025 14:04

tomponline reviewed Feb 20, 2025

View reviewed changes

lxd/device/disk.go Show resolved Hide resolved

hamistao force-pushed the nocloud_user_config branch 2 times, most recently from ea8403c to cb014b3 Compare February 20, 2025 13:51

tomponline reviewed Feb 20, 2025

View reviewed changes

hamistao force-pushed the nocloud_user_config branch from cb014b3 to 5d93e3c Compare February 20, 2025 14:21

hamistao added 2 commits February 20, 2025 11:45

lxd/device/disk: Include user.* keys on meta-data file

a3adf3e

We can't add make these values available through `ds.config` as we do when using the LXD datastore, so this is the next best thing. Signed-off-by: hamistao <[email protected]>

hamistao force-pushed the nocloud_user_config branch from 5d93e3c to c4207d2 Compare February 20, 2025 14:46

tomponline approved these changes Feb 20, 2025

View reviewed changes

tomponline merged commit 6ce7c48 into canonical:main Feb 20, 2025
29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cloud-init: Include `user.*` keys in `meta-data` file #14983

cloud-init: Include `user.*` keys in `meta-data` file #14983

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline commented Feb 14, 2025

stew3254 commented Feb 14, 2025

tomponline commented Feb 14, 2025 •

edited

Loading

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline commented Feb 14, 2025

tomponline Feb 14, 2025

tomponline Feb 14, 2025 •

edited

Loading

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline commented Feb 14, 2025

hamistao commented Feb 14, 2025

tomponline Feb 14, 2025

hamistao Feb 17, 2025

tomponline left a comment

tomponline Feb 18, 2025

hamistao Feb 19, 2025

tomponline Feb 19, 2025

hamistao Feb 19, 2025

tomponline Feb 19, 2025 •

edited

Loading

hamistao Feb 19, 2025 •

edited

Loading

hamistao Feb 19, 2025

tomponline Feb 19, 2025

tomponline Feb 19, 2025

hamistao Feb 19, 2025 •

edited

Loading

tomponline Feb 20, 2025

hamistao Feb 20, 2025

tomponline left a comment

hamistao commented Feb 20, 2025 •

edited

Loading

tomponline left a comment

cloud-init: Include user.* keys in meta-data file #14983

cloud-init: Include user.* keys in meta-data file #14983

Conversation

hamistao commented Feb 14, 2025 • edited Loading

tomponline commented Feb 14, 2025

stew3254 commented Feb 14, 2025

tomponline commented Feb 14, 2025 • edited Loading

hamistao commented Feb 14, 2025 • edited Loading

tomponline commented Feb 14, 2025

Choose a reason for hiding this comment

tomponline Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

hamistao commented Feb 14, 2025 • edited Loading

tomponline commented Feb 14, 2025

hamistao commented Feb 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomponline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomponline Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

hamistao Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamistao Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomponline left a comment

Choose a reason for hiding this comment

hamistao commented Feb 20, 2025 • edited Loading

tomponline left a comment

Choose a reason for hiding this comment

cloud-init: Include `user.*` keys in `meta-data` file #14983

cloud-init: Include `user.*` keys in `meta-data` file #14983

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline commented Feb 14, 2025 •

edited

Loading

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline Feb 14, 2025 •

edited

Loading

hamistao commented Feb 14, 2025 •

edited

Loading

tomponline Feb 19, 2025 •

edited

Loading

hamistao Feb 19, 2025 •

edited

Loading

hamistao Feb 19, 2025 •

edited

Loading

hamistao commented Feb 20, 2025 •

edited

Loading