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.

Sign up for GitHub

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

Jump to bottom

Implementation of new conf flag AutoUpdate.UpdateToLatestVersion support #3027

Merged
merged 11 commits into from
Feb 6, 2024
Merged

Implementation of new conf flag AutoUpdate.UpdateToLatestVersion support #3027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
adb897c
GA update to latest version flag
nagworld9 Jan 17, 2024
d214aa5
address comments
nagworld9 Jan 23, 2024
8982384
resloving comments
nagworld9 Jan 30, 2024
fd34c0c
added TODO
nagworld9 Jan 30, 2024
68ac618
ignore warning
nagworld9 Jan 31, 2024
4b94769
resolving comment
nagworld9 Feb 1, 2024
5c0f4d2
address comments
nagworld9 Feb 2, 2024
5261a63
config present check
nagworld9 Feb 2, 2024
6d6ccd5
added a comment
nagworld9 Feb 2, 2024
41fbdc3
Merge branch 'develop' into updateflag
nagworld9 Feb 5, 2024
b526bda
Merge branch 'develop' into updateflag
nagworld9 Feb 6, 2024
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
21 changes: 11 additions & 10 deletions azurelinuxagent/common/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,8 @@ def get_autoupdate_gafamily(conf=__conf__):
return conf.get("AutoUpdate.GAFamily", "Prod")


def get_autoupdate_enabled(conf=__conf__):
return conf.get_switch("AutoUpdate.Enabled", True)
def get_autoupdate_enabled(conf=__conf__, default=True):
Copy link
Member

Choose a reason for hiding this comment

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

callers of get_autoupdate_enabled should not be able to override the default.

the conf module is responsible for defining the defaults

return conf.get_switch("AutoUpdate.Enabled", default)


def get_autoupdate_frequency(conf=__conf__):
Expand Down Expand Up @@ -507,14 +507,15 @@ def get_auto_update_to_latest_version(conf=__conf__):
"""
If set to True, agent will update to the latest version
Copy link
Member

Choose a reason for hiding this comment

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

we should add unit tests that codify these rules: use all combinations of y/n/absent for both flags and check the return value of UpdateToLatestVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

NOTE:
when turned on, both AutoEnabled.Enabled and UpdateToLatestVersion same meaning: update to latest version
when turned off, AutoEnabled.Enabled: reverts to pre-installed agent, UpdateToLatestVersion: uses latest version installed on the vm
Since we are deprecating AutoEnabled.Enabled, we need to honor if existing flag explicitly set to 'n' for backward compatibility.
"""
auto_update_enabled = get_autoupdate_enabled()
if not auto_update_enabled:
return auto_update_enabled
return conf.get_switch("AutoUpdate.UpdateToLatestVersion", True)
when both turned on, both AutoUpdate.Enabled and AutoUpdate.UpdateToLatestVersion same meaning: update to latest version
when turned off, AutoUpdate.Enabled: reverts to pre-installed agent, AutoUpdate.UpdateToLatestVersion: uses latest version already installed on the vm and does not download new agents
Even we are deprecating AutoUpdate.Enabled, we still need to support if users explicitly setting it instead new flag.
If AutoUpdate.UpdateToLatestVersion is present, it overrides any value set for AutoUpdate.Enabled (if present).
If AutoUpdate.Enabled is present and set to 'n', we adhere to AutoUpdate.Enabled flag's behavior
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here as other PR, lines 513 and 514 seem to contradict each other to me

if both not present, we default to True.
"""
default = get_autoupdate_enabled()
return conf.get_switch("AutoUpdate.UpdateToLatestVersion", default)


def get_cgroup_check_period(conf=__conf__):
Expand Down
4 changes: 2 additions & 2 deletions azurelinuxagent/ga/update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,8 @@ def log_if_agent_versioning_feature_disabled():
log_if_op_disabled("AutoUpdate.Enabled", conf.get_autoupdate_enabled())
log_if_op_disabled("AutoUpdate.UpdateToLatestVersion", conf.get_auto_update_to_latest_version())

if not conf.get_autoupdate_enabled():
msg = "AutoUpdate.Enabled property is **Deprecated** but it was used and set to False. Please switch to using AutoUpdate.UpdateToLatestVersion instead."
if conf.get_autoupdate_enabled(default=None) is not None and conf.get_autoupdate_enabled() != conf.get_auto_update_to_latest_version():
msg = "AutoUpdate.Enabled property is **Deprecated** now but it's set to different value from AutoUpdate.UpdateToLatestVersion. Please consider removing it if added by mistake"
logger.warn(msg)
add_event(AGENT_NAME, op=WALAEventOperation.ConfigurationChange, message=msg)

Expand Down
4 changes: 4 additions & 0 deletions tests/common/test_conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class TestConf(AgentTestCase):
"OS.CheckRdmaDriver": False,
"AutoUpdate.Enabled": True,
"AutoUpdate.GAFamily": "Prod",
"AutoUpdate.UpdateToLatestVersion": False,
Copy link
Member

Choose a reason for hiding this comment

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

default should be true

"EnableOverProvisioning": True,
"OS.AllowHTTP": False,
"OS.EnableFirewall": False
Expand Down Expand Up @@ -144,3 +145,6 @@ def test_write_agent_disabled(self):

def test_get_extensions_enabled(self):
self.assertTrue(conf.get_extensions_enabled(self.conf))

def test_get_get_auto_update_to_latest_version(self):
self.assertFalse(conf.get_auto_update_to_latest_version(self.conf))
6 changes: 5 additions & 1 deletion tests/data/test_waagent.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,12 @@ OS.SshDir=/notareal/path
# OS.CheckRdmaDriver=n


# Enable or disable goal state processing auto-update, default is enabled.
# Deprecated now but keep it for backward compatibility
AutoUpdate.Enabled=y

# Enable or disable goal state processing auto-update, default is enabled
# AutoUpdate.Enabled=y
AutoUpdate.UpdateToLatestVersion=n

# Determine the update family, this should not be changed
# AutoUpdate.GAFamily=Prod
Expand Down
2 changes: 1 addition & 1 deletion tests/test_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
EXPECTED_CONFIGURATION = \
"""AutoUpdate.Enabled = True
AutoUpdate.GAFamily = Prod
AutoUpdate.UpdateToLatestVersion = True
AutoUpdate.UpdateToLatestVersion = False
Copy link
Member

Choose a reason for hiding this comment

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

default should be true

Autoupdate.Frequency = 3600
DVD.MountPoint = /mnt/cdrom/secure
Debug.AgentCpuQuota = 50
Expand Down
Loading