Skip to content

[sos-spyre] collect spyre users podman disk free details#38

Open
sahithiRavindranath wants to merge 1 commit intolinux-ras:spyrefrom
sahithiRavindranath:sos-spyre
Open

[sos-spyre] collect spyre users podman disk free details#38
sahithiRavindranath wants to merge 1 commit intolinux-ras:spyrefrom
sahithiRavindranath:sos-spyre

Conversation

@sahithiRavindranath
Copy link

issue:
podman disk free data was not collected for non root users.

solution:
collecting the podman disk free data for all non root spyre users(part of sentient group)

for user in users:
# since root user outputs are collected in podman plugin
# skipping here
if not user or user == 'root':
Copy link
Contributor

Choose a reason for hiding this comment

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

User may not have updated podman plugin that collect podman system df [-v]. So the above assumption may not hold true for all user.

So run this for root user always.

Copy link
Author

@sahithiRavindranath sahithiRavindranath Feb 17, 2026

Choose a reason for hiding this comment

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

removed root exception.

# get unique list of users
users = list(set(users))

for user in users:
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the code to find non-root users of sentient group to a helper function.

Choose a reason for hiding this comment

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

moved

return spyre_cards_bus_ids

def get_spyre_usernames(self):

Copy link
Contributor

Choose a reason for hiding this comment

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

What is this newline for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use static Linters to find basic issue in the code. I recommend pylint.

Choose a reason for hiding this comment

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

checked

groupname = "sentient"
spyre_users = self.exec_cmd("getent group "+ groupname)
if spyre_users['status'] == 0 and spyre_users['output'].strip():

Copy link
Contributor

Choose a reason for hiding this comment

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

Why we have an empty new line just after if condition?

Choose a reason for hiding this comment

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

removed


users = []
# All spyre users must be part of sentient group
groupname = "sentient"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we really need groupname variable. It is only used at one place.

Choose a reason for hiding this comment

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

corrected


def get_spyre_usernames(self):

users = []
Copy link
Contributor

Choose a reason for hiding this comment

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

We have two list variables in this function users and spyre_users.

The function scoped list variable should be named as spyre_users and while extracting the users from output lines users name can be used.

Choose a reason for hiding this comment

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

corrected

if u.strip()
]
# get unique list of users
users = list(set(users))
Copy link
Contributor

Choose a reason for hiding this comment

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

You are making this function unnecessarily complicated. It is not only very inefficient but also hard to review.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about keeping things simple:

"""
get_spyre_username(): Get the names of users belonging to
the sentient group.

Currently, all users belonging to the sentient group are
considered Spyre card users.

Args:
None

Returns:
List of spyre card users
"""
def get_spyre_username():
spyre_users = []
getent_cmd_output = self.exec_cmd("getent group sentient")

# Multiple lines are not expected but trying to be safe
for line in getent_cmd_output.splitlines():
    line = line.strip().split(':')

    # Expected line format
    # group_name:x:group_id:user1,user2
    # No user at this line
    if len(line) < 4:
        continue

    for user in line[3].split(','):
        if user not in sentinet_users:
              sentient_users.add(user)

return sentient_users

Choose a reason for hiding this comment

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

fixed all

Previously, podman disk usage data was collected only for the root user.

This fix collects the podman disk usage for all non root spyre users(part
of sentient group) as well.

Signed-off-by: Sahithi Ravindranath <Sahithi.Ravindranath@ibm.com>
])

sentient_users = self.get_spyre_users()
print(sentient_users)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you forget to remove debug prints?

"/etc/security/limits.d/memlock.conf",
])

sentient_users = self.get_spyre_users()
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable name should be spyre_users.


sentient_users = self.get_spyre_users()
print(sentient_users)
for user in sentient_users:
Copy link
Contributor

Choose a reason for hiding this comment

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

What if root is not part of sentient group?

We should always run these commands for root user regardless.

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.

2 participants

Comments