-
Notifications
You must be signed in to change notification settings - Fork 59
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
(maint) Add testing around exec via Puppet #583
Conversation
CLA signed by all contributors. |
Depends on puppetlabs/puppet#5844 being fixed first. |
13f92ef
to
6b741b6
Compare
Should be ready to go with the latest build of puppet-agent. |
message.data = message_data.to_json | ||
|
||
message_expiry = 10 # Seconds for the PCP message to be considered failed | ||
message.expires(message_expiry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we get rid of the message_expiry
variable altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's no longer needed, so we could remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tempted to ignore it for now though.
|
||
# Also halt the sleep process here, as on Windows the parent process won't exit while the child process | ||
# has an open handle shared with it (the stdout pipe). | ||
stop_sleep_process(agents) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agents
-> agent
otherwise there is an attempt to kill the sleep process on all agents on every iteration through the agents
array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense.
PE-14681 and MODULES-3125 identified issues running commands via Puppet exec from other service agents. These issues led to PUP-6675: selinux prevents processes from writing to files in /tmp if they don't have a reason to do so. The Puppet service selinux policy appears to exempt this, but other services that invoke Puppet - such as pxp-agent - aren't configured that way and cause silent exec failures. PUP-6675 fixes this issue. We add an acceptance test to ensure that it doesn't recur in pxp-agent. [skip ci]
The test around Puppet starting after a previous Puppet run has completed previously relied on Windows behavior where the Puppet process would exit completely even though a child process is still running. PUP-6675 changes Puppet to use pipes for stdout from invoking a child process; if Puppet is terminated while the child process is still running, it will still have a valid process handle until the child process exits (and closes the shared pipe). This is explained in https://msdn.microsoft.com/en-us/library/windows/desktop/ms686714(v=vs.85).aspx. Puppet terminating while a direct child process is still running is not normal behavior, so modify the test to not rely on it. [skip ci]
With pcp-broker 1.0, we removed the ability to address multiple targets. Acceptance test code still relied on it, so that a run with more than one agent configured would fail. Update the code to send individual messages for each target so it works again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
create_remote_file(master, site_manifest, <<-SITEPP) | ||
node default { | ||
exec { "hostname": | ||
path => ["/bin", "/usr/bin", "C:/cygwin32/bin", "C:/cygwin64/bin"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikaelSmith this is failing CI when running 32-bit puppet/ruby on 32-bit windows (using the win-10-ent-i386
template), because the path needs to be C:/cygwin/bin
. I think the C:/cygwin32
would only happen when using 32-bit cygwin on 64-bit windows, which shouldn't happen in our current set of windows templates.
Note the puppet-agent pipelines alternate architectures, which is why it was failing yesterday, but not today: https://jenkins-master-prod-1.delivery.puppetlabs.net/view/puppet-agent/view/master/view/Suite/job/platform_puppet-agent_intn-van-sys_suite-daily-pxp-agent-master/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PE-14681 and MODULES-3125 identified issues running commands via Puppet
exec from other service agents. These issues led to PUP-6675: selinux
prevents processes from writing to files in /tmp if they don't have a
reason to do so. The Puppet service selinux policy appears to exempt
this, but other services that invoke Puppet - such as pxp-agent - aren't
configured that way and cause silent exec failures.
PUP-6675 fixes this issue. We add an acceptance test to ensure that it
doesn't recur in pxp-agent.