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

remove version suffix from extension slice #2782

Merged
merged 4 commits into from
Mar 13, 2023

Conversation

nagworld9
Copy link
Contributor

Description

Today we are creating extension_slice with version on it. Example
└─azure-vmextensions.slice
└─azure-vmextensions-Microsoft.Azure.Monitor.AzureMonitorLinuxAgent_1.25.1.slice
└─enable_c1f3b25b-93d5-4594-9a2d-ce5deee5536f.scope
└─3319627 /usr/bin/python3 /var/lib/waagent/Microsoft.Azure.Monitor.AzureMonitorLinuxAgent-1.25.1/agent.py -metrics

This makes it difficult for user to override the limits because they need to place drop-in files on every upgrade if extension slice is different for each version.

In this pr, I have updated logic and creating slice without version and will be same slice even extension updated to new version. We only modify it if the extension publishers change the limits. That way users don't need to create drop files again if they already done it for having custom settings.

Issue #


PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made

Quality of Code and Contribution Guidelines

maddieford
maddieford previously approved these changes Mar 8, 2023
@@ -455,6 +455,11 @@ def __create_all_files(files_to_create):

def is_extension_resource_limits_setup_completed(self, extension_name, cpu_quota=None):
unit_file_install_path = systemd.get_unit_file_install_path()
old_extension_slice_path = os.path.join(unit_file_install_path, SystemdCgroupsApi.get_extension_slice_name(extension_name, old_slice=True))
# clean up the old slice from the desk
Copy link
Member

Choose a reason for hiding this comment

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

typo: "desk"

def get_extension_slice_name(extension_name, old_slice=False):
# old slice includes <HandlerName>.<ExtensionName>-<HandlerVersion>
# new slice without version <HandlerName>.<ExtensionName>
if not old_slice:
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a comment that we renamed the slice and why for future reference?

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

narrieta
narrieta previously approved these changes Mar 9, 2023
@nagworld9 nagworld9 dismissed stale reviews from narrieta and maddieford via 1a73311 March 9, 2023 21:02
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #2782 (f2de055) into develop (355dd09) will decrease coverage by 0.01%.
The diff coverage is 77.77%.

@@             Coverage Diff             @@
##           develop    #2782      +/-   ##
===========================================
- Coverage    71.99%   71.98%   -0.01%     
===========================================
  Files          104      104              
  Lines        15844    15851       +7     
  Branches      2268     2271       +3     
===========================================
+ Hits         11407    11411       +4     
- Misses        3914     3915       +1     
- Partials       523      525       +2     
Impacted Files Coverage Δ
azurelinuxagent/common/cgroupconfigurator.py 72.10% <66.66%> (-0.10%) ⬇️
azurelinuxagent/common/cgroupapi.py 81.21% <100.00%> (+0.20%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@nagworld9 nagworld9 merged commit b584057 into Azure:develop Mar 13, 2023
@nagworld9 nagworld9 deleted the remove-version branch March 13, 2023 18:52
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

3 participants