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

support ubuntu24.04 with python3.12 #1964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions AzureMonitorAgent/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import hashlib
import fileinput
import contextlib
import distro

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the drive-by comments @marcurdy - I don't work on this project but I would like to get this issue resolved so my team can use Ubuntu 24.04 hosts on Azure without issue!

distro is a third-party package, and from what I can see in the way that this AzureMonitorAgent project is set up, the project has no requirements.txt / setup.py or any other mechanism to declare third-party dependencies. So I think this is intended to be deployed as just a single script, and adding a third-party dep will not work?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on my ubuntu 24.04, distro comes ootb even in minimized installations. IMHO dropping it would be wrong, but it would be prudent to put it behind a try clause.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gilbahat cool, good to know. a try/except sounds appropriate here. When I get a chance this week I can do some tests with Docker to see whether major distributions include this library out of the box.

from collections import OrderedDict
from hashlib import sha256
from shutil import copyfile
Expand Down Expand Up @@ -812,7 +813,7 @@ def handle_mcs_config(public_settings, protected_settings, default_configs):
bypass = json_data["proxy.bypass"]
if bypass == "AMA":
BypassProxy = True

if not BypassProxy and json_data is not None and "proxy.url" in json_data:
url = json_data["proxy.url"]
# only non-authenticated proxy config is supported
Expand Down Expand Up @@ -1651,12 +1652,11 @@ def find_vm_distro(operation):
# platform commands used below aren't available after Python 3.6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment and check seem wrong actually - platform.dist and platform.linux_distribution were documented in Python 3.7 and removed in Python 3.8.

if sys.version_info < (3,7):
try:
vm_dist, vm_ver, vm_id = platform.linux_distribution()
vm_dist = distro.id()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you're adding fixes for Python 3.12 and higher, I'd leave this branch alone that was meant for Python <3.7

vm_ver = distro.version()
vm_id = distro.codename()
except AttributeError:
try:
vm_dist, vm_ver, vm_id = platform.dist()
except AttributeError:
hutil_log_info("Falling back to /etc/os-release distribution parsing")
hutil_log_info("Falling back to /etc/os-release distribution parsing")

# Some python versions *IF BUILT LOCALLY* (ex 3.5) give string responses (ex. 'bullseye/sid') to platform.dist() function
# This causes exception in the method below. Thus adding a check to switch to manual parsing in this case
Expand Down Expand Up @@ -1695,7 +1695,7 @@ def find_vm_distro(operation):
def is_vm_supported_for_extension(operation):
"""
Checks if the VM this extension is running on is supported by AzureMonitorAgent
Returns for platform.linux_distribution() vary widely in format, such as
Returns for distro.linux_distribution() vary widely in format, such as
'7.3.1611' returned for a VM with CentOS 7, so the first provided
digits must match
The supported distros of the AzureMonitorLinuxAgent are allowed to utilize
Expand All @@ -1720,7 +1720,7 @@ def is_vm_supported_for_extension(operation):
}

supported_dists_aarch64 = {'red hat' : ['8'], # Rhel
'ubuntu' : ['18.04', '20.04', '22.04'], # Ubuntu
'ubuntu' : ['18.04', '20.04', '22.04', '24.04'], # Ubuntu
'alma' : ['8'], # Alma
'centos' : ['7'], # CentOS
'mariner' : ['2'], # Mariner 2.0
Expand Down
72 changes: 32 additions & 40 deletions Common/WALinuxAgent-2.0.16/waagent
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import zipfile
import json
import datetime
import xml.sax.saxutils
import distro

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above - waagent seems to be set up to be a single file script, so I don't think third-party dependencies would be allowed

# distutils.version was deprecated in Python 3.10 and removed in Python 3.12
def import_loose_version():
if sys.version_info >= (3, 12):
Expand Down Expand Up @@ -594,7 +595,7 @@ class AbstractDistro(object):
Log("GPT detected.")
# GPT(Guid Partition Table) is used.
# Get partitions.
parts = filter(lambda x: re.match("^\s*[0-9]+", x), ret[1].split("\n"))
parts = filter(lambda x: re.match(r"^\s*[0-9]+", x), ret[1].split("\n"))
# If there are more than 1 partitions, remove all partitions
# and create a new one using the entire disk space.
if len(parts) > 1:
Expand Down Expand Up @@ -769,7 +770,7 @@ class AbstractDistro(object):
try:
with open("/var/lib/hyperv/.kvp_pool_0", "r") as f:
lines = f.read()
r = re.search("NdDriverVersion\0+(\d\d\d\.\d)", lines)
r = re.search(r"NdDriverVersion\0+(\d\d\d\.\d)", lines)
if r is not None:
NdDriverVersion = r.groups()[0]
return NdDriverVersion # e.g. NdDriverVersion = 142.0
Expand Down Expand Up @@ -1093,7 +1094,7 @@ class SuSEDistro(AbstractDistro):
"""
error, output = RunGetOutput(self.zypper_path + " info " + RdmaConfig.rmda_package_name)
if (error == RdmaConfig.process_success):
r = re.search("Version: (\S+)", output)
r = re.search(r"Version: (\S+)", output)
if r is not None:
package_version = r.groups()[0] # e.g. package_version is "20150707.140.0_k3.12.28_4-3.1."
return package_version
Expand Down Expand Up @@ -1183,7 +1184,7 @@ class SuSEDistro(AbstractDistro):
r = os.listdir("/opt/microsoft/rdma")
if r is not None:
for filename in r:
if re.match(RdmaConfig.rmda_package_name + "-\d{8}\.(%s).+" % host_version, filename):
if re.match(RdmaConfig.rmda_package_name + r"-\d{8}\.(%s).+" % host_version, filename):
error, output = RunGetOutput(
self.zypper_path + " --non-interactive remove " + RdmaConfig.rmda_package_name)
Log("remove rdma package result is " + str(error) + " output is: " + str(output))
Expand Down Expand Up @@ -1212,7 +1213,7 @@ class SuSEDistro(AbstractDistro):
# nd_driver_version 140.0
Log("nd_driver_version is " + str(nd_driver_version) + " package_version is " + str(package_version))
if (nd_driver_version is not None):
r = re.match("^[0-9]+[.](%s).+" % nd_driver_version,
r = re.match(r"^[0-9]+[.](%s).+" % nd_driver_version,
package_version) # NdDriverVersion should be at the end of package version
if not r: # host ND version is the same as the package version, do an update
return RdmaConfig.OutOfDate
Expand Down Expand Up @@ -3122,7 +3123,7 @@ def DoInstallRHUIRPM():
SetFileContents(LibDir + "/rhuirpminstalled", "")

Log("Begin to install RHUI RPM")
cmd = "grep '<Location>' /var/lib/waagent/ExtensionsConfig* --no-filename | sed 's/<Location>//g' | sed 's/<\/Location>//g' | sed 's/ //g' | tr 'A-Z' 'a-z' | uniq"
cmd = "grep '<Location>' /var/lib/waagent/ExtensionsConfig* --no-filename | sed 's/<Location>//g' | sed 's/<\\/Location>//g' | sed 's/ //g' | tr 'A-Z' 'a-z' | uniq"

retcode, out = RunGetOutput(cmd, True)
region = out.rstrip("\n")
Expand Down Expand Up @@ -3936,9 +3937,10 @@ class RdmaHandler(object):
def write_dat_conf(self, dat_conf_file):
Log("Write config to {0}".format(dat_conf_file))
old = ("ofa-v2-ib0 u2.0 nonthreadsafe default libdaplofa.so.2 "
"dapl.2.0 \"\S+ 0\"")
r"dapl.2.0 \"\S+ 0\"")
new = ("ofa-v2-ib0 u2.0 nonthreadsafe default libdaplofa.so.2 "
"dapl.2.0 \"{0} 0\"").format(self.ip_addr)
r"dapl.2.0 \"{0} 0\"").format(self.ip_addr)

lines = GetFileContents(dat_conf_file)
lines = re.sub(old, new, lines)
SetFileContents(dat_conf_file, lines)
Expand Down Expand Up @@ -5462,7 +5464,7 @@ class WALAEventMonitor(WALAEvent):
if not self.issysteminfoinitilized:
self.issysteminfoinitilized = True
try:
self.sysInfo["OSVersion"] = platform.system() + ":" + "-".join(DistInfo(1)) + ":" + platform.release()
self.sysInfo["OSVersion"] = distro.name(pretty=True) + ":" + "-".join(DistInfo(1)) + ":" + distro.codename()
self.sysInfo["GAVersion"] = GuestAgentVersion
self.sysInfo["RAM"] = MyDistro.getTotalMemory()
self.sysInfo["Processors"] = MyDistro.getProcessorCores()
Expand Down Expand Up @@ -6488,15 +6490,15 @@ def ApplyVNUMAWorkaround():
If kernel version has NUMA bug, add 'numa=off' to
kernel boot options.
"""
VersionParts = platform.release().replace('-', '.').split('.')
VersionParts = distro.version().split('.')
if int(VersionParts[0]) > 2:
return
if int(VersionParts[1]) > 6:
return
if int(VersionParts[2]) > 37:
return
if AppendToLinuxKernelCmdline("numa=off") == 0:
Log("Your kernel version " + platform.release() + " has a NUMA-related bug: NUMA has been disabled.")
Log("Your kernel version " + distro.codename() + " has a NUMA-related bug: NUMA has been disabled.")
else:
"Error adding 'numa=off'. NUMA has not been disabled."

Expand Down Expand Up @@ -6560,17 +6562,11 @@ def GetMyDistro(dist_class_name=''):
Return MyDistro object.
NOTE: Logging is not initialized at this point.
"""
if dist_class_name == '':
if 'Linux' in platform.system():
Distro = DistInfo()[0]
else: # I know this is not Linux!
if 'FreeBSD' in platform.system():
Distro = platform.system()
Distro = Distro.strip('"')
Distro = Distro.strip(' ')
dist_class_name = Distro + 'Distro'
if 'FreeBSD' in distro.name(pretty=True):
Distro = distro.name(pretty=True)
else:
Distro = dist_class_name
Distro = DistInfo()[0]
dist_class_name = Distro + 'Distro'
if dist_class_name not in globals():
msg = Distro + ' is not a supported distribution. Reverting to DefaultDistro to support scenarios in ' \
'unknown/unsupported distribution.'
Expand All @@ -6583,21 +6579,17 @@ def GetMyDistro(dist_class_name=''):


def DistInfo(fullname=0):
if 'FreeBSD' in platform.system():
release = re.sub('\-.*\Z', '', str(platform.release()))
if 'FreeBSD' in distro.name(pretty=True):
release = distro.version()
distinfo = ['FreeBSD', release]
return distinfo

if 'linux_distribution' in dir(platform):
distinfo = list(platform.linux_distribution(full_distribution_name=fullname))
distinfo[0] = distinfo[0].strip() # remove trailing whitespace in distro name
else:
distinfo = list((distro.id(), distro.version(), distro.codename()))
if not distinfo[0]:
distinfo = dist_info_SLES15()
if not distinfo[0]:
distinfo = dist_info_opensuse15()
return distinfo
else:
return platform.dist()


def dist_info_SLES15():
Expand Down Expand Up @@ -6780,20 +6772,20 @@ def main():
global force
force = False
for a in sys.argv[1:]:
if re.match("^([-/]*)(help|usage|\?)", a):
if re.match(r"^([-/]*)(help|usage|\?)", a):
sys.exit(Usage())
elif re.match("^([-/]*)version", a):
elif re.match(r"^([-/]*)version", a):
print(GuestAgentVersion + " running on " + LinuxDistro)
sys.exit(0)
elif re.match("^([-/]*)verbose", a):
elif re.match(r"^([-/]*)verbose", a):
myLogger.verbose = True
elif re.match("^([-/]*)force", a):
elif re.match(r"^([-/]*)force", a):
force = True
elif re.match("^(?:[-/]*)conf=.+", a):
elif re.match(r"^(?:[-/]*)conf=.+", a):
conf_file = re.match("^(?:[-/]*)conf=(.+)", a).groups()[0]
elif re.match("^([-/]*)(setup|install)", a):
elif re.match(r"^([-/]*)(setup|install)", a):
sys.exit(MyDistro.Install())
elif re.match("^([-/]*)(uninstall)", a):
elif re.match(r"^([-/]*)(uninstall)", a):
sys.exit(Uninstall())
else:
args.append(a)
Expand All @@ -6812,13 +6804,13 @@ def main():
global daemon
daemon = False
for a in args:
if re.match("^([-/]*)deprovision\+user", a):
if re.match(r"^([-/]*)deprovision\+user", a):
sys.exit(Deprovision(force, True))
elif re.match("^([-/]*)deprovision", a):
elif re.match(r"^([-/]*)deprovision", a):
sys.exit(Deprovision(force, False))
elif re.match("^([-/]*)daemon", a):
elif re.match(r"^([-/]*)daemon", a):
daemon = True
elif re.match("^([-/]*)serialconsole", a):
elif re.match(r"^([-/]*)serialconsole", a):
AppendToLinuxKernelCmdline("console=ttyS0 earlyprintk=ttyS0")
Log("Configured kernel to use ttyS0 as the boot console.")
sys.exit(0)
Expand Down
4 changes: 2 additions & 2 deletions Utils/HandlerUtil.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ def error(self, message):

@staticmethod
def redact_protected_settings(content):
redacted_tmp = re.sub('"protectedSettings":\s*"[^"]+=="', '"protectedSettings": "*** REDACTED ***"', content)
redacted = re.sub('"protectedSettingsCertThumbprint":\s*"[^"]+"', '"protectedSettingsCertThumbprint": "*** REDACTED ***"', redacted_tmp)
redacted_tmp = re.sub(r'"protectedSettings":\s*"[^"]+=="', '"protectedSettings": "*** REDACTED ***"', content)
redacted = re.sub(r'"protectedSettingsCertThumbprint":\s*"[^"]+"', '"protectedSettingsCertThumbprint": "*** REDACTED ***"', redacted_tmp)
return redacted

def _parse_config(self, ctxt):
Expand Down