Skip to content

Commit ddc1af8

Browse files
adfoster-r7adfoster-r7
committed
Update failing checks for chef#1056 
1 parent 4a1754f commit ddc1af8

File tree

2 files changed

+141
-21
lines changed

2 files changed

+141
-21
lines changed

‎lib/omnibus/health_check.rb‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -511,16 +511,40 @@ def read_shared_libs(find_command, ldd_command, &output_proc)
511511
# feed the list of files to the "ldd" command
512512
#
513513

514-
# this command will typically fail if the last file isn't a valid lib/binary which happens often
515-
ldd_output = shellout(ldd_command, input: find_output.join).stdout
514+
# instance Mixlib::ShellOut
515+
ldd_cmd = shellout(ldd_command, input: find_output.join)
516+
517+
# Optimized path: Attempt to run the `ldd` command on all file paths. If it succeeds, then process
518+
# the stdout result in bulk. If the command returned a non-zero exit status code, then something went wrong.
519+
# Each path will have to be manually resolved
520+
unless ldd_cmd.error?
521+
# do the output process to determine if the files are good or bad
522+
ldd_cmd.stdout.each_line do |line|
523+
output_proc.call(line)
524+
end
525+
else
526+
log.debug(log_key) { "Failed running #{ldd_command} with exit status #{ldd_cmd.exitstatus} when resolving individually" }
516527

517-
#
518-
# do the output process to determine if the files are good or bad
519-
#
528+
# Verify each path separately
529+
find_output.each do |path|
530+
ldd_result = shellout(ldd_command, input: path)
531+
if ldd_result.error?
532+
log.debug(log_key) { "Failed running #{ldd_command} with exit status #{ldd_cmd.exitstatus} against: #{path}" }
533+
end
520534

521-
ldd_output.each_line do |line|
522-
output_proc.call(line)
535+
ldd_output = ldd_result.stdout
536+
537+
# Yield the path first
538+
output_proc.call("#{path.rstrip}:")
539+
540+
# do the output process to determine if the files are good or bad
541+
ldd_output.each_line do |line|
542+
output_proc.call(line)
543+
end
544+
end
523545
end
546+
547+
nil
524548
end
525549

526550
#

‎spec/unit/health_check_spec.rb‎

Lines changed: 110 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,26 @@ def mkdump(base, size, x64 = false)
2929

3030
context "on mac_os_x" do
3131
before do
32-
stub_ohai(platform: "mac_os_x", version: "10.13")
32+
stub_ohai(platform: "mac_os_x", version: "10.14")
33+
end
34+
35+
# file_list just needs to have one file which is inside of the install_dir
36+
let(:file_list) do
37+
double("Mixlib::Shellout",
38+
error!: false,
39+
error?: false,
40+
stdout: <<~EOH
41+
/opt/chefdk/shouldnt/matter
42+
EOH
43+
)
3344
end
3445

3546
let(:absolute_links) do
3647
double("Mixlib::Shellout",
48+
error!: false,
49+
error?: false,
3750
stdout: <<~EOH
38-
/opt/chefdk/embedded/bin/ruby:
51+
/opt/chefdk/shouldnt/matter:
3952
/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 57337.60.9)
4053
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1259.32.0)
4154
/opt/chefdk/embedded/lib/libruby.2.6.dylib (compatibility version 2.6.0, current version 2.6.6)
@@ -48,8 +61,10 @@ def mkdump(base, size, x64 = false)
4861

4962
let(:portable_links) do
5063
double("Mixlib::Shellout",
64+
error!: false,
65+
error?: false,
5166
stdout: <<~EOH
52-
/opt/chefdk/embedded/bin/ruby:
67+
/opt/chefdk/shouldnt/matter:
5368
/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 57337.60.9)
5469
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1259.32.0)
5570
@executable_path/../lib/libruby.2.6.dylib (compatibility version 2.6.0, current version 2.6.6)
@@ -62,8 +77,10 @@ def mkdump(base, size, x64 = false)
6277

6378
let(:unknown_absolute_link) do
6479
double("Mixlib::Shellout",
80+
error!: false,
81+
error?: false,
6582
stdout: <<~EOH
66-
/opt/chefdk/embedded/bin/ruby:
83+
/opt/chefdk/shouldnt/matter:
6784
/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 57337.60.9)
6885
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1259.32.0)
6986
/opt/chefdk/embedded/lib/libruby.2.6.dylib (compatibility version 2.6.0, current version 2.6.6)
@@ -80,24 +97,36 @@ def mkdump(base, size, x64 = false)
8097

8198
it "raises an exception when there are absolute links" do
8299
allow(subject).to receive(:shellout)
83-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
100+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
101+
.and_return(file_list)
102+
103+
allow(subject).to receive(:shellout)
104+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
84105
.and_return(absolute_links)
85106

86107
expect { subject.run! }.to raise_error(HealthCheckFailed)
87108
end
88109

89110
it "does not raise an exception when links are portable" do
90111
allow(subject).to receive(:shellout)
91-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
92-
.and_return(portable_links)
112+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
113+
.and_return(file_list)
114+
115+
allow(subject).to receive(:shellout)
116+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
117+
.and_return(portable_links)
93118

94119
expect { subject.run! }.to_not raise_error
95120
end
96121

97122
it "raises an exception when there are unknown absolute links" do
98123
allow(subject).to receive(:shellout)
99-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
100-
.and_return(unknown_absolute_link)
124+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
125+
.and_return(file_list)
126+
127+
allow(subject).to receive(:shellout)
128+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
129+
.and_return(unknown_absolute_link)
101130

102131
expect { subject.run! }.to raise_error(HealthCheckFailed)
103132
end
@@ -108,24 +137,37 @@ def mkdump(base, size, x64 = false)
108137

109138
it "does not raise an exception when there are absolute links" do
110139
allow(subject).to receive(:shellout)
111-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
140+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
141+
.and_return(file_list)
142+
143+
allow(subject).to receive(:shellout)
144+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
112145
.and_return(absolute_links)
113146

147+
114148
expect { subject.run! }.to_not raise_error
115149
end
116150

117151
it "raises an exception when links are portable" do
118152
allow(subject).to receive(:shellout)
119-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
120-
.and_return(portable_links)
153+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
154+
.and_return(file_list)
155+
156+
allow(subject).to receive(:shellout)
157+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
158+
.and_return(portable_links)
121159

122160
expect { subject.run! }.to raise_error(HealthCheckFailed)
123161
end
124162

125163
it "raises an exception when there are unknown absolute links" do
126164
allow(subject).to receive(:shellout)
127-
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$' | xargs otool -L")
128-
.and_return(unknown_absolute_link)
165+
.with("find #{project.install_dir}/ -type f | egrep '\.(dylib|bundle)$'")
166+
.and_return(file_list)
167+
168+
allow(subject).to receive(:shellout)
169+
.with("xargs otool -L", { input: "/opt/chefdk/shouldnt/matter\n" })
170+
.and_return(unknown_absolute_link)
129171

130172
expect { subject.run! }.to raise_error(HealthCheckFailed)
131173
end
@@ -211,15 +253,28 @@ def mkdump(base, size, x64 = false)
211253
let(:file_list) do
212254
double("Mixlib::Shellout",
213255
error!: false,
256+
error?: false,
214257
stdout: <<~EOH
215258
/opt/chefdk/shouldnt/matter
216259
EOH
217260
)
218261
end
219262

263+
let(:file_list_multiple) do
264+
double("Mixlib::Shellout",
265+
error!: false,
266+
error?: false,
267+
stdout: <<~EOH
268+
/opt/chefdk/first
269+
/opt/chefdk/second
270+
EOH
271+
)
272+
end
273+
220274
let(:empty_list) do
221275
double("Mixlib::Shellout",
222276
error!: false,
277+
error?: false,
223278
stdout: <<~EOH
224279
EOH
225280
)
@@ -238,6 +293,7 @@ def mkdump(base, size, x64 = false)
238293
let(:bad_list) do
239294
double("Mixlib::Shellout",
240295
error!: false,
296+
error?: false,
241297
stdout: <<~EOH
242298
/somewhere/other/than/install/dir
243299
EOH
@@ -247,6 +303,7 @@ def mkdump(base, size, x64 = false)
247303
let(:bad_healthcheck) do
248304
double("Mixlib::Shellout",
249305
error!: false,
306+
error?: false,
250307
stdout: <<~EOH
251308
/bin/ls:
252309
linux-vdso.so.1 => (0x00007fff583ff000)
@@ -269,6 +326,7 @@ def mkdump(base, size, x64 = false)
269326
let(:good_healthcheck) do
270327
double("Mixlib::Shellout",
271328
error!: false,
329+
error?: false,
272330
stdout: <<~EOH
273331
/bin/echo:
274332
linux-vdso.so.1 => (0x00007fff8a6ee000)
@@ -282,6 +340,17 @@ def mkdump(base, size, x64 = false)
282340
)
283341
end
284342

343+
let(:bad_exitstatus_healthcheck) do
344+
double("Mixlib::Shellout",
345+
error!: -> { raise Mixlib::ShellOut::ShellCommandFailed },
346+
error?: true,
347+
exitstatus: 135,
348+
stdout: <<~EOH
349+
/bin/echo:
350+
EOH
351+
)
352+
end
353+
285354
it "raises an exception when there are external dependencies" do
286355
allow(subject).to receive(:shellout)
287356
.with("find /opt/chefdk/ -type f | xargs file | grep \"ELF\" | awk -F: '{print $1}' | sed -e 's/:$//'")
@@ -306,6 +375,33 @@ def mkdump(base, size, x64 = false)
306375
expect { subject.run! }.to_not raise_error
307376
end
308377

378+
it "does not raise an exception if the initial batch ldd lookup fails" do
379+
allow(subject).to receive(:shellout)
380+
.with("find /opt/chefdk/ -type f | xargs file | grep \"ELF\" | awk -F: '{print $1}' | sed -e 's/:$//'")
381+
.and_return(file_list_multiple)
382+
383+
# Bulk ldd command fails
384+
allow(subject).to receive(:shellout)
385+
.with("xargs ldd", { input: "/opt/chefdk/first\n/opt/chefdk/second\n" })
386+
.and_return(bad_exitstatus_healthcheck)
387+
388+
# First file ldd fails
389+
allow(subject).to receive(:shellout)
390+
.with("xargs ldd", { input: "/opt/chefdk/first\n" })
391+
.and_return(bad_exitstatus_healthcheck)
392+
393+
# Second file lld succeeds
394+
allow(subject).to receive(:shellout)
395+
.with("xargs ldd", { input: "/opt/chefdk/second\n" })
396+
.and_return(good_healthcheck)
397+
398+
output = capture_logging do
399+
expect { subject.run! }.to_not raise_error
400+
end
401+
expect(output).to match(%r{Failed running xargs ldd with exit status 135 when resolving individually})
402+
expect(output).to match(%r{Failed running xargs ldd with exit status 135 against: /opt/chefdk/first})
403+
end
404+
309405
it "will not perform dll base relocation checks" do
310406
expect(subject.relocation_checkable?).to be false
311407
end

0 commit comments

Comments
 (0)