Sling
  1. Sling
  2. SLING-922

Load modules on startup from an external directory

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Launchpad Base 2.1.0
    • Component/s: Launchpad
    • Labels:
      None

      Description

      Need a way to install and start bundles automatically from an external directory (sling home) on startup. The ideas below are from the list.
      ----------
      > I would like to be able to setup sling to start with a bunch of
      > installed bundles when it first is started. It seems like there are a
      > couple way to do this that I have found but neither is ideal:
      > 1) Rebuild sling from source with the extra bundles in the launcher
      > pom (this seems to create a bunch of resources/# folders with bundles
      > in them)
      > 2) Configure all bundles in the sling.properties file (this requires
      > the bundles to be in an accessible obr)
      > http://incubator.apache.org/sling/site/provisioning-and-startup.html
      >
      > I would like to ideally do something in between like so:
      > Get a binary copy of the sling jar
      > Create a folder with sub-folders like /1 /10 /15
      > Put my bundles in the various subfolders
      > Configure sling.properties to point to the folder
      > Start sling and have all bundles in the sub-folders installed and started

      • Aaron Zeckoski
        ===============
        Currently, as you say in (1), the BootstrapInstaller of the Sling
        launchpad looks into its own resources enclosed in the JAR or WAR file
        for bundles to install on startup.

      How about extending this mechanism like this:

      • Copy all bundles from enclosed resources to
        $ {sling.home}/startup. This gives something like
        ${sling.home}

        /startup/0, /1, /10, /15, ...
        Existing files are only replaced if the files
        enclosed in the Sling launchpad jar/war file are
        newer.

      • Scan $ {sling.home}

        /startup for bundles to install
        in the same way as today the enclosed resources
        are scanned directly.

      So you could place your bundles in that structure and get them installed
      at the requested start level (0 being "default bundle start level").

      A nice side effect of this is, that you can quickly see, which bundles
      have been installed at all.

      • Felix
        =================
        I like this, and agree that this should replace the current mechanism.

      How about adding a sling.properties option to completely ignore the
      bundles that come from the Sling jar/war file? Might make it easier to
      have precise control on what's installed.

      -Bertrand
      =================
      Maybe worth it to make this optional or controllable via a property in the sling properties.

      • Aaron Zeckoski
      1. binary-test-resources.zip
        10 kB
        Aaron Zeckoski
      2. SLING922_fmeschbe.patch
        30 kB
        Felix Meschberger
      3. SLING-922.patch
        45 kB
        Aaron Zeckoski

        Issue Links

          Activity

          Hide
          Aaron Zeckoski added a comment -

          Just so it is clear, I am working on a patch for this so please please let me know if someone else (like you) are thinking about writing this so I don't waste my time.

          Show
          Aaron Zeckoski added a comment - Just so it is clear, I am working on a patch for this so please please let me know if someone else (like you) are thinking about writing this so I don't waste my time.
          Hide
          Felix Meschberger added a comment -

          Provisionally assigning to me. I know that you Aaron are working on it so I will surely just be sitting and waiting for the patch to arrive.

          Thanks alot.

          Show
          Felix Meschberger added a comment - Provisionally assigning to me. I know that you Aaron are working on it so I will surely just be sitting and waiting for the patch to arrive. Thanks alot.
          Hide
          Aaron Zeckoski added a comment -

          A few notes as I am going through this:
          1) It is possible to check the bundles which are being copied from the war/jar against the installed bundles but I am not sure if this is what we want. If I check them as they are being copied then I can avoid copying over any bundles which are already installed (maybe good?) so the startup will contain a more accurate set of installed bundles. If I wait and check them later then the startup could contain bundles that were not installed. Currently I am checking before copying and not copying if the bundle is installed already.
          2) the if (!isAlreadyInstalled(context)) check keeps any new bundles which are added to the jar/war or in the new case, to the startup directory from being installed normally. I would prefer this allow any new bundles in the startup to be installed after the initial startup so I am making it so the check only keeps the DeploymentPackageInstaller from running more than once (for now). Maybe this could be a sling property.
          3) Invalid or duplicate bundles in the startup are currently just causing warnings in the logs. I think this is consistent with current behavior but perhaps there should be a strict mode that dies when bundles are invalid or duplicated? Thoughts?

          Show
          Aaron Zeckoski added a comment - A few notes as I am going through this: 1) It is possible to check the bundles which are being copied from the war/jar against the installed bundles but I am not sure if this is what we want. If I check them as they are being copied then I can avoid copying over any bundles which are already installed (maybe good?) so the startup will contain a more accurate set of installed bundles. If I wait and check them later then the startup could contain bundles that were not installed. Currently I am checking before copying and not copying if the bundle is installed already. 2) the if (!isAlreadyInstalled(context)) check keeps any new bundles which are added to the jar/war or in the new case, to the startup directory from being installed normally. I would prefer this allow any new bundles in the startup to be installed after the initial startup so I am making it so the check only keeps the DeploymentPackageInstaller from running more than once (for now). Maybe this could be a sling property. 3) Invalid or duplicate bundles in the startup are currently just causing warnings in the logs. I think this is consistent with current behavior but perhaps there should be a strict mode that dies when bundles are invalid or duplicated? Thoughts?
          Hide
          Felix Meschberger added a comment -

          ad 1): With respect to copying bundles out of the enclosing into the startup folder, I would check the bundle contents before actually copying it. The problem of having out-of-sync situation with the actually installed bundles is acceptable and actually even an expected situation frm the POV of OSGi: You may install, update or remove bundles at any time.

          ad 2): IIRC the isAlreadyInstalled method compares a timestamp stored in the filesystem with the timestamp of the launchpad jar file. So whenever this file is updated that modification time is changed (it changes when you modify embedded bundles), the isAlreadyInstalled method returns false. So changing embedded bundles has the "side-effect" of causing the BootstrapInstaller to inspect all enclosed bundles.

          ad 3): I am not sure, what warn message you mean. There is a info logged when a bundle is enclosed whose version is lower than (or equal to) the version of a bundle of the same symbolic name which is already installed. I think, this is a valid situation, since someone might already have updated the bundle through other means. Other than that I cannot remember any duplicity checks in the BootstrapInstaller

          Show
          Felix Meschberger added a comment - ad 1): With respect to copying bundles out of the enclosing into the startup folder, I would check the bundle contents before actually copying it. The problem of having out-of-sync situation with the actually installed bundles is acceptable and actually even an expected situation frm the POV of OSGi: You may install, update or remove bundles at any time. ad 2): IIRC the isAlreadyInstalled method compares a timestamp stored in the filesystem with the timestamp of the launchpad jar file. So whenever this file is updated that modification time is changed (it changes when you modify embedded bundles), the isAlreadyInstalled method returns false. So changing embedded bundles has the "side-effect" of causing the BootstrapInstaller to inspect all enclosed bundles. ad 3): I am not sure, what warn message you mean. There is a info logged when a bundle is enclosed whose version is lower than (or equal to) the version of a bundle of the same symbolic name which is already installed. I think, this is a valid situation, since someone might already have updated the bundle through other means. Other than that I cannot remember any duplicity checks in the BootstrapInstaller
          Hide
          Aaron Zeckoski added a comment -

          1) OK, so no checking before copying then?
          2) So only copy if this trips then (that makes sense to me)
          3) Sorry, poor choice of words. I mean currently in the code I am working on, not in the sling trunk.
          They are info level like below (this is one from trunk which checks to see if the bundle is already installed and newer and skips it if so). That is similar to a duplicate check in my mind:
          logger.log(Logger.LOG_INFO, "Ignoring " + path
          + ": More recent version already installed");
          I was mosting thinking of the situation where there is a bundle in startup/0 and startup/1 which are the same. I could just have the first loaded or last loaded win but I am not sure what the best way to handle it is. I was thinking I would just use the ignore(installedBundle, manifest) check in this case. Am I making sense?

          Show
          Aaron Zeckoski added a comment - 1) OK, so no checking before copying then? 2) So only copy if this trips then (that makes sense to me) 3) Sorry, poor choice of words. I mean currently in the code I am working on, not in the sling trunk. They are info level like below (this is one from trunk which checks to see if the bundle is already installed and newer and skips it if so). That is similar to a duplicate check in my mind: logger.log(Logger.LOG_INFO, "Ignoring " + path + ": More recent version already installed"); I was mosting thinking of the situation where there is a bundle in startup/0 and startup/1 which are the same. I could just have the first loaded or last loaded win but I am not sure what the best way to handle it is. I was thinking I would just use the ignore(installedBundle, manifest) check in this case. Am I making sense?
          Hide
          Felix Meschberger added a comment -

          ad 1) To the contrary: I would check before actually copying

          ad 3) I see. That would be a new check, since currently this is not checked. But it might make sense to check this (if easly feasible) and at least WARN or ERROR about it. Whether we should die ? I am not so sure, really.

          Show
          Felix Meschberger added a comment - ad 1) To the contrary: I would check before actually copying ad 3) I see. That would be a new check, since currently this is not checked. But it might make sense to check this (if easly feasible) and at least WARN or ERROR about it. Whether we should die ? I am not so sure, really.
          Hide
          Aaron Zeckoski added a comment -

          OK, I had some trouble generating this patch file so my apologies if it ends up being a little tricky to apply the patch. I have attached a zip which has the binary pieces used in the test. The remaining stuff should be visible in the patch file. So apply the patch and then extract the binary-test-resources into the sling trunk and then the build should be fine.

          Show
          Aaron Zeckoski added a comment - OK, I had some trouble generating this patch file so my apologies if it ends up being a little tricky to apply the patch. I have attached a zip which has the binary pieces used in the test. The remaining stuff should be visible in the patch file. So apply the patch and then extract the binary-test-resources into the sling trunk and then the build should be fine.
          Hide
          Aaron Zeckoski added a comment -

          Everything seems to be working on my machine so I think this is ready to go. Please let me know if there are any issues and I will generate a new patch.

          Show
          Aaron Zeckoski added a comment - Everything seems to be working on my machine so I think this is ready to go. Please let me know if there are any issues and I will generate a new patch.
          Hide
          Felix Meschberger added a comment -

          Thanks for the patch, I try to look at it shortly.

          Show
          Felix Meschberger added a comment - Thanks for the patch, I try to look at it shortly.
          Hide
          Felix Meschberger added a comment -

          Thanks for providing the patch and testcase. All in all, this looks very promising and a really nice feature.

          Yet I have some comments:

          (1) Protected Methods: Many methods are protected. I prefer to default methods to private and only if they prove to be required outside of the clase, I make them default visibility, protected or public, but very reluctant. So I would like to have visibility reduced on all protected methods to private. If some methods are used for testcases, we should make them have default visibility. Reason: A protected method becomes part of the API of a class. As such, it must be thought about, whether it really constitutes API or is just a helper method. Also removing or changning API is a relatively expensive task.

          If you agree, I would just "downgrade" all methods to private (or package if required by the test case).

          (2) The BootstrapInstallerTest class has an unusual file header, which makes it hard to include it as is. Generally we have the default license header (see the BootstrapInstaller class for example).

          If you agree, I will just "fix" this whille committing the class.

          (3) The helper bundles are only provided as binaries. I am somewhat reluctant to addding binaries to the SVN without their accompanying source. I would probably be more comfortable with the source in SVN and creating the binaries during the test run (using the bnd tool and ant script for example).

          (4) The BootstrapInstaller.start method now always checks the bundles in the startup folder on each startup. This is a potentially expensive operation. Therefore, I would think it would be better to enhance the isAlreadyInstalled test to see, whether there are files in the startup folder, which are more recent that the flag. If so the installation may take place.

          (5) You add a dependency to Junit 4.5. Is this required or can we live with Sling's current setup of JUnit 4.3 ?

          If 4.3 does it (it does on my system), I will just remove th version, scope and type elements from the pom while checking in.

          I will attach a patch with the BundleInstaller class slightly modified from your patch.

          Show
          Felix Meschberger added a comment - Thanks for providing the patch and testcase. All in all, this looks very promising and a really nice feature. Yet I have some comments: (1) Protected Methods: Many methods are protected. I prefer to default methods to private and only if they prove to be required outside of the clase, I make them default visibility, protected or public, but very reluctant. So I would like to have visibility reduced on all protected methods to private. If some methods are used for testcases, we should make them have default visibility. Reason: A protected method becomes part of the API of a class. As such, it must be thought about, whether it really constitutes API or is just a helper method. Also removing or changning API is a relatively expensive task. If you agree, I would just "downgrade" all methods to private (or package if required by the test case). (2) The BootstrapInstallerTest class has an unusual file header, which makes it hard to include it as is. Generally we have the default license header (see the BootstrapInstaller class for example). If you agree, I will just "fix" this whille committing the class. (3) The helper bundles are only provided as binaries. I am somewhat reluctant to addding binaries to the SVN without their accompanying source. I would probably be more comfortable with the source in SVN and creating the binaries during the test run (using the bnd tool and ant script for example). (4) The BootstrapInstaller.start method now always checks the bundles in the startup folder on each startup. This is a potentially expensive operation. Therefore, I would think it would be better to enhance the isAlreadyInstalled test to see, whether there are files in the startup folder, which are more recent that the flag. If so the installation may take place. (5) You add a dependency to Junit 4.5. Is this required or can we live with Sling's current setup of JUnit 4.3 ? If 4.3 does it (it does on my system), I will just remove th version, scope and type elements from the pom while checking in. I will attach a patch with the BundleInstaller class slightly modified from your patch.
          Hide
          Felix Meschberger added a comment -

          Modifications to the BundleInstaller patch proposal:

          • some method reordering
          • all protected methods are now private or package
          • extending getSelfTimeStamp to include the bundles from startup folder
          • only install/update if there is something to do
          • copying bundles from the launcher does not check them any more (done on install/update)
          • use default JUnit version /change in pom.xml)

          WDYT ?

          Show
          Felix Meschberger added a comment - Modifications to the BundleInstaller patch proposal: some method reordering all protected methods are now private or package extending getSelfTimeStamp to include the bundles from startup folder only install/update if there is something to do copying bundles from the launcher does not check them any more (done on install/update) use default JUnit version /change in pom.xml) WDYT ?
          Hide
          Bertrand Delacretaz added a comment -

          > ...(3) The helper bundles are only provided as binaries.,,,I would probably be more comfortable with the
          > source in SVN and creating the binaries during the test run (using the bnd tool and ant script
          > for example). ..

          FWIW, I'm doing something like this in the installClonedBundle() method of the JcrinstallTestBase class [1]. The BundleCloner class uses bnd to create several copies of test bundles with different symbolic names. See also SLING-946, for some reason the bnd call currently fails on our Hudson build server.

          [1] http://svn.eu.apache.org/repos/asf/incubator/sling/trunk/contrib/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/jcrinstall/JcrinstallTestBase.java

          Show
          Bertrand Delacretaz added a comment - > ...(3) The helper bundles are only provided as binaries.,,,I would probably be more comfortable with the > source in SVN and creating the binaries during the test run (using the bnd tool and ant script > for example). .. FWIW, I'm doing something like this in the installClonedBundle() method of the JcrinstallTestBase class [1] . The BundleCloner class uses bnd to create several copies of test bundles with different symbolic names. See also SLING-946 , for some reason the bnd call currently fails on our Hudson build server. [1] http://svn.eu.apache.org/repos/asf/incubator/sling/trunk/contrib/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/jcrinstall/JcrinstallTestBase.java
          Hide
          Aaron Zeckoski added a comment -

          (1) Protected Methods: Many methods are protected. I prefer to default methods to private and only if they prove to be required outside of the clase, I make them default visibility, protected or public, but very reluctant. So I would like to have visibility reduced on all protected methods to private. If some methods are used for testcases, we should make them have default visibility. Reason: A protected method becomes part of the API of a class. As such, it must be thought about, whether it really constitutes API or is just a helper method. Also removing or changning API is a relatively expensive task.

          If you agree, I would just "downgrade" all methods to private (or package if required by the test case).

          • Sounds fine. Default is most appropriate anyway for the methods which are being tested. I should probably have left them as default anyway.

          (2) The BootstrapInstallerTest class has an unusual file header, which makes it hard to include it as is. Generally we have the default license header (see the BootstrapInstaller class for example).

          If you agree, I will just "fix" this whille committing the class.

          • Please change the headers however you like. These are probably just some default ones I use.

          (3) The helper bundles are only provided as binaries. I am somewhat reluctant to addding binaries to the SVN without their accompanying source. I would probably be more comfortable with the source in SVN and creating the binaries during the test run (using the bnd tool and ant script for example).

          • You mean the test bundles here I assume. That's fine with me. I just wanted something that would serve as good static test data and it proved impossible for me to get bnd to generate invalid bundles but if you can do this then definitely do it.

          (4) The BootstrapInstaller.start method now always checks the bundles in the startup folder on each startup. This is a potentially expensive operation. Therefore, I would think it would be better to enhance the isAlreadyInstalled test to see, whether there are files in the startup folder, which are more recent that the flag. If so the installation may take place.

          • The modification to getSelfTimestamp looks good to me.

          (5) You add a dependency to Junit 4.5. Is this required or can we live with Sling's current setup of JUnit 4.3 ?

          • 4.3 should work fine.
          Show
          Aaron Zeckoski added a comment - (1) Protected Methods: Many methods are protected. I prefer to default methods to private and only if they prove to be required outside of the clase, I make them default visibility, protected or public, but very reluctant. So I would like to have visibility reduced on all protected methods to private. If some methods are used for testcases, we should make them have default visibility. Reason: A protected method becomes part of the API of a class. As such, it must be thought about, whether it really constitutes API or is just a helper method. Also removing or changning API is a relatively expensive task. If you agree, I would just "downgrade" all methods to private (or package if required by the test case). Sounds fine. Default is most appropriate anyway for the methods which are being tested. I should probably have left them as default anyway. (2) The BootstrapInstallerTest class has an unusual file header, which makes it hard to include it as is. Generally we have the default license header (see the BootstrapInstaller class for example). If you agree, I will just "fix" this whille committing the class. Please change the headers however you like. These are probably just some default ones I use. (3) The helper bundles are only provided as binaries. I am somewhat reluctant to addding binaries to the SVN without their accompanying source. I would probably be more comfortable with the source in SVN and creating the binaries during the test run (using the bnd tool and ant script for example). You mean the test bundles here I assume. That's fine with me. I just wanted something that would serve as good static test data and it proved impossible for me to get bnd to generate invalid bundles but if you can do this then definitely do it. (4) The BootstrapInstaller.start method now always checks the bundles in the startup folder on each startup. This is a potentially expensive operation. Therefore, I would think it would be better to enhance the isAlreadyInstalled test to see, whether there are files in the startup folder, which are more recent that the flag. If so the installation may take place. The modification to getSelfTimestamp looks good to me. (5) You add a dependency to Junit 4.5. Is this required or can we live with Sling's current setup of JUnit 4.3 ? 4.3 should work fine.
          Hide
          Aaron Zeckoski added a comment -

          Is this waiting on some action from me? Just let me know if there is something you would like me to do.
          it might be easiest for now to just remove the tests which use the binary files if there is an issue around those. I would not want that to be the reason why this is held up.

          Show
          Aaron Zeckoski added a comment - Is this waiting on some action from me? Just let me know if there is something you would like me to do. it might be easiest for now to just remove the tests which use the binary files if there is an issue around those. I would not want that to be the reason why this is held up.
          Hide
          Ray Davis added a comment -

          I've voted for this, but FWIW as a primitive workaround I've modified my local launcher POM to include the Felix File Install bundle. This lets me load an arbitrary set of separately built and deployed components at initial runtime without the launcher POM becoming a contention point.

          Show
          Ray Davis added a comment - I've voted for this, but FWIW as a primitive workaround I've modified my local launcher POM to include the Felix File Install bundle. This lets me load an arbitrary set of separately built and deployed components at initial runtime without the launcher POM becoming a contention point.
          Hide
          Mike Müller added a comment -

          Seems to be pretty finished. Is there any reason to not commit this patch?

          Show
          Mike Müller added a comment - Seems to be pretty finished. Is there any reason to not commit this patch?
          Hide
          Felix Meschberger added a comment -

          lack of time .... ?

          Show
          Felix Meschberger added a comment - lack of time .... ?
          Hide
          Felix Meschberger added a comment -

          At long last (sorry for the massive delay) I have applied the patch in Rev. 810786 and 810787.

          Please give it a test ride and close if working. Thanks.

          Show
          Felix Meschberger added a comment - At long last (sorry for the massive delay) I have applied the patch in Rev. 810786 and 810787. Please give it a test ride and close if working. Thanks.
          Hide
          Bertrand Delacretaz added a comment -

          Reopening as there's no documentation for this feature, AFAIK.

          Will ask on list if JIRA would allow a "needs documentation" state for the issue.

          Show
          Bertrand Delacretaz added a comment - Reopening as there's no documentation for this feature, AFAIK. Will ask on list if JIRA would allow a "needs documentation" state for the issue.
          Hide
          Felix Meschberger added a comment -

          Excellent point I completely forgot about.

          Show
          Felix Meschberger added a comment - Excellent point I completely forgot about.
          Hide
          Carsten Ziegeler added a comment - - edited

          Moving to next version as this one is already released - fixed in 2.1.0

          Show
          Carsten Ziegeler added a comment - - edited Moving to next version as this one is already released - fixed in 2.1.0

            People

            • Assignee:
              Felix Meschberger
              Reporter:
              Aaron Zeckoski
            • Votes:
              3 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 72h
                72h
                Remaining:
                Remaining Estimate - 72h
                72h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development