add and remove extension slice #2315
Conversation
nagworld9
requested review from
dhivyaganesan,
kevinclark19a,
larohra,
narrieta and
ZhidongPeng
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #2315 +/- ##
===========================================
+ Coverage 70.66% 70.69% +0.02%
===========================================
Files 97 97
Lines 13988 14017 +29
Branches 2006 2010 +4
===========================================
+ Hits 9885 9909 +24
- Misses 3656 3660 +4
- Partials 447 448 +1
Continue to review full report at Codecov.
|
azurelinuxagent/common/cgroupapi.py
Outdated
| # Since '-' is used as a separator in systemd unit names, we replace it with '_' to prevent side-effects. | ||
| return extension_name.replace('-', '_') | ||
| return "azure-vmextensions-" + extension_name.replace('-', '_') |
There was a problem hiding this comment.
let's use CGroupConfigurator._VMEXTENSIONS_SLICE instead
There was a problem hiding this comment.
It has .slice suffix which we don't want.
There was a problem hiding this comment.
let's declare another constant in the same place (minus ".slice") or remove ".slice" here
There was a problem hiding this comment.
This is creating cycling dependency issue.. I have created constant in the cgroupapi Module.
azurelinuxagent/common/cgroupapi.py
Outdated
| @@ -67,9 +67,9 @@ def track_cgroups(extension_cgroups): | |||
| "Error: {1}".format(cgroup.path, ustr(exception))) | |||
|
|
|||
| @staticmethod | |||
| def _get_extension_cgroup_name(extension_name): | |||
| def get_extension_cgroup_name(extension_name): | |||
There was a problem hiding this comment.
the slice names are particular to systemd, so let's make this change in SystemdCgroupsApi instead of CGroupsApi
| @@ -48,6 +48,14 @@ | |||
| [Slice] | |||
| CPUAccounting=yes | |||
| """ | |||
| _EXTENSION_SLICE_CONTENTS = """ | |||
| [Unit] | |||
| Description=Slice for Azure VM Publisher Extension | |||
There was a problem hiding this comment.
Let's remove "Publishers" and add the name of the extension
azurelinuxagent/ga/exthandlers.py
Outdated
| @@ -1503,6 +1506,8 @@ def uninstall(self, extension=None): | |||
| uninstall_cmd = man.get_uninstall_command() | |||
| self.logger.info("Uninstall extension [{0}]".format(uninstall_cmd)) | |||
| self.launch_command(uninstall_cmd, extension=extension) | |||
| CGroupConfigurator.get_instance().remove_extension_slice( | |||
There was a problem hiding this comment.
the cleanup of extensions files is done in remove_ext_handler below, we should move this call there
nagworld9
force-pushed
the
cgroup-ext
branch
4 times, most recently
from
8f682de
to
19c09ea
Compare
azurelinuxagent/common/cgroupapi.py
Outdated
| @@ -36,7 +36,7 @@ | |||
|
|
|||
| CGROUPS_FILE_SYSTEM_ROOT = '/sys/fs/cgroup' | |||
| CGROUP_CONTROLLERS = ["cpu", "memory"] | |||
|
|
|||
| _EXTENSION_SLICE_PREFIX = "azure-vmextensions-" | |||
There was a problem hiding this comment.
Thanks for adding this constant. The idea of using CGroupConfigurator._VMEXTENSIONS_SLICE was that if we ever need to update the name, the change should be done in 1 single place. Could you define CGroupConfigurator._VMEXTENSIONS_SLICE in terms of _EXTENSION_SLICE_PREFIX?
Description
Issue # TASK 10370976
PR information
Quality of Code and Contribution Guidelines