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

(MODULES-10589) Exit early when puppet.list config file has been modified #472

Merged
merged 1 commit into from Mar 17, 2020

Conversation

beechtom
Copy link
Contributor

@beechtom beechtom commented Mar 3, 2020

This updates the install_shell task to check whether the configuration
file /etc/apt/repos.list.d/puppet.list has been modified prior to
installing the puppet-release repo.

Previously, if a user had modified the file (such as by manually
maintaining the repo), dpkg would prompt the user whether they wanted
to keep the old config file or replace it with the maintainer's config
file. This caused the script to stall as it waited for user input. While
dpkg can answer these prompts non-interactively using the
--force-conf* options, this behavior would be undesirable as it could
result in the script making a critical decision for the user. Instead,
the script will provide a helpful warning that the config file has been
modified and that agent installation will be skipped.

Closes puppetlabs/bolt#1593

local result=$?
local file="/etc/apt/sources.list.d/puppet.list"

if [ -f "$file" ] && [ "$result" -eq 0 ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The file check is here since the previous command will report that the file was modified if it doesn't exist. If the file doesn't exist, then there won't be a conflict with a manually maintained file, so we should go ahead and install the repo and agent.

dpkg -i "$2"
assert_unmodified_apt_config

dpkg -i --force-confmiss "$2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--force-confmiss will install any missing config files.

@beechtom beechtom requested review from m0dular and a team March 3, 2020 20:16
@puppetcla
Copy link

CLA signed by all contributors.

@puppetcla
Copy link

Waiting for CLA signature by @m0dular

@m0dular - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppet.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppet.com/community/trivial_patch_exemption.html

@m0dular
Copy link

m0dular commented Mar 6, 2020

I updated my account but haven't receive the cla email ¯_(ツ)_/¯

@puppetcla
Copy link

CLA signed by all contributors.

@lucywyman
Copy link
Contributor

@beechtom I think you need to update the commit message to adhere to contributing guidelines:

$ bundle exec rake $CHECK
Checking commits e85f549f0877387bce97372f137b53fbef60ea12..6f0d3480096eea365763e071c71be0c576cf20ff
rake aborted!

	This commit summary didn't match CONTRIBUTING.md guidelines:
		Only check for puppet.list in the puppet-release package
	The commit summary (i.e. the first line of the commit message) should start with one of:
		(MODULES-<digits>) # this is most common and should be a ticket at tickets.puppet.com
		(docs)
		(docs)(DOCUMENT-<digits>)
		(packaging)

We might need to make a modules ticket for this :/

@beechtom beechtom changed the title (maint) Exit early when puppet.list config file has been modified (MODULES-10589) Exit early when puppet.list config file has been modified Mar 12, 2020
@beechtom beechtom requested a review from lucywyman March 12, 2020 16:23
@@ -35,6 +35,25 @@ exists() {
fi
}

# Check whether the apt config file has been modified, warning and exiting early if it has
assert_unmodified_apt_config() {
list_file=/etc/apt/sources.list.d/puppet.list
Copy link
Contributor

Choose a reason for hiding this comment

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

With the latest puppet docs this would actually be puppet6.list - we should definitely look for both. I don't think dpkg will let a user have both at the same time, so just checking for whichever exists is sufficient until someone runs into issues with both existing.

…fied

This updates the `install_shell` task to check whether the configuration
file `/etc/apt/repos.list.d/puppet.list' has been modified prior to
installing the `puppet-release` repo.

Previously, if a user had modified the file (such as by manually
maintaining the repo), `dpkg` would prompt the user whether they wanted
to keep the old config file or replace it with the maintainer's config
file. This caused the script to stall as it waited for user input. While
`dpkg` can answer these prompts non-interactively using the
`--force-conf*` options, this behavior would be undesirable as it could
result in the script making a critical decision for the user. Instead,
the script will provide a helpful warning that the config file has been
modified and that agent installation will be skipped.
@donoghuc
Copy link
Member

Dont want to be bug but https://jenkins-master-prod-1.delivery.puppetlabs.net/view/Bolt-bundled-content/job/forge-module_puppetlabs-puppet-agent-module_intn-run_task_acceptance-master/ are still failing. Started with #471 but that is not a full solution. Is getting CI green a priority before merging new task content?

@lucywyman lucywyman merged commit 444ca84 into puppetlabs:master Mar 17, 2020
@beechtom beechtom deleted the maint-debian-unattended branch March 17, 2020 22:00
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.

The task puppet_agent::install doesn't configure apt to run in unattended mode
5 participants