Skip to content

fix(docker): fix set kernel perf event paranoid set #10635

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

Conversation

soyacz
Copy link
Contributor

@soyacz soyacz commented Apr 14, 2025

When using docker and setting print_kernel_callstack param we need to set kernel.perf_event_paranoid=0 - which didn't work due missing params and proper privileges for docker.

refs: #10633

Testing

  • - tested locally with docker backend

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

When using docker and setting `print_kernel_callstack` param we need to
set `kernel.perf_event_paranoid=0` - which didn't work due missing
params and proper privileges for docker.

refs: scylladb#10633
@soyacz soyacz added the backport/none Backport is not required label Apr 14, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • docker/scylla-sct/centos/Dockerfile: Language not supported
Comments suppressed due to low confidence (1)

sdcm/cluster_docker.py:59

  • Using bool() directly on the print_kernel_callstack parameter may lead to unexpected behavior if the parameter is provided as a non-boolean value (e.g., the string 'false'). Consider converting or validating the parameter to explicitly obtain a boolean value.
privileged = bool(self.parent_cluster.params.get('print_kernel_callstack'))

Copy link
Contributor

@yaronkaikov yaronkaikov left a comment

Choose a reason for hiding this comment

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

Thanks @soyacz

@yaronkaikov
Copy link
Contributor

@lsfreitas FYI

@soyacz
Copy link
Contributor Author

soyacz commented Apr 14, 2025

@lsfreitas FYI

this is not solving the original issue, just helps with mentioned error when setting this system setting.

@vponomaryov
Copy link
Contributor

vponomaryov commented Apr 14, 2025

@soyacz I don't think we should update the kernel.perf_event_paranoid host config option on docker backend.
With this change everyone running a docker SCT test will get this option changed on his/her machine.

I think we should update the related SCT config option instead:

print_kernel_callstack: false

@soyacz
Copy link
Contributor Author

soyacz commented Apr 15, 2025

@soyacz I don't think we should update the kernel.perf_event_paranoid host config option on docker backend. With this change everyone running a docker SCT test will get this option changed on his/her machine.

hmm, yes, that seems to be true. Better not change host settings.

I think we should update the related SCT config option instead:

print_kernel_callstack: false

so, I'd propose to disable it at code by adding and not self.is_docker() level in condition, as anyway it is not working and won't generate unrelevant errors. We could consider adding a warning that we skip it.:

if self.node_type == "db" and not self.is_kubernetes() \
                and self.parent_cluster.params.get("print_kernel_callstack"):

@soyacz
Copy link
Contributor Author

soyacz commented Apr 15, 2025

wrong approach. closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/none Backport is not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants