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

Add UDTF that detects linux kernel header installation and add column to GetAgentStatus #2052

Merged

Conversation

ddelnano
Copy link
Member

@ddelnano ddelnano commented Dec 2, 2024

Summary: Add UDTF that detects linux kernel header installation and add column to GetAgentStatus

This is a prerequisite to accomplish #2051. The px deploy command uses the GetAgentStatus UDTF in its final healthcheck step. With this kernel header detection in place, the px cli can use the results from the px/agent_status script to print a warning message if kernel headers aren't detected.

The helm install flow needs to be covered as well. My hope is that this UDTF could be used for that use case as well, but I need to further investigate the details of that.

Relevant Issues: #2051

Type of change: /kind feature

Test Plan: Skaffolded to a Ubuntu GKE cluster and tested the following

  • Kelvin always reports false as it doesn't bind mount / to /host
  • PEM running on host without linux-headers-$(uname -r) package reports false
  • PEM running on host with linux-headers-$(uname -r) package reports true
$ gcloud compute ssh gke-dev-cluster-ddelnano-default-pool-a27c1ac2-x5k2 --internal-ip -- 'ls -alh /lib/modules/$(uname -r)/build'

lrwxrwxrwx 1 root root 38 Aug  9 15:25 /lib/modules/5.15.0-1065-gke/build -> /usr/src/linux-headers-5.15.0-1065-gke

$ gcloud compute ssh gke-dev-cluster-ddelnano-default-pool-a27c1ac2-j6pg --internal-ip -- 'ls -alh /lib/modules/$(uname -r)/build'

ls: cannot access '/lib/modules/5.15.0-1065-gke/build': No such file or directory

Screen Shot 2024-12-02 at 9 30 29 AM

Changelog Message: Add GetLinuxHeadersStatus UDTF and add kernel_headers_installed column to GetAgentStatus

@ddelnano ddelnano requested review from a team as code owners December 2, 2024 17:40
@ddelnano ddelnano force-pushed the ddelnano/add-kernel-headers-installed-md-udtf branch from 779cdf4 to 4aca78b Compare December 2, 2024 17:41
Comment on lines 75 to 84
auto kernel_headers_installed = false;
auto uname = px::system::GetUname();
if (uname.ok()) {
const auto host_path = px::system::Config::GetInstance().ToHostPath(absl::Substitute("$0/$1/$2", kLinuxHeadersPath, uname.ConsumeValueOrDie(), "build"));

const auto resolved_host_path = px::system::ResolvePossibleSymlinkToHostPath(host_path);
kernel_headers_installed = resolved_host_path.ok();
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This duplicates a portion of the work done by BCCWrapper. Another option would be to move that header installation here rather than checking for the presence of the headers. That wouldn't suffer from the test drawbacks outlined here.

…ude kernel_headers_installed

Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
@ddelnano ddelnano force-pushed the ddelnano/add-kernel-headers-installed-md-udtf branch from 6caf3fe to 312fc86 Compare December 2, 2024 18:26
@ddelnano
Copy link
Member Author

ddelnano commented Dec 4, 2024

@pixie-io/maintainers this is ready for review when you have the chance!

src/vizier/services/agent/pem/pem_manager.h Outdated Show resolved Hide resolved
src/vizier/services/agent/pem/pem_main.cc Outdated Show resolved Hide resolved
@ddelnano ddelnano merged commit a95d661 into pixie-io:main Dec 11, 2024
35 checks passed
@ddelnano ddelnano deleted the ddelnano/add-kernel-headers-installed-md-udtf branch December 11, 2024 18:40
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.

3 participants