-
Notifications
You must be signed in to change notification settings - Fork 372
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
Clean up old daemon cgroups #1595
Conversation
@@ -69,6 +69,7 @@ def run(self, child_args=None): | |||
self.initialize_environment() | |||
|
|||
CGroupConfigurator.get_instance().create_agent_cgroups(track_cgroups=False) | |||
CGroupConfigurator.get_instance().cleanup_old_cgroups() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the cleanup is just temporary we should move the logic to create_agent_cgroups instead of calling a separate function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favour of keeping it separate to maintain one responsibility per function and to make the testing easier. The removal logic stays the same wherever the call happens.
|
||
contents = fileutil.read_file(os.path.join(old_path, "cgroup.procs")) | ||
|
||
if daemon_pid in contents: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep it to make sure that we're only writing the daemon PID if it's not already there (by explicitly checking it's still in the old cgroup).
Description
Daemons from version 2.2.31 to 2.2.40 added their PID to a now-obsolete cgroup
WALinuxAgent/WALinuxAgent
. Add logic to move that PID to the new cgroup,walinuxagent.service
for each controller to ensure we track both the daemon and extension handler process.PR information
Quality of Code and Contribution Guidelines
This change is