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

(maint) Add testing around exec via Puppet #583

Merged
merged 3 commits into from
Jun 29, 2017

Conversation

MikaelSmith
Copy link
Contributor

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.

@puppetcla
Copy link

CLA signed by all contributors.

@MikaelSmith
Copy link
Contributor Author

Depends on puppetlabs/puppet#5844 being fixed first.

@MikaelSmith MikaelSmith force-pushed the PE-14681 branch 2 times, most recently from 13f92ef to 6b741b6 Compare May 8, 2017 22:47
@MikaelSmith MikaelSmith changed the base branch from stable to master May 10, 2017 21:15
@MikaelSmith
Copy link
Contributor Author

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)
Copy link
Contributor

@mruzicka mruzicka Jun 29, 2017

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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)
Copy link
Contributor

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

Copy link
Contributor Author

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.
Copy link
Contributor

@mruzicka mruzicka left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@mruzicka mruzicka left a comment

Choose a reason for hiding this comment

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

👍

@mruzicka mruzicka merged commit bdc0318 into puppetlabs:master Jun 29, 2017
@MikaelSmith MikaelSmith deleted the PE-14681 branch June 29, 2017 20:24
create_remote_file(master, site_manifest, <<-SITEPP)
node default {
exec { "hostname":
path => ["/bin", "/usr/bin", "C:/cygwin32/bin", "C:/cygwin64/bin"],
Copy link
Contributor

@joshcooper joshcooper Jul 8, 2017

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/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

4 participants