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

Update healthchecks to handle ldd failures #1137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adfoster-r7
Copy link

@adfoster-r7 adfoster-r7 commented Nov 22, 2023

Description

Relates to #1056 which added more rigorous checks to the ldd healthchecks


The scenario is - I've just upgraded to a newer version of Omnibus, and it looks like the ldd command here can break halfway through with this current pattern. It took a while to track down things as the status code here is getting ignored, so the error was being swallowed.

Example, reading 3 files and the 2nd ELF file causes an error - shows that the third file is never evaluated - leading to incorrect health checks:

[9] pry(#<Omnibus::HealthCheck>)> puts ldd_output = shellout(ldd_command, input: "/opt/project/file1\n/opt/project/file2\n/opt/project/file3").stdout
/opt/project/file1:
	not a dynamic executable
/opt/project/file2:
=> nil

We get a partial result in the current implementation, but if you extract the status code you can see it has failed an exitstatus of 135:

[11] pry(#<Omnibus::HealthCheck>)> puts ldd_output = shellout(ldd_command, input: "/opt/project/file1\n/opt/project/file2\n/opt/project/file3").result
NoMethodError: undefined method `result' for <Mixlib::ShellOut#1230: command: 'xargs ldd' process_status: #<Process::Status: pid 46708 exit 123> stdout: '/opt/project/file1:
	not a dynamic executable
/opt/project/file2:' stderr: 'ldd: exited with unknown exit code (135)' child_pid: 46708 environment: {} timeout: 7200 user:  group:  working_dir:  >:Mixlib::ShellOut
from (pry):11:in `block in read_shared_libs'

For my current setup, this new code path skips multiple healthchecks as a result of exiting earlier than expect

Intended solution

For the happy path, the ldd command is run against all libraries as before. If the exit code is non-zero, we try the failed files indivdually.

Maintainers

Please ensure that you check for:

  • If this change impacts git cache validity, it bumps the git cache
    serial number
  • If this change impacts compatibility with omnibus-software, the
    corresponding change is reviewed and there is a release plan
  • If this change impacts compatibility with the omnibus cookbook, the
    corresponding change is reviewed and there is a release plan

@adfoster-r7 adfoster-r7 requested review from a team as code owners November 22, 2023 00:58
@adfoster-r7 adfoster-r7 force-pushed the update-healthchecks-to-handle-ldd-failures branch 3 times, most recently from 6c218ea to 825b958 Compare November 22, 2023 01:06
Copy link

sonarcloud bot commented Nov 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
No Duplication information No Duplication information

@adfoster-r7 adfoster-r7 force-pushed the update-healthchecks-to-handle-ldd-failures branch from 825b958 to 66ce785 Compare January 11, 2024 11:02
Copy link

sonarcloud bot commented Jan 11, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@adfoster-r7 adfoster-r7 force-pushed the update-healthchecks-to-handle-ldd-failures branch from 66ce785 to feb28e3 Compare April 19, 2024 11:48
Copy link

sonarcloud bot commented Apr 19, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

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.

1 participant