Bug 1026473 - When upgrading bundle with compliance=full deployment folder still contains empty directories not found in bundle
When upgrading bundle with compliance=full deployment folder still contains e...
Status: CLOSED CURRENTRELEASE
Product: JBoss Operations Network
Classification: JBoss
Component: Provisioning (Show other bugs)
JON 3.2
Unspecified Unspecified
unspecified Severity low
: DR02
: JON 3.2.1
Assigned To: Libor Zoubek
Mike Foley
: Triaged
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-04 13:03 EST by Libor Zoubek
Modified: 2015-11-01 19:43 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-05-08 13:44:10 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
bundle version1 (1.48 KB, application/zip)
2013-11-05 08:37 EST, Libor Zoubek
no flags Details

  None (edit)
Description Libor Zoubek 2013-11-04 13:03:48 EST
Description of problem:

Documentation says: 
full - this means that the contents of the bundle completely replaces the contents of the deployment directory. I.e. after deploying the bundle, the deployment directory contains no other files but those deployed by the bundle.


Version-Release number of selected component (if applicable):
JON 3.2.ER4

How reproducible: always


Steps to Reproduce:
1. have bundle with deployment unit having compliance=full


<rhq:deployment-unit name="bundle.war" preinstallTarget="preinstall" postinstallTarget="postinstall" compliance="full">
  <rhq:file name="bundle.war" destinationFile="subdir1/bundle.war"/>      
</rhq:deployment-unit>


2. create /tmp/foo/subdir2
3. deploy (use isCleanDeployment=false) such bundle to platform destination, set target directory to /tmp/foo
4. verify /tmp/foo/subdir2 does not exist (so far OK)
5. create new version of your bundle, just by bumping it's version value, upload it to server
6. create /tmp/foo/subdir2 again
7. upgrade to new bundle version again with isCleanDeployment=false

Actual results:

/tmp/foo/subdir2 exists


Expected results:

/tmp/foo/subdir2 should be removed, because compliance=full guarantees /tmp/foo will contain *only* files provisioned by bundle deployment
Comment 1 John Mazzitelli 2013-11-04 15:26:45 EST
these two unit tests appear to test for this very use case.

org.rhq.core.util.updater.DeployerTest.baseUpdateTest(boolean, boolean)
org.rhq.plugins.ant.AntBundlePluginComponentTest.upgrade(boolean)

Can you attach the exact bundle distro file you are using?

(as per libor - the replication step #2 includes putting a file in /tmp/foo/subdir2)
Comment 2 Libor Zoubek 2013-11-05 08:37:52 EST
Created attachment 819742 [details]
bundle version1
Comment 3 Libor Zoubek 2013-11-05 09:13:07 EST
Here's my test that fails

on line 71

https://github.com/RedHatQE/jon-tests/blob/master/clitest/src/test/java/com/redhat/qe/jon/clitest/tests/bundles/BundleComplianceTest.java#L63

and after manual check it looks like files are removed but empty directories survive bundle upgrade.
Comment 4 John Mazzitelli 2013-11-05 09:19:57 EST
(In reply to Libor Zoubek from comment #3)
> Here's my test that fails
> 
> on line 71
> 
> https://github.com/RedHatQE/jon-tests/blob/master/clitest/src/test/java/com/
> redhat/qe/jon/clitest/tests/bundles/BundleComplianceTest.java#L63
> 
> and after manual check it looks like files are removed but empty directories
> survive bundle upgrade.

Right, OK, then I think this behavior hasn't changed and is working as before. The bundle upgrade stuff cares about file content, not directories. As long as the files are gone after an upgrade, that means we did the right thing. I believe bundle upgrading allows for created directories (that were created after the installation) to remain, though they are empty. Empty directories we do not delete after an upgrade. I'm pretty sure nothing changed here from earlier RHQ versions, but I'm not 100% sure of that. In any case, there is no real deterimental effects that I know of if the directories are left empty.
Comment 5 Libor Zoubek 2013-11-05 10:45:42 EST
I agree, keeping empty dirs is harmless, but at least we should mention it in docs, that we do not clean up empty dirs. I'll udpate my tests not to assert empty dirs
Comment 7 Larry O'Leary 2014-01-07 15:34:02 EST
I don't think documentation is the way to go. That would only confuse the issue and be the stand-in bandage approach.

From a user perspective the last thing I want to see is:

   compliance = full except in cases where its not


Furthermore, the existence of these directories can lead to confusion. For example, if a conf or META-INF directory gets left behind which could optionally contain some other file. A user may add that some other file without giving it another thought. The fact that after an upgrade the directory remains intact, would leave one to believe that all is well. However, due to compliance = full and our out-of-band documented exception explaining that compliance = full != full, the loss of the data in the directory may not be noticed right away. This resulting in loss of time and resources.

Considering directories are treated as files on all support operating systems, I am not sure why we can't just handle this correctly. Is there a reason we avoid removal of a directory entry because it is not of a specific type?
Comment 8 Larry O'Leary 2014-01-07 15:42:52 EST
I have dropped the severity to low and updated the title as comment 4 does indicate that this issue is cosmetic or a usability issue only and not functionally impacting.
Comment 10 Larry O'Leary 2014-02-13 20:54:46 EST
Re-targeted to CP02 to reduce CP01 overall payload.
Comment 11 Libor Zoubek 2014-02-20 08:59:52 EST
in master

commit d306411243c361b1fb3a009e6d959447322c3b5f
Author: Libor Zoubek <lzoubek@redhat.com>
Date:   Fri Feb 14 16:49:55 2014 +0100

    [BZ 1026473] When upgrading bundle with compliance=full deployment folder
    still contains empty directories not found in bundle

    This patch adds support for handling directories in bundle-deployment client
    side. The impact is only for doing updrade or revert (not initial deploy).
    FileHashcodeMap.rescan now reports every single directory with special HASH.
    Then in Deployer code we need to fill it directories to 2 other maps
    (original=from hashcodes.dat, newFiles=from bundle content). Then when
    processing diff we have to sort items to be backed up/deleted and process
    them in reverse order. (Then if there is 'dir/file', then 'dir/file' is
    processed before 'dir'). Finally backup for dir is just creating it in
    backup directory. Several tests have been fixed - especially directories now
    appear in diff additions together with new files. SimpleDeployerTest has
    been updated to test empty dir handling.
Comment 12 Libor Zoubek 2014-02-20 09:08:09 EST
in 3.2.x

commit 79814ab96769a1227d616d49d3c38a9a0a76caef
Author: Libor Zoubek <lzoubek@redhat.com>
Date:   Fri Feb 14 16:49:55 2014 +0100

(cherry picked from commit d306411243c361b1fb3a009e6d959447322c3b5f)

Signed-off-by: John Mazzitelli <mazz@redhat.com>
Comment 13 Libor Zoubek 2014-03-04 04:42:20 EST
Previous commit caused regression in ant-bundle plugin tests.

Now fixed in master

commit e9b330e8597c6607d4c6fa3126a6c6cb9148364d
Author: Libor Zoubek <lzoubek@redhat.com>
Date:   Tue Mar 4 10:32:43 2014 +0100

    [BZ 1026473] When upgrading bundle with compliance=full deployment folder

    As we now handle directories in bundle, directory name 'test' wouldn't match
    pattern 'test/.*' (which is 'rhq:ignore test/**'). Fixed by adding trailing
    slash when evaluating match.
Comment 14 Jirka Kremser 2014-03-04 05:06:46 EST
branch:  release/jon3.2.x
link:    https://github.com/rhq-project/rhq/commit/f53570725
time:    2014-03-04 10:48:28 +0100
commit:  f53570725a030fc2a883f395ebd3c93ca7ef1f00
author:  Libor Zoubek - lzoubek@redhat.com
message: [BZ 1026473] When upgrading bundle with compliance=full deployment
         folder
Comment 15 Jirka Kremser 2014-03-04 05:11:59 EST
Moving to 3.2.1 as this has already been fixed and cherry-picked to the release branch.
Comment 16 Simeon Pinder 2014-03-05 17:21:35 EST
Moving to ON_QA as available for testing in the following brew build:
https://brewweb.devel.redhat.com//buildinfo?buildID=340294

Note: the installed version is still JON 3.2.0.GA by design and this represents part of the payload for JON 3.2.1 also known as cumulative patch 1 for 3.2.0.GA.  How this will be delivered to customers is still being discussed.
Comment 17 Filip Brychta 2014-03-10 12:09:28 EDT
Verified on:
Version :	
3.2.0.GA
Build Number :	
d18651a:f535707
Comment 18 Mike Foley 2014-05-08 13:44:10 EDT
JON 3.2.1 released week of 5/5/2014

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