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

Is loading of waagent obsolete? If so, please document that fact and update the SampleExtension #1956

Open
JohnRusk opened this issue Aug 16, 2024 · 0 comments

Comments

@JohnRusk
Copy link
Member

JohnRusk commented Aug 16, 2024

After some investigation, I understand that it's no longer encouraged for extensions to load waagent. I.e. this coding pattern is now discouraged, if I understand correctly:

from Utils.WAAgentUtil import waagent

I believe it's function was, once upon a time, to offer a level of convenience so that the extensions could call helper functions in waagent.

But now, I think there are three problems with it:

  1. If you do it without shipping your own copy of waagent, I believe it will fail. I believe this because, if I understand the code correctly, this code in the latest WALinuxAgent repo will refuse to be loaded as waagent, if the VM runs Python 3. https://github.com/Azure/WALinuxAgent/blob/master/bin/waagent. I that code explicitly deprecates this coding pattern, and in fact makes it impossible in most cases.
  2. Therefore, the only way to make this coding pattern work is to ship your own file name waagent. That requires you to use a very old copy of wwagent that is quite a few years old.
  3. The above leads to a really confusing code pattern, when extensions ship complete but very old copies of waagent, simply so they can call a small number of it's helper methods. All the rest of the code those old copies of waagent is unused.
  4. There have recently been changes to WAAgent util with the aim of making it compatible with Python 3.12. However, I don't actually think that continued use of this pattern is the best option. I believe the better option is to deprecate this pattern and stop loading waagent from within the extensions.

As far as I can tell, the best solution is the one used in this PR #1097. I.e. drop all dynamic loading of any kind of wagent file. Instead, use the new handlerutil2.py and logger.py which provide equivalent functionality without any dynamic loading of waagent. Not that the referenced pull request doesn't just include a simple replacement of WAAgentUtil and the waagent file. It also includes things that are specific to the particular extension for which that PR was done. So that makes the PR a little harder to read.

Can you please confirm whether the above is corrrect, and if it is,
(a) update the SampleExtension to stop using WAAgentUtil.py (your PR here can show folks clearly how to convert from the old approach to the new. I.e. it will be like a slimmed down versio of #1097, not introducing any new files, but just converting the SampleExtension over to call the files that were introduced in that RP). and
(b) please add comments into WAAgentUtil explaining that it is deprecated, and why it is deprecated.

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

No branches or pull requests

1 participant