Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1185838 - Content View Puppet repo not deleted from puppet master
Summary: Content View Puppet repo not deleted from puppet master
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite 6
Classification: Red Hat
Component: Content Management
Version: 6.0.7
Hardware: Unspecified
OS: Unspecified
high
high vote
Target Milestone: 6.2
Assignee: Justin Sherrill
QA Contact: jcallaha
URL: http://projects.theforeman.org/issues...
Whiteboard:
Depends On:
Blocks: 1122832 1360463
TreeView+ depends on / blocked
 
Reported: 2015-01-26 11:55 UTC by Peter Vreman
Modified: 2018-12-06 20:45 UTC (History)
20 users (show)

Fixed In Version: katello-3.0.0-12
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1360463 (view as bug list)
Environment:
Last Closed: 2016-09-14 21:02:32 UTC
Target Upstream Version:


Attachments (Terms of Use)
foreman-debug attached (deleted)
2015-04-28 10:24 UTC, Tazim Kolhar
no flags Details
logs - production log (deleted)
2015-09-02 10:55 UTC, Tazim Kolhar
no flags Details


Links
System ID Priority Status Summary Last Updated
Pulp Redmine 732 High CLOSED - CURRENTRELEASE As a user, repo deletion causes puppet modules to be uninstalled Never
Red Hat Product Errata RHBA-2016:1885 normal SHIPPED_LIVE Satellite 6.2.2 bug fix update 2016-09-15 00:57:56 UTC
Foreman Issue Tracker 15845 None None None 2016-08-01 15:05:58 UTC
Red Hat Bugzilla 1182842 None None None 2019-04-11 10:41:12 UTC

Description Peter Vreman 2015-01-26 11:55:58 UTC
Description of problem:
When a Content View is removed from a lifecycle environment the published puppet repo is not deleted from the puppet master (/etc/puppet/environments directory)


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. Create Content View
2. Add Puppet module
3. Publish Content View
4. Remove Content View From Environment Library
5. List the /etc/puppet/environments directory on the puppet master(s)

Actual results:
/etc/puppet/environments still contains a directory for the Content View

Expected results:
Directory of the Content View is deleted from /etc/puppet/environments

Additional info:
Tested only with the internal capsule running on the Sat6 host. This might also affect remote capsules.

Comment 1 RHEL Product and Program Management 2015-01-26 12:03:32 UTC
Since this issue was entered in Red Hat Bugzilla, the release flag has been
set to ? to ensure that it is properly evaluated for this release.

Comment 3 Justin Sherrill 2015-03-05 03:05:21 UTC
It appears the pulp install distributor is not cleaning up after itself.

Comment 6 Michael Hrivnak 2015-03-05 14:55:09 UTC
This was deliberate, but we're happy to change this behavior. Since the distributor is actually installing content, it seems like a user could be surprised to discover than deleting a repository in pulp has caused puppet modules to be uninstalled.

That said, katello is likely the only user of this feature, so if you think it will always be desirable to uninstall the puppet modules on repo delete, we can make that happen.

Comment 7 Justin Sherrill 2015-03-05 15:00:24 UTC
Michael, 

Yes, i think for our use case that is desirable.  An option on the distributor or something would be fine if you did not want to change the default behavior.  putting back on needinfo to open (or link) an upstream issue as per new process.

Thanks!

-Justin

Comment 8 Peter Vreman 2015-03-06 09:46:48 UTC
Michael,

At least Katello has ownership of all directories matching the regex:
'^KT_.+[0-9]+#'

Alternative that is more robust:

Manage all distributed files under /var/lib/pulp/published/puppet (this directory is current not used)

$ find /var/lib/pulp/published/puppet -type f
$

Then use symlinks in the /etc/puppet/environment directory.

Regards,
Peter

Comment 9 Brian Bouterse 2015-03-23 19:30:25 UTC
The Pulp upstream bug status is at ASSIGNED. Updating the external tracker on this bug.

Comment 10 Brian Bouterse 2015-03-24 23:00:24 UTC
The Pulp upstream bug status is at POST. Updating the external tracker on this bug.

Comment 11 Brian Bouterse 2015-03-25 14:00:20 UTC
The Pulp upstream bug status is at MODIFIED. Updating the external tracker on this bug.

Comment 12 Mike McCune 2015-03-25 17:07:53 UTC
Upstream commit:

https://pulp.plan.io/issues/732#note-9

Comment 15 Tazim Kolhar 2015-04-28 10:21:51 UTC
FAILEDQA:

# rpm -qa | grep foreman
foreman-libvirt-1.7.2.17-1.el6_6sat.noarch
ruby193-rubygem-foreman_bootdisk-4.0.2.10-1.el6_6sat.noarch
ruby193-rubygem-foreman_hooks-0.3.7-2.el6_6sat.noarch
rubygem-hammer_cli_foreman_tasks-0.0.3.3-1.el6_6sat.noarch
rubygem-hammer_cli_foreman_bootdisk-0.1.2.5-1.el6_6sat.noarch
foreman-postgresql-1.7.2.17-1.el6_6sat.noarch
foreman-debug-1.7.2.17-1.el6_6sat.noarch
foreman-1.7.2.17-1.el6_6sat.noarch
foreman-ovirt-1.7.2.17-1.el6_6sat.noarch
ruby193-rubygem-foreman-tasks-0.6.12.3-1.el6_6sat.noarch
foreman-proxy-1.7.2.4-1.el6_6sat.noarch
qe-sat6-rhel66.usersys.redhat.com-foreman-client-1.0-1.noarch
qe-sat6-rhel66.usersys.redhat.com-foreman-proxy-client-1.0-1.noarch
foreman-selinux-1.7.2.13-1.el6_6sat.noarch
rubygem-hammer_cli_foreman-0.1.4.9-1.el6_6sat.noarch
foreman-compute-1.7.2.17-1.el6_6sat.noarch
foreman-vmware-1.7.2.17-1.el6_6sat.noarch
ruby193-rubygem-foreman-redhat_access-0.1.0-1.el6_6sat.noarch
ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el6_6sat.noarch
qe-sat6-rhel66.usersys.redhat.com-foreman-proxy-1.0-2.noarch
ruby193-rubygem-foreman_docker-1.2.0.9-1.el6_6sat.noarch
rubygem-hammer_cli_foreman_discovery-0.0.1.7-1.el6_6sat.noarch
foreman-gce-1.7.2.17-1.el6_6sat.noarch
ruby193-rubygem-foreman_discovery-2.0.0.9-1.el6_6sat.noarch

steps:

1. Create Content View named (con_viewB)
2. Add Puppet module
3. Publish Content View
4. Remove Content View From Environment Library
5. List the /etc/puppet/environments directory on the puppet master(s)

# ls
example_env
KT_Default_Organization_Library_con_viewB_5

foreman-debug attached

Comment 16 Tazim Kolhar 2015-04-28 10:24:08 UTC
Created attachment 1019614 [details]
foreman-debug attached

Comment 17 Justin Sherrill 2015-05-05 15:46:26 UTC
Michael,  Could you work to verify if all the needed commits made it to the appropriate builds downstream?

Comment 18 Michael Hrivnak 2015-05-10 22:17:30 UTC
That looks like the correct commit.

It would be helpful to confirm which pulp build was tested.

When operating correctly, at the time the repository is deleted, you should see this message in the system log at the INFO level: "removing installed modules from environment at..."

Does step 4 definitely delete the repository? Is it possible that the deletion took some time, or the system was busy with other tasks, and so it just hadn't been completed yet at the time step 5 was done?

Comment 19 Justin Sherrill 2015-05-13 18:45:04 UTC
Tazim, could you address Michael's questions?

Comment 20 Justin Sherrill 2015-05-28 13:43:32 UTC
Michael,

I reproduced this and noticed that the modules do get deleted:

[root@qe-sat6-rhel71 ~]# ls /etc/puppet/environments/KT_Default_Organization_Library_jsherrill_test_459/modules
[root@qe-sat6-rhel71 ~]#

but the actual environment directory does not.  This is better but I'm not sure why the directory shouldn't be deleted (as an empty environment isn't of much use).

As you said I do see:

removing installed modules from environment at /etc/puppet/environments/KT_Default_Organization_Library_jsherrill_test_459/modules

Comment 21 Michael Hrivnak 2015-06-08 22:05:50 UTC
Looks like a simple oversight. Do you need a patch against pulp 2.6? How soon?

Comment 22 Justin Sherrill 2015-06-08 22:38:53 UTC
Yes to a pulp 2.6 patch, but just needed in Sat 6.1.1

Comment 23 pulp-infra@redhat.com 2015-06-11 17:31:38 UTC
The Pulp upstream bug status is at POST. Updating the external tracker on this bug.

Comment 24 Michael Hrivnak 2015-06-12 13:04:59 UTC
Additional patch: https://github.com/pulp/pulp_puppet/pull/187

Comment 25 pulp-infra@redhat.com 2015-06-12 13:30:32 UTC
The Pulp upstream bug status is at MODIFIED. Updating the external tracker on this bug.

Comment 26 pulp-infra@redhat.com 2015-06-15 21:00:35 UTC
The Pulp upstream bug status is at ON_QA. Updating the external tracker on this bug.

Comment 27 Justin Sherrill 2015-06-19 17:12:11 UTC
Moving to POST as the upstream fix was merged:  https://github.com/pulp/pulp_puppet/pull/187

Comment 29 Bryan Kearney 2015-06-26 14:32:32 UTC
Delivered in Snap10

Comment 30 jnikolak 2015-06-30 02:34:16 UTC
if the puppet module in the content_view is changed instead of deleted, will this also reflect the new filenames in /etc/puppet/environments?

Comment 31 Tazim Kolhar 2015-07-07 09:26:38 UTC
FAILEDQA:

# rpm -qa | grep foreman
ruby193-rubygem-foreman-tasks-0.6.12.8-1.el7sat.noarch
rubygem-hammer_cli_foreman_docker-0.0.3.9-1.el7sat.noarch
foreman-debug-1.7.2.29-1.el7sat.noarch
foreman-postgresql-1.7.2.29-1.el7sat.noarch
foreman-vmware-1.7.2.29-1.el7sat.noarch
rubygem-hammer_cli_foreman_bootdisk-0.1.2.7-1.el7sat.noarch
foreman-selinux-1.7.2.13-1.el7sat.noarch
foreman-1.7.2.29-1.el7sat.noarch
foreman-ovirt-1.7.2.29-1.el7sat.noarch
ruby193-rubygem-foreman_hooks-0.3.7-2.el7sat.noarch
rubygem-hammer_cli_foreman_discovery-0.0.1.10-1.el7sat.noarch
foreman-proxy-1.7.2.5-1.el7sat.noarch
ibm-x3655-03.ovirt.rhts.eng.bos.redhat.com-foreman-proxy-1.0-2.noarch
foreman-compute-1.7.2.29-1.el7sat.noarch
foreman-gce-1.7.2.29-1.el7sat.noarch
ruby193-rubygem-foreman-redhat_access-0.2.0-8.el7sat.noarch
rubygem-hammer_cli_foreman-0.1.4.14-1.el7sat.noarch
foreman-libvirt-1.7.2.29-1.el7sat.noarch
ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el7sat.noarch
ibm-x3655-03.ovirt.rhts.eng.bos.redhat.com-foreman-client-1.0-1.noarch
ibm-x3655-03.ovirt.rhts.eng.bos.redhat.com-foreman-proxy-client-1.0-1.noarch
ruby193-rubygem-foreman_bootdisk-4.0.2.13-1.el7sat.noarch
ruby193-rubygem-foreman_docker-1.2.0.18-1.el7sat.noarch
rubygem-hammer_cli_foreman_tasks-0.0.3.4-1.el7sat.noarch
ruby193-rubygem-foreman_discovery-2.0.0.15-1.el7sat.noarch


steps:
1. Create Content View named (con_viewB)
2. Add Puppet module
3. Publish Content View
4. Remove Content View From Environment Library
5. List the /etc/puppet/environments directory on the puppet master(s)

# cd /etc/puppet/environments
[root@ibm-x3655-03 environments]# ls
example_env
KT_Default_Organization_Library_con_viewD_6

Comment 32 Michael Hrivnak 2015-07-14 17:20:38 UTC
Please provide the version of pulp used for verification, and also the system log contents seen during the test.

And just so I understand the test, were you expecting "KT_Default_Organization_Library_con_viewD_6" to get deleted? Does that correspond to "con_viewB"? How do you know? This may be a very basic question, but I'm not familiar with the naming scheme used by katello.

Comment 35 Tazim Kolhar 2015-09-02 10:54:05 UTC
Hi,

  I verified it again with sat6.1.2 snap1 

  the steps:
   
   1. Create Content View named (con_viewB)
   2. Add Puppet module
   3. Publish Content View
   4. Remove Content View From Environment Library
   5. List the /etc/puppet/environments directory on the puppet master(s)

   # ls
   example_env
   KT_Default_Organization_Library_con_viewA_2
   KT_Default_Organization_Library_con_viewB_3

   Here, I expected KT_Default_Organization_Library_con_viewB_3
   to be deleted since, I removed the same from UI

   the pulp version :
   # rpm -qa | grep pulp
   python-isodate-0.5.0-4.pulp.el7sat.noarch
   pulp-docker-plugins-0.2.5-1.el7sat.noarch
   python-kombu-3.0.24-10.pulp.el7sat.noarch
   python-pulp-docker-common-0.2.5-1.el7sat.noarch
   pulp-puppet-tools-2.6.0.15-1.el7sat.noarch
   pulp-server-2.6.0.15-1.el7sat.noarch
   pulp-nodes-parent-2.6.0.15-1.el7sat.noarch
   python-pulp-common-2.6.0.15-1.el7sat.noarch
   python-pulp-rpm-common-2.6.0.15-1.el7sat.noarch
   python-pulp-bindings-2.6.0.15-1.el7sat.noarch
   pulp-rpm-plugins-2.6.0.15-1.el7sat.noarch
   pulp-katello-0.5-1.el7sat.noarch
   python-pulp-puppet-common-2.6.0.15-1.el7sat.noarch
   rubygem-smart_proxy_pulp-1.0.1.2-1.el7sat.noarch
   pulp-selinux-2.6.0.15-1.el7sat.noarch
   pulp-puppet-plugins-2.6.0.15-1.el7sat.noarch
   pulp-nodes-common-2.6.0.15-1.el7sat.noarch

   with system log I assume you mean production.log so I am attaching the same
   please let me know if any other log file is what you meant
   thanks


Thanks and Regards,
Tazim

Comment 36 Tazim Kolhar 2015-09-02 10:55:33 UTC
Created attachment 1069360 [details]
logs - production log

Comment 38 Tazim Kolhar 2015-10-19 10:52:48 UTC
*** This bug is failing in upstream ****.

FAILEDQA:

# rpm -qa | grep foreman
nec-em17.rhts.eng.bos.redhat.com-foreman-client-1.0-1.noarch
foreman-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-hammer_cli_foreman_docker-0.0.3-4.el7.noarch
nec-em17.rhts.eng.bos.redhat.com-foreman-proxy-client-1.0-1.noarch
tfm-rubygem-hammer_cli_foreman-0.4.0-1.201510071112git33fd59b.el7.noarch
foreman-debug-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-release-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-postgresql-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-vmware-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-foreman_hooks-0.3.9-1.el7.noarch
tfm-rubygem-foreman-tasks-0.7.6-1.fm1_10.el7.noarch
tfm-rubygem-hammer_cli_foreman_tasks-0.0.8-1.el7.noarch
tfm-rubygem-foreman_bootdisk-6.0.0-2.fm1_10.el7.noarch
foreman-release-scl-1-1.el7.x86_64
foreman-libvirt-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-selinux-1.11.0-0.develop.201510071426git6234447.el7.noarch
foreman-ovirt-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
tfm-rubygem-hammer_cli_foreman_bootdisk-0.1.3-3.el7.noarch
tfm-rubygem-foreman_gutterball-0.0.1-3.el7.noarch
nec-em17.rhts.eng.bos.redhat.com-foreman-proxy-1.0-2.noarch
tfm-rubygem-foreman_discovery-4.1.0-1.fm1_10.el7.noarch
tfm-rubygem-foreman_docker-1.4.1-2.fm1_10.el7.noarch
foreman-proxy-1.11.0-0.develop.201510120849git5f36f2e.el7.noarch
foreman-compute-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch
foreman-gce-1.11.0-0.develop.201510121538gitb6b977a.el7.noarch

steps:
   1. Create Content View named (con_view1)
   2. Add Puppet module
   3. Publish Content View
   4. Remove Content View From Environment Library
   5. List the /etc/puppet/environments directory on the puppet master(s)

# cd /etc/puppet/environments/
[root@nec-em17 environments]# ls
example_env  KT_Default_Organization_Library_con_view1_3  production

here, KT_Default_Organization_Library_con_view1_3 should be deleted

Comment 41 pulp-infra@redhat.com 2015-11-17 21:30:54 UTC
The Pulp upstream bug status is at CLOSED - CURRENTRELEASE. Updating the external tracker on this bug.

Comment 42 Justin Sherrill 2015-12-02 12:02:13 UTC
Yes, if the corresponding content view is no longer present, those are safe to delete

Comment 43 Bryan Kearney 2016-01-04 15:58:12 UTC
This is fixed in Pulp 2.7. Moving to POST, please verify when a 2.7+ build is generated.

Comment 44 Peter 2016-01-08 07:05:13 UTC
The similar issue:
I have removed all of my puppet modules but have remnants of the puppet classes in katello and not sure why.  The modules,  repos and content views that they were related to have all been deleted but they are still showing as options.
See the following output.  I only expect ID 1 to be listed. All of the others have been removed via the 
# hammer puppet-module list
---|------|--------|--------
ID | NAME | AUTHOR | VERSION
---|------|--------|--------

# hammer puppet-class list
---|-------------------------------
ID | NAME
---|-------------------------------
1  | access_insights_client
10 | auditd
9  | auditd::auditd_rules
7  | auditd::disable_immutable_mode
8  | auditd::enable_immutable_mode
11 | auditd::sync
15 | cis_benchmark
13 | cis_benchmark::auditd_rules
16 | cis_benchmark::modprobe_lines
12 | cis_benchmark::sysctl_settings
2  | concat::setup
5  | modprobe
6  | modprobe::modprobe_lines
3  | stdlib
4  | stdlib::stages
14 | test_cis_benchmark
---|-------------------------------

After doing a bit of digging the modules are found in /var/lib/pulp/content/puppet_module.

Comment 47 Tazim Kolhar 2016-03-31 13:12:45 UTC
FAILEDQA:
# rpm -qa | grep foreman
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-client-1.0-1.noarch
foreman-debug-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman-tasks-0.7.14.1-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_docker-0.0.4-1.el6sat.noarch
tfm-rubygem-foreman_bootdisk-6.1.0-1.el6sat.noarch
tfm-rubygem-foreman_gutterball-0.0.1-6.el6sat.noarch
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-proxy-client-1.0-1.noarch
foreman-vmware-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman_hooks-0.3.9-2.el6sat.noarch
foreman-postgresql-1.11.0.9-1.el6sat.noarch
foreman-selinux-1.11.0-1.el6sat.noarch
foreman-installer-katello-3.0.0.14-1.el6sat.noarch
foreman-discovery-image-3.1.1-2.el7sat.noarch
foreman-ovirt-1.11.0.9-1.el6sat.noarch
foreman-libvirt-1.11.0.9-1.el6sat.noarch
foreman-gce-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman_docker-2.0.1.2-1.el6sat.noarch
foreman-1.11.0.9-1.el6sat.noarch
foreman-compute-1.11.0.9-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_tasks-0.0.10-2.el6sat.noarch
tfm-rubygem-foreman_discovery-5.0.0.3-1.el6sat.noarch
tfm-rubygem-foreman-redhat_access-1.0.1-2.el6sat.noarch
foreman-proxy-1.11.0.2-1.el6sat.noarch
foreman-installer-1.11.0.0-1.el6sat.noarch
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-proxy-1.0-1.noarch
puppet-foreman_scap_client-0.3.3-10.el6.noarch
tfm-rubygem-foreman_remote_execution-0.3.0.2-1.el6sat.noarch
tfm-rubygem-foreman_openscap-0.5.3.3-1.el6sat.noarch
tfm-rubygem-foreman_theme_satellite-0.1.3-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman-0.5.1.3-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_bootdisk-0.1.3-4.el6sat.noarch



Steps:
1. Create Content View
2. Add Puppet module
3. Publish Content View
4. Remove Content View From Environment Library
5. List the /etc/puppet/environments directory on the puppet master(s)

/etc/puppet/environments
# ls
example_env  KT_Default_Organization_Library_con_view3_4

here, KT_Default_Organization_Library_con_view3_4 should be deleted

Comment 48 Tazim Kolhar 2016-03-31 13:13:07 UTC
FAILEDQA:
# rpm -qa | grep foreman
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-client-1.0-1.noarch
foreman-debug-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman-tasks-0.7.14.1-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_docker-0.0.4-1.el6sat.noarch
tfm-rubygem-foreman_bootdisk-6.1.0-1.el6sat.noarch
tfm-rubygem-foreman_gutterball-0.0.1-6.el6sat.noarch
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-proxy-client-1.0-1.noarch
foreman-vmware-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman_hooks-0.3.9-2.el6sat.noarch
foreman-postgresql-1.11.0.9-1.el6sat.noarch
foreman-selinux-1.11.0-1.el6sat.noarch
foreman-installer-katello-3.0.0.14-1.el6sat.noarch
foreman-discovery-image-3.1.1-2.el7sat.noarch
foreman-ovirt-1.11.0.9-1.el6sat.noarch
foreman-libvirt-1.11.0.9-1.el6sat.noarch
foreman-gce-1.11.0.9-1.el6sat.noarch
tfm-rubygem-foreman_docker-2.0.1.2-1.el6sat.noarch
foreman-1.11.0.9-1.el6sat.noarch
foreman-compute-1.11.0.9-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_tasks-0.0.10-2.el6sat.noarch
tfm-rubygem-foreman_discovery-5.0.0.3-1.el6sat.noarch
tfm-rubygem-foreman-redhat_access-1.0.1-2.el6sat.noarch
foreman-proxy-1.11.0.2-1.el6sat.noarch
foreman-installer-1.11.0.0-1.el6sat.noarch
dell-pe-sc1435-02.rhts.englab.brq.redhat.com-foreman-proxy-1.0-1.noarch
puppet-foreman_scap_client-0.3.3-10.el6.noarch
tfm-rubygem-foreman_remote_execution-0.3.0.2-1.el6sat.noarch
tfm-rubygem-foreman_openscap-0.5.3.3-1.el6sat.noarch
tfm-rubygem-foreman_theme_satellite-0.1.3-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman-0.5.1.3-1.el6sat.noarch
tfm-rubygem-hammer_cli_foreman_bootdisk-0.1.3-4.el6sat.noarch



Steps:
1. Create Content View
2. Add Puppet module
3. Publish Content View
4. Remove Content View From Environment Library
5. List the /etc/puppet/environments directory on the puppet master(s)

/etc/puppet/environments
# ls
example_env  KT_Default_Organization_Library_con_view3_4

here, KT_Default_Organization_Library_con_view3_4 should be deleted

Comment 49 Michael Hrivnak 2016-04-06 19:57:16 UTC
For bugs fixed in pulp, it's very helpful to know the version of pulp being tested. I'm not sure how to determine that based on the version of foreman that's installed.

When a content view gets removed, do the repositories in it get deleted by katello? In case not, that would explain this.

During repo delete, there should be a log message like this from pulp: "removing installed modules from environment at <the path>"

and then if there's an error, there will be a follow-up logged at the warn level: "error removing environment: <reason>"

Seeing the values of both of those log statements would be very helpful.

Comment 51 Justin Sherrill 2016-05-11 13:48:11 UTC
Michael

Yes the repo is being deleted by katello:

Here are the logs:

May 11 13:42:36 satellite pulp: pulp_puppet.plugins.distributors.installdistributor:INFO: removing installed modules from environment at /etc/puppet/environments/KT_Summit2016_Library_tester_5/modules
May 11 13:42:36 satellite pulp: celery.worker.job:INFO: Task pulp.server.tasks.repository.delete[aea61a00-a456-4333-979d-b8d62e40110c] succeeded in 0.314634217s: <pulp.server.async.tasks.TaskResult object at 0x3f59f50>

Yet the directory remains: 

drwxr-xr-x 2 apache apache 6 May 11 13:42 /etc/puppet/environments/KT_Summit2016_Library_tester_5

The directory is empty though.  So we are left with an empty directory which is not expected since pulp has created that directory.  This is very similar to comment #20 but /etc/puppet/environments/KT_Summit2016_Library_tester_5/modules is deleted but not /etc/puppet/environments/KT_Summit2016_Library_tester_5

pulp-client-1.0-1.noarch
pulp-docker-plugins-2.0.0.2-1.el7sat.noarch
pulp-katello-1.0-3.el7sat.noarch
pulp-puppet-plugins-2.8.1-2.el7sat.noarch
pulp-puppet-tools-2.8.1-2.el7sat.noarch
pulp-rpm-plugins-2.8.1.3-1.el7sat.noarch
pulp-selinux-2.8.1.3-1.el7sat.noarch
pulp-server-2.8.1.3-1.el7sat.noarch
python-pulp-common-2.8.1.3-1.el7sat.noarch
python-pulp-docker-common-2.0.0.2-1.el7sat.noarch
python-pulp-oid_validation-2.8.1.3-1.el7sat.noarch
python-pulp-puppet-common-2.8.1-2.el7sat.noarch
python-pulp-repoauth-2.8.1.3-1.el7sat.noarch
python-pulp-rpm-common-2.8.1.3-1.el7sat.noarch
python-pulp-streamer-2.8.1.3-1.el7sat.noarch
rubygem-smart_proxy_pulp-1.2.0-1.el7sat.noarch

Comment 52 Michael Hrivnak 2016-05-12 13:43:49 UTC
I think I see what's going on.

The "install_path" was set to "/etc/puppet/environments/KT_Summit2016_Library_tester_5/modules".

On first publish, pulp noticed that "KT_Summit2016_Library_tester_5" did not exist. So it created that directory, and the "modules" subdirectory before extracting units into it.

Pulp had no idea that "KT_Summit2016_Library_tester_5" was a special directory it owns forever, or that the name happens to be related to the repo id. All pulp knew is that it should create as many subdirectories as required to publish at the install_path. Pulp did not save a record of which directories it did or did not create.

When the repo was removed, during cleanup, pulp removed the "modules" directory and of course all of the modules underneath it. We can reasonably assume it is always safe to remove the most-sub-of-subdirectories. But what are the deletion rules pulp should follow walking up the directory tree? That could get risky.

Can pulp assume because it created a directory when this repo was published, now it can delete it? Perhaps only if it is empty? Would that always be reasonable?

Comment 53 Justin Sherrill 2016-05-12 13:54:11 UTC
Can pulp assume because it created a directory when this repo was published, now it can delete it? Perhaps only if it is empty? Would that always be reasonable?


For our purposes yes.  We're intending for pulp to create that install_path and delete it as well.  I think its sane to say after deleting the modules directory, go ahead and delete the install_path if its empty.

Comment 54 Michael Hrivnak 2016-05-12 14:03:27 UTC
Ok. I think pulp will need to remember which directories it created (by saving that in the DB), and only consider deleting those. We would not (I think) want pulp to try deleting /etc/puppet/environments itself even if it is empty, right?

There would also be potential for order of repo deletion to matter. Consider two repos, A and B.

A's install_path is /etc/puppet/environments/favorites/foo/modules/
B's install_path is /etc/puppet/environments/favorites/bar/modules/

If you publish A first, it will create the "favorites" directory and remember as such.

If you later delete A before deleting B, A will see the "favorites" directory is not empty, and leave it in-place. When you delete B, it won't try to delete "favorites", because B's distributor didn't create it.

Make sense? Is that acceptable?

Another option in theory would be to "mark" each directory as being created by pulp, for example maybe with an empty file called ".created_by_pulp". Then pulp could always know it's safe to delete those dirs if they are otherwise empty. Do you think that's better?

Comment 55 Peter Vreman 2016-05-12 14:11:42 UTC
My 2 cents:

The application that decides to use '/etc/puppet/environments/KT_favorites' shall also keep track of it.

You cannot rely that other applications how to handle directories outside of their scope. That is complex and error prone.

That means if Katello decides to that /etc/puppet/environments/KT_favorites has to be used then it in fact must both create also delete it.

I also recommend that Pulp would only create the only the last subdir of install_path and rely that the rest of the path exists. When that is implemented it implicity means that Katello has to take care of the rest of the path.

Alternative: Have Pulp manage complete environment subdirs and not only the modules directory.

Comment 56 Michael Hrivnak 2016-05-12 14:21:28 UTC
I also like that idea. Katello, or any other app using pulp in this way, has the knowledge of when it is appropriate to delete a directory. Pulp can only make a reasonable guess that errs on the side of caution.

Comment 57 Justin Sherrill 2016-05-12 14:33:42 UTC
That would be great, except we have no way to do that on the capsule.  We need pulp to create that directory for us. 

I think i also maybe see some confusion.  It sounds like pulp takes whatever directory we specify:

/etc/puppet/environments/foo/bar/zed

and creates any parent that is needed.  Our expectation is that the directory we specify:

/etc/puppet/environments/foo/  

is the only directory that needs creation.  /etc/puppet/environments is assumed to already exist and be writeable by pulp.  Because the exact install path is the only directory we expect to be created and the fact it will always be created, for us it is perfectly safe to delete.  That directory is assumed (in a way) to be 'owned' by pulp and nothing outside of pulp is going to touch it.

Now i understand that there may be other use cases that differ from Satellite and Katello's expectations, but i wanted to clarify those expectations.

Comment 58 Michael Hrivnak 2016-05-12 17:35:04 UTC
From what I see on this issue, it looks like the install path being specified by katello is:

/etc/puppet/environments/$SOME_IDENTIFIER/modules/

So I think katello is specifying two directories deep within /etc/puppet/environments/. Is that correct?

I don't think the "modules" directory has any special meaning to pulp. Does it have a special meaning to puppet? Does it need to exist at all?

Comment 59 Justin Sherrill 2016-05-12 17:41:00 UTC
Ah yeah you are right, as pulp isn't publishing with the modules directory itself.  We are specifying it.  

I'm open to suggestions, the thought of pulp 'going up' the tree and deleting directories seems pretty dirty.  Any ideas?

Comment 61 Michael Hrivnak 2016-06-13 13:02:49 UTC
The only consequence of this bug is a small number of empty directories. Assuming we need to address that somehow...

Perhaps katello could have a cron job to periodically look for empty directories where the mtime is sufficiently in the past that it's obviously been orphaned, and remove if so? It would be wise to log each directory being removed to the system log just in case there was a race-condition conflict with another operation.

Is the "modules" directory required? Or could you remove it from the path? If it could be omitted, that would solve the problem.

Comment 62 Justin Sherrill 2016-07-26 18:38:24 UTC
Michael,

After some discussion with the katello team, we're okay adding a simple cronjob as a temporary solution, but would be curious about your thoughts about a future solution.


If we were to go back and design this again, we'd probably insist that this distributor be designed not as a generic 'stick module HERE', but instead as a 'install modules to puppet master' distributor.  If that were the case we would have likely suggested that the distributor deploy to $DEPLOY_DIR/modules/  rather than $DEPLOY_DIR.

Since we didn't do that, what would you be your thoughts on adding an option to the puppet install distributor to do the above, as well as clean up $DEPLOY_DIR/modules if the option is set to true.  This would provide the functionality we need as well as providing backwards compatibility.

Comment 63 Michael Hrivnak 2016-07-26 19:43:05 UTC
That sounds reasonable. I imagine we'd add a setting called something like "create_modules_dir", that would have it create that directory during deployment, and clean it up on removal. Would you mind filing a story about it in our redmine?

Comment 64 Justin Sherrill 2016-07-26 19:56:52 UTC
Michael,

Sounds good.  I will clone this to the backlog and open a pulp issue.

Comment 65 Justin Sherrill 2016-08-01 14:28:12 UTC
Connecting redmine issue http://projects.theforeman.org/issues/15845 from this bug

Comment 66 Bryan Kearney 2016-08-01 16:05:15 UTC
Upstream bug component is Content Management

Comment 67 jcallaha 2016-09-09 18:36:02 UTC
Verified in Satellite 6.2.2 Snap 1.1

After removing the content view from Library, the directory isn't immediately deleted. The cleanup will be carried out by the cron job located here: /etc/cron.weekly/katello-clean-empty-puppet-environments
Alternatively, manually running the script will also cleanup the empty directories.

Comment 68 Edu Alcaniz 2016-09-12 11:37:42 UTC
Hi, could we get backport/hotfix for Satellite 6.1.9?

Comment 69 Justin Sherrill 2016-09-12 16:31:26 UTC
Edu,

The fix for 6.2 is simply using this cronjob:

#!/bin/bash
find /etc/puppet/environments/KT* -type d -empty -delete

dropped into /etc/cron.weekly/.

You could do the same on 6.1 and get the same result (or drop into cron.daily or cron.hourly).

We could likely work on an official hotfix if its still needed

-Justin

Comment 72 errata-xmlrpc 2016-09-14 21:02:32 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2016:1885


Note You need to log in before you can comment on or make changes to this bug.