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

(BKR-474) Add missing shellescape #74

Closed
wants to merge 1 commit into from

Conversation

mcanevet
Copy link
Member

@mcanevet mcanevet commented Sep 1, 2015

No description provided.

mcanevet referenced this pull request Sep 1, 2015
...through hosts

- found that beaker was unable to complete its acceptance tests when
  provided with multiple SUTs of multiple os types in the same hosts
  file
- need to correctly detectos when we switch to a new os, otherwise we
  assume that we are executing on the same os and generate the wrong
  command strings
- keeps a hash of known hosts associated with their os type so that we
  don't have to run detect_os (which can be somewhat time expensive)
  more than once per-SUT
@puppetlabs-jenkins
Copy link
Contributor

Can one of the admins verify this patch?

@DavidS
Copy link
Contributor

DavidS commented Sep 2, 2015

I've got a acceptance test that is failing on the original error, and this patch only changes the error I get.

Using the original line from before a6b3e50 works for me.

@rick
Copy link

rick commented Sep 2, 2015

@DavidS is the acceptance test linkable?

@DavidS
Copy link
Contributor

DavidS commented Sep 2, 2015

It's the puppetlabs-apache acceptance test suite, which uses serverspec and therefore hits this.

@rick
Copy link

rick commented Sep 2, 2015

From @DavidS -- here's the build output from that run, with various failures caused by this problem:

https://jenkins-modules.puppetlabs.com/job/forge-module_puppetlabs-apache_intn-sys_smoke-master/PLATFORM=ubuntu-1404-64mda,WORKER_LABEL=beaker/3/console

and the suite in question: https://github.com/puppetlabs/puppetlabs-apache/tree/00b0da75cbe2a11a8577b87cb635d0c04440db10/spec/acceptance

It seems like we should supplement our in-tool acceptance testing (probably we can use that suite above as a guide) here, since we know our suite isn't detecting the problem...

@justinstoller
Copy link
Contributor

@rick @DavidS I believe this QENG-2889

@rick
Copy link

rick commented Sep 2, 2015

@rick @DavidS I believe this QENG-2889

Meaning, this is what's causing QENG-2889?

@justinstoller
Copy link
Contributor

Yeah, sorry. In complete sentences this time.

I'm in a meeting with @DavidS and from our discussion, I thought this was possibly the root cause of QENG-2889. On further review of the apache logs and comparing them to failures for concat, inifile, & ntp, I think this is a similar (failing when sending a command with a space in it) but different issue with QENG-2889 only affecting Solaris.

Sorry, for the noise.

@DavidS
Copy link
Contributor

DavidS commented Sep 2, 2015

Since the jankins-modules is not public, here the relevant log snippet:

    File "/etc/apache2/apache2.conf"

pxs25p4f40hrvzw.delivery.puppetlabs.net (ubuntu-1404-agent) 11:27:29$ /bin/sh -c "test -f /etc/apache2/apache2.conf"

pxs25p4f40hrvzw.delivery.puppetlabs.net (ubuntu-1404-agent) executed in 0.01 seconds
      should be file

pxs25p4f40hrvzw.delivery.puppetlabs.net (ubuntu-1404-agent) 11:27:29$ /bin/sh -c "grep -qs -- ServerRoot\ \"/tmp/root\" /etc/apache2/apache2.conf || grep -qFs -- ServerRoot\ \"/tmp/root\" /etc/apache2/apache2.conf"

pxs25p4f40hrvzw.delivery.puppetlabs.net (ubuntu-1404-agent) executed in 0.01 seconds
Exited: 1
      should contain "ServerRoot \"/tmp/root\"" (FAILED - 1)

compare this to the last good run:

https://jenkins-modules.puppetlabs.com/job/forge-module_puppetlabs-apache_intn-sys_smoke-master/PLATFORM=ubuntu-1404-64mda,WORKER_LABEL=beaker/1/consoleFull

    File "/etc/apache2/apache2.conf"

n8m7f3oqnwn5ceh.delivery.puppetlabs.net (ubuntu-1404-agent) 11:36:51$ /bin/sh -c test\ -f\ /etc/apache2/apache2.conf

n8m7f3oqnwn5ceh.delivery.puppetlabs.net (ubuntu-1404-agent) executed in 0.01 seconds
      should be file

n8m7f3oqnwn5ceh.delivery.puppetlabs.net (ubuntu-1404-agent) 11:36:51$ /bin/sh -c grep\ -qs\ --\ ServerRoot\\\ \\\"/tmp/root\\\"\ /etc/apache2/apache2.conf\ \|\|\ grep\ -qFs\ --\ ServerRoot\\\ \\\"/tmp/root\\\"\ /etc/apache2/apache2.conf

n8m7f3oqnwn5ceh.delivery.puppetlabs.net (ubuntu-1404-agent) executed in 0.01 seconds
      should contain "ServerRoot \"/tmp/root\""

@electrical
Copy link

For some reason i'm getting the following error when implementing this fix:

  28) elasticsearch class: module removal Service "elasticsearch-es-02" should not be running
      Failure/Error: it { should_not be_running }
      TypeError:
        can't convert Symbol into Integer

      # ./.vendor/ruby/1.9.1/gems/specinfra-2.43.0/lib/specinfra/command_factory.rb:26:in `[]'
      # ./.vendor/ruby/1.9.1/gems/specinfra-2.43.0/lib/specinfra/command_factory.rb:26:in `create_command_class'
      # ./.vendor/ruby/1.9.1/gems/specinfra-2.43.0/lib/specinfra/command_factory.rb:16:in `get'
      # ./.vendor/ruby/1.9.1/gems/specinfra-2.43.0/lib/specinfra/processor.rb:4:in `check_service_is_running'
      # ./.vendor/ruby/1.9.1/bundler/gems/beaker-rspec-49c45f6102fe/lib/beaker-rspec/helpers/serverspec.rb:132:in `method_missing'
      # ./.vendor/ruby/1.9.1/gems/serverspec-2.23.0/lib/serverspec/type/service.rb:24:in `running?'
      # ./.vendor/ruby/1.9.1/gems/serverspec-2.23.0/lib/serverspec/matcher/be_running.rb:4:in `block (2 levels) in <top (required)>'
      # ./spec/acceptance/002_class_spec.rb:144:in `block (4 levels) in <top (required)>'

Any idea's ?

@DavidS
Copy link
Contributor

DavidS commented Sep 3, 2015

@electrical Yeah that is the error I was referring to in #74 (comment) - reverting the line to what was there before works for Linux, but fails on Windows, so it's not the whole solution.

@mcanevet
Copy link
Member Author

mcanevet commented Sep 4, 2015

Fixed by #75

@mcanevet mcanevet closed this Sep 4, 2015
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.

6 participants