Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Azure metadata providers #3585

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

schnerring
Copy link

@schnerring schnerring commented Dec 28, 2020

This affects issues:

  • Improve Azure support #2447
  • linuxkit limitations, mentioned in docs/platform-azure.md:
    • Since the image currently does not contain the Azure Linux Agent, the Azure Portal will report the creation as failed.

    • The metadata package does not yet support the Azure metadata.

What I did

  1. Add Azure WireServer client
  2. Add Azure IMDS provider
  3. Add Azure OVF provider

How I did it

  1. I implement a simple WireServer client provider_azure_wire.go that reports ready to Azure. Both IMDS and OVF provider depend on it, otherwise provisioning won't succeed. For generic steps, see Microsoft docs: Creating generalized images without a provisioning agent
  2. Implement Azure IMDS metadata provider provider_azure_imds.go that utilizes the Instance Metadata Service API. User data is currently disabled, see:
  3. Implement Azure OVF metadata provider_azure_ovf.go which extracts user data from the ovf-env.xml file on the Azure attached provisioning DVD. The IMDS provider also uses this provider as a fallback to get user data. The implementation is inspired by cloud-init's Azure docs and https://github.com/Azure/WALinuxAgent

How to verify it

k3OS uses linuxkit's metadata package. With the Azure provisioners I can successfully create managed k3OS images for Azure and provision Azure VMs with it. SSH keys and user data provided via Azure CLI / Portal GUI during provisioning works.
I forked k3OS, created a Packer template and and swapped linuxkit/linuxkit for schnerring/linuxkit.

You can find the Packer template using the forked versions on the packer-azure-test branch.

Description for the changelog

Add IMDS and OVF Azure providers to metadata package

Questions / Thoughts

I don't know how the metadata package integrates with the rest of linuxkit but it feels very much like reinventing the wheel. Why not bundle cloud-init or WALinuxAgent agents with linuxkit? Reimplementations will always be inferior compared to these. Is it because those agents are too bloated?


The metadata package's distinction between cdromProviders and netProviders doesn't make sense for Azure providers. The IMDS provider (netProvider) uses the OVF provider (technically cdromProvider) as a fallback for getting user data. For example, cloud-init also uses this fallback mechanism for SSH keys:

Cloud-init will first try to utilize SSH keys returned from IMDS, and if they are not provided from IMDS then it will fallback to using the OVF file provided from the CD-ROM

Should the fallback mechanism be part of the IMDS provider itself (relying on another provider) or should main.go implement the "hybrid" provider?


Thus far, the OVF provider only extracts user data but not hostname or SSH keys, so it's not usable as standalone provider, only as a fallback for the IMDS provider.


Any ideas or feedback is appreciated.

@schnerring schnerring force-pushed the provider-azure branch 2 times, most recently from 2d489f8 to d57d7c6 Compare December 28, 2020 15:41
@schnerring schnerring changed the title Add Azure providers Add Azure metadata providers Dec 28, 2020
@schnerring schnerring force-pushed the provider-azure branch 4 times, most recently from b3fc21d to 8664a2d Compare December 29, 2020 18:18
@justincormack
Copy link
Member

Hey thanks for this. Yes cloud-init is very large and assumes it can run shell scripts to configure things on the host, which doesn't generally work, and conflicts with immutability. WALinuxAgent is gigantic and tries to guess how to poke around to configure things based on distro, so does not work. The aim of the metadata package is to gather minimal config info that is useful, and to be workable across multiple providers for a base setup. You can use other options if you like if they work, LinuxKit always gives you a choice.

@schnerring
Copy link
Author

schnerring commented Dec 30, 2020

Hey thanks for this.

Thanks for the quick response. Fun opportunity to learn some Go :)

The aim of the metadata package is to gather minimal config info that is useful, and to be workable across multiple providers for a base setup. You can use other options if you like if they work, LinuxKit always gives you a choice.

So this would be hostname, SSH key(s) and user data? Am I missing something?

Regarding immutability, the AzureOVF provider's Probe() will only ever succeed on the very first boot of a VM. For subsequent reboots, Azure will not reattach the provisioning CD. I believe this doesn't conflict with the metadata package design?

@stevefan1999-personal
Copy link

Hi I would like to merge your PR with mine, is that ok to you ;p

@schnerring
Copy link
Author

Hi I would like to merge your PR with mine, is that ok to you ;p

What do you mean by "merge"? If this PR gets merged, you can rebase your PR off of new master, no reason to include the same set of changes in two PRs.

@stevefan1999-personal
Copy link

stevefan1999-personal commented Jan 5, 2021

@schnerring I remember I also had an implementation for Azure, though I have reported the metadata API isn't working as intended, and even after months Microsoft aren't doing anything about it. I just wait and left it in the cold until you also popped up and shown an alternative implementation.

But fine, I will just drop mine so you can go first.

Oh wait I see in the original post you mentioned me already. I shouldn't take this as a TLDR.

@schnerring
Copy link
Author

Is there anything I can do to get this merged? #3593 was merged 14 days ago, I don't know what's keeping this PR from getting merged besides rebasing. Any input is appreciated.

pkg/metadata/main.go Outdated Show resolved Hide resolved
@schnerring schnerring force-pushed the provider-azure branch 4 times, most recently from fabaf55 to bee6e00 Compare February 11, 2021 20:00
@schnerring
Copy link
Author

schnerring commented Feb 12, 2021

So, as previously discussed, I have implemented the following improvements:

  • rebase existing commits onto latest master
  • move retry() function to OVF provider
  • remove retry() from IMDS provider
  • remove OVF provider from default provider list in metadata/main.tf. The default provider list now only contains a single azure provider

Unfortunately we cannot get rid of the retry() logic. As long as MicrosoftDocs/azure-docs#64154 is not resolved, there's no chance of getting user-data from IMDS directly. This means we have to use the OVF provider as a fallback inside the IMDS provider.

The couple of times I've tested, mounting the CDROM device always succeeded with the 3rd try, so it takes around 10 seconds for probing to succeed. I guess this could take longer when the Azure platform has latency issues. I just stuck to what WALinuxAgent does (6 times 5 seconds).

So the best we can do here @rn is to move the Azure provider to the end of the provider list, so it gets probed last in case no arguments were provided.

@schnerring
Copy link
Author

schnerring commented Apr 25, 2021

@justincormack @rn Please don't code review, yet. There has been an update regarding the Microsoft IMDS API. See my comment in the GitHub issue: MicrosoftDocs/azure-docs#64154 (comment)

The customData feature has been deprecated in favor of the userData which was introduced with version 2021-01-01 of the IMDS API on April 15th 2021. This means that the OVF provider is obsolete which solves the retry() problem.

I'm excited to test this and I'll be refactoring the code within the next two weeks.

Signed-off-by: Michael Schnerring <3743342+schnerring@users.noreply.github.com>
Signed-off-by: Michael Schnerring <3743342+schnerring@users.noreply.github.com>
… to Azure-OVF provider

Signed-off-by: Michael Schnerring <3743342+schnerring@users.noreply.github.com>
Signed-off-by: Michael Schnerring <3743342+schnerring@users.noreply.github.com>
@schnerring
Copy link
Author

schnerring commented May 24, 2021

Finally got around doing this and I'm ready for a review @rn @justincormack

I'm excited to tell you guys that all the previous issues regarding retry() logic are now resolved because the OVF provider is not required anymore, now that userData is available through the IMDS.

The provider is now as lightweight as any other provider, like GCP or AWS. One issue I'm having is that customData is not obsolete as I previously suspected:

  • customData is only available during initial provisioning and would require the OVF provider if we wanted to support it because it will never be available through the IMDS API.
  • userData is available and mutable during the lifecycle of a VM and available through the IMDS API.

It's two separate sets of data that can be provided during VM creation, as you can see here (Azure Portal):

image

We could keep the OVF provider but just not include it in the default provider list to also support customData. Or we could just decide to not support it at all. Let me know what you guys think.

@Itxaka
Copy link
Contributor

Itxaka commented Jul 1, 2021

@rn @justincormack Is there something we can do to move this forward? Is something missing from the PR? We are also using linuxkit and we are missing on the azure functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants