Added metadata cloud config support to CD ROM and openstack config drive providers

[robertwbl]

Continued from rancher/elemental-toolkit#2126

[davidcassany]

Would you mind amending the commit to include the Singed-off-by footer in the commit message?

To avoid having PRs blocked in the future, always include Signed-off-by: Author Name [email protected] in every commit message. You can also do this automatically by using the -s flag (i.e., git commit -s).

[davidcassany]

I am not sure this will be enough, but looks good to me adding support for meta-data

[davidcassany]

According to rancher/elemental-toolkit#2125 both files (user-data and metadata) are created for the Harvester use case. However the change here will only honor first match. I am not sure this is what we want. I am fine with the change proposed here to also check for meta-data, however the logic of this datasource only reads the first match (see snipped below), as soon as file is read stops trying the next one. So if both files are available at the same time we might need so more elaborated change.

@robertwbl what you think? does it make sense to you? I have the feeling this change is not enough after reading the issue you created and checking how this userdataFiles slice is being used.

linuxkit/providers/cdrom_shared.go

Lines 80 to 101 in fa64b2f

	func NewProviderCDROM(device string, datafiles []string, providerType string) *ProviderCDROM {
	mountPoint, err := os.MkdirTemp("", "cd")
	p := ProviderCDROM{providerType, device, mountPoint, err, []byte{}}
	if err == nil {
	if p.err = p.mount(); p.err == nil {
	defer p.unmount()
	// read the userdata - we read the spec file and the fallback, but eventually
	// will remove the fallback
	for _, f := range datafiles {
	userdata, err := os.ReadFile(path.Join(p.mountPoint, f))
	// did we find a file?
	if err == nil && userdata != nil {
	p.userdata = userdata
	break
	}
	}
	if p.userdata == nil {
	log.Debugf("no userdata file found at any of %v", datafiles)
	}
	}
	}
	return &p

[robertwbl]

Ah, good spot - I hadn't read the usage clearly! You're right that this wouldn't work since Harvester provides both meta-data and user-data separately and simultaneously. I'll have a think about how this might work.

[robertwbl]

I've just been exploring the original linuxkit repo this was forked from and it seems that has a way of handling the meta-data file: https://github.com/linuxkit/linuxkit/blob/be7dfdd42c6cc9365079f101b01536062729eda7/pkg/metadata/provider_cdrom.go#L121

[robertwbl]

It looks as though in this fork, that behaviour got factored out in #1 . @davidcassany your commit message on 9106b9d makes it sounds like at the time it was just considered to be unused and there's no other reason it was removed? I'd be keen to factor it back in this way if so.

[robertwbl]

I've factored this back in in a way that also considers the open stack config drive provider. According to https://docs.openstack.org/nova/rocky/user/config-drive.html it can also be expected to find a meta_data.json file here. Let me know what you think, thanks!

[davidcassany]

Interesting, this looks good to me.

However I still think this is not enough as this doesn't change anything to the providers interface, there is no method to extract the metadata from the interface.

Some options would be adding an extra method ExtractMetadata to the interface and simply return empty data for providers not having any metadata. Another option, if we are certain that this meta-data is only going to provide hostname and some limited set of variables, we could follow a similar approach as it is in done with most public cloud providers. Serveral public cloud providers write the hostname to a specific file that Elemental Toolkit consumes. See

linuxkit/providers/provider.go

Lines 25 to 26 in ac35c7b

	// Hostname is the filename in configPath where the hostname is stored
	Hostname = "hostname"

I am trying to figure out the format and syntax of meta-data file now.

In any case I think your PR is a great base for the final solution.

[davidcassany]

Ok after finding the related cloud-init docs here I am starting to consider that we probably should only read the hostname form the metadata and use the same mechanism that is used for other cloud providers in this library.

@robertwbl does it make sense to you? Do you feel like providing such an implementation? I can try to help if needed or even take it from this current PR if you just prefer us to finalize it.