kura icon indicating copy to clipboard operation
kura copied to clipboard

fix(core.deployment): prevent unnecessary modifications inside the packages folder.

Open kandrej opened this issue 2 years ago • 8 comments

On every restart Kura updates the dpa.properties file even if there are no changes in installed packages.

Prevent changes to the dpa.properties file if properties do not change. Save .dp files with consistent naming "NAME_VERSION.dp". Fixed a bug in which installed .dp files would be copied to a new dp file named "NAME_VERSION.dp" on startup if they are not already named this way.

Signed-off-by: Andrej Krota [email protected]

kandrej avatar Jun 10 '22 09:06 kandrej

@mattdibi Can you please have a review? Thanks

MMaiero avatar Jun 10 '22 11:06 MMaiero

Hi @kandrej, thank you for your contribution.

For starters: as you might have noticed we recently switched to Conventional Commit for our PR titles and currently your PR doesn't conform to the standard (see the failing Lint PR action). May I suggest the title: fix(core.deployment): prevent unnecessary modifications inside the packages folder. For details see the available "types" and "scopes" in the documentation.

mattdibi avatar Jun 10 '22 11:06 mattdibi

Secondly: please take a look at the core.deployment and deployment.agent unit test and update them as the CI is reporting test failures.

mattdibi avatar Jun 10 '22 12:06 mattdibi

From the functionality standpoint I have a few issues.

On every restart Kura updates the dpa.properties file even if there are no changes in installed packages.

I'm not able to reproduce the behaviour you're mentioning here. Tested on raspberry-pi with the latest develop snapshot: I don't see any change in the /opt/eclipse/kura/packages folder after a systemctl restart kura or a reboot. Can you provide the steps to reproduce this behaviour?

Fixed a bug in which installed .dp files would be copied to a new dp file named "NAME_VERSION.dp" on startup if they are not already named this way.

I was able to reproduce this but it persists on your PR.

Steps to reproduce:

  1. Install org.eclipse.kura.wire.script.filter_1.1.0.dp
  2. systemctl stop kura
  3. Rename opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp to opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filterTEST.dp
  4. Update opt/eclipse/kura/packages/dpa.properties according to the above mentioned rename
  5. systemctl start kura

I see the package renamed as "NAME_VERSION.dp" but the old version is still there:

root@raspberrypi:/opt/eclipse/kura/packages# ls
dpa.properties	org.eclipse.kura.wire.script.filterTEST.dp  org.eclipse.kura.wire.script.filter_1.1.0.dp

This is true both for develop and kandrej:dp_modifications

Save .dp files with consistent naming "NAME_VERSION.dp".

Can you provide more details on where/when this take effect and how to reproduce?

mattdibi avatar Jun 13 '22 12:06 mattdibi

I'm sorry but currenty I'm out of office, so I will decribe the issue from memory.

To verify the first issue just check the last modified date of dpa.properties after restart (a .dp has to be installed). I will have to double-check this, maybe it was related to the second issue

Steps to reproduce the second issue.

  1. Install a .dp using DEPLOY_V2. Let's say from http://localhost:8080/test.dp
  2. The file is saved to opt/eclipse/kura/packages/test.dp
  3. systemctl restart kura
  4. opt/eclipse/kura/packages/test.dp is copied to opt/eclipse/kura/packages/test_1.0.0.dp and dpa.properties is updated

kandrej avatar Jun 14 '22 08:06 kandrej

Hi @kandrej

sorry for taking so long to getting back to you.

To verify the first issue just check the last modified date of dpa.properties after restart (a .dp has to be installed). I will have to double-check this, maybe it was related to the second issue

I cannot reproduce the issue.

Steps to reproduce the second issue.

I'll give it a go in the next days and double check if the above mentioned issue appears.

I take this opportunity to remind you that the unit tests are still failing and need to be updated.

mattdibi avatar Jul 08 '22 10:07 mattdibi

Hi @kandrej

Steps to reproduce the second issue.

  1. Install a .dp using DEPLOY_V2. Let's say from http://localhost:8080/test.dp
  2. The file is saved to opt/eclipse/kura/packages/test.dp
  3. systemctl restart kura
  4. opt/eclipse/kura/packages/test.dp is copied to opt/eclipse/kura/packages/test_1.0.0.dp and dpa.properties is updated

I followed these steps but I still can't reproduce all the issues you mentioned before. Here's the details:

Steps to reproduce

  1. Grab a .dp from the marketplace and rename it. For this I used the Wire Script Filter downloaded on my workstation and renamed it from org.eclipse.kura.wire.script.filter_1.1.0.dp to org.eclipse.kura.wire.script.filter.dp
  2. Serve the .dp from a web server. I used the halverneus/static-file-server Docker images for this with the following command (where dp_test containes the .dp):
docker run --rm -it \
    -v $(pwd)/dp_test:/web \
    -p 8080:8080 \
    halverneus/static-file-server:latest
  1. Install the latest version of Kura on a device (I used a Raspberry Pi for this) and connect to the cloud.
  2. Through the EC web UI install the package as per the following screenshot (it leverages DEPLOY-V2 under the hood) Screenshot 2022-07-12 at 08 55 27

Results with 5.1.1

The package is installed inside /opt/eclipse/kura/packages with the name org.eclipse.kura.wire.script.filter-1.1.0.dp (which should be investigated by itself, this behaviour diverges a little from your description @kandrej). Upon restarting Kura (using systemctl restart kura) the package is copied to the same location with the name org.eclipse.kura.wire.script.filter_1.1.0.dp and the dpa.properties file updated. The issue here seems to be related to the fact that the old, wrongly named .dp is not removed from the filesystem.

After subsequent reboots/restarts the dpa.properties file is not updated.

Results with kandrej:dp_modifications

The package is installed inside /opt/eclipse/kura/packages with the name org.eclipse.kura.wire.script.filter_1.1.0.dp.

After subsequent reboots/restarts the dpa.properties file is not updated.

Note: the check I performed in my previous comment still stands. If we bypass DEPLOY-V2 and manually change the dpa.properties file the problem persists even with your changes.


After all of this I still can't reproduce

On every restart Kura updates the dpa.properties file even if there are no changes in installed packages.

@kandrej can you share some more details about your setup/how to reproduce the issue?

mattdibi avatar Jul 12 '22 12:07 mattdibi

Sorry, I did not reply, I was away.

I can't reproduce either. Maybe the file changed because of the second issue, but I didn't notice the .dp file also changed. However, with my change, if you reinstall a .dp package that is already installed, the dpa.properties will not be needlessly updated.

I did not try to change the dpa.properties file manually. I'll take a look.

kandrej avatar Jul 12 '22 12:07 kandrej

Hi @kandrej, sorry for taking so long to answer.

I was able to review the PR with the latest changes.


Test 1: dpa.properties changed at every restart

Steps:

  1. Install org.eclipse.kura.wire.script.filter_1.1.0.dp from here
  2. systemctl restart kura
  3. Check /opt/eclipse/kura/packages folder for changes

Results for develop

  • Kura develop@bbe229b746f54c695f5e218119fd353bfc8fb76f

No change in the /opt/eclipse/kura/packages/dpa.properties file when the restart is issued.

Results for PR#4016

  • Kura kandrej:dp_modifications@9b1963c7ce29f50e369718a3174a3d04b743676b

No change in the /opt/eclipse/kura/packages/dpa.properties file when the restart is issued.


Test 2: Manual package renaming (DEPLOY-V2 bypass)

Steps:

  1. Install org.eclipse.kura.wire.script.filter_1.1.0.dp from here
  2. systemctl stop kura
  3. Rename opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp to opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filterTEST.dp using the CLI on the target device
  4. Update opt/eclipse/kura/packages/dpa.properties according to the above mentioned rename
  5. systemctl start kura

Results develop

  • Kura develop@bbe229b746f54c695f5e218119fd353bfc8fb76f

The package org.eclipse.kura.wire.script.filterTEST.dp is copied to the same location with the name org.eclipse.kura.wire.script.filter_1.1.0.dp and the dpa.properties file updated. The old file is still present on disk.

root@raspberrypi:/opt/eclipse/kura/packages# systemctl start kura
root@raspberrypi:/opt/eclipse/kura/packages# ls
dpa.properties	org.eclipse.kura.wire.script.filterTEST.dp  org.eclipse.kura.wire.script.filter_1.1.0.dp
root@raspberrypi:/opt/eclipse/kura/packages# cat dpa.properties
#Wed Nov 02 09:34:09 GMT 2022
org.eclipse.kura.wire.script.filter=file\:/opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp

Results for PR#4016

  • Kura kandrej:dp_modifications@9b1963c7ce29f50e369718a3174a3d04b743676b

The package org.eclipse.kura.wire.script.filterTEST.dp is renamed to the same location with the name org.eclipse.kura.wire.script.filter_1.1.0.dp and the dpa.properties file updated. The old file is deleted from disk.


Test 3: dpa.properties changed at every restart with a wrongly named file

Steps:

  1. Install org.eclipse.kura.wire.script.filter_1.1.0.dp from here
  2. systemctl stop kura
  3. Rename opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp to opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filterTEST.dp
  4. Update opt/eclipse/kura/packages/dpa.properties according to the above mentioned rename
  5. systemctl start kura
  6. Check /opt/eclipse/kura/packages folder for changes
  7. systemctl restart kura
  8. Check /opt/eclipse/kura/packages folder for changes

Results develop

  • Kura develop@bbe229b746f54c695f5e218119fd353bfc8fb76f

The file dpa.properties file is not updated even though the wrongly named file is present on disk.

root@raspberrypi:/opt/eclipse/kura/packages# ls -l
total 36
-rw-r--r-- 1 kurad kurad   144 Nov  2 09:34 dpa.properties
-rw-r--r-- 1 kurad kurad 12881 Nov  2 09:25 org.eclipse.kura.wire.script.filterTEST.dp
-rw-r--r-- 1 kurad kurad 12881 Nov  2 09:25 org.eclipse.kura.wire.script.filter_1.1.0.dp
root@raspberrypi:/opt/eclipse/kura/packages# systemctl restart kura
root@raspberrypi:/opt/eclipse/kura/packages# ls -l
total 36
-rw-r--r-- 1 kurad kurad   144 Nov  2 09:34 dpa.properties
-rw-r--r-- 1 kurad kurad 12881 Nov  2 09:25 org.eclipse.kura.wire.script.filterTEST.dp
-rw-r--r-- 1 kurad kurad 12881 Nov  2 09:25 org.eclipse.kura.wire.script.filter_1.1.0.dp
root@raspberrypi:/opt/eclipse/kura/packages# cat dpa.properties
#Wed Nov 02 09:34:09 GMT 2022
org.eclipse.kura.wire.script.filter=file\:/opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp

Checked even with a reboot.

Results for PR#4016

  • Kura kandrej:dp_modifications@9b1963c7ce29f50e369718a3174a3d04b743676b

Cannot reproduce since file is deleted at Kura startup


Test 4: Download and install wrongly named package

Steps:

  1. Grab a .dp from the marketplace and rename it. For this I used the Wire Script Filter downloaded on my workstation and renamed it from org.eclipse.kura.wire.script.filter_1.1.0.dp to org.eclipse.kura.wire.script.filter.dp
  2. Serve the .dp from a web server. I used the halverneus/static-file-server Docker images for this with the following command (where dp_test containes the .dp):
docker run --rm -it \
    -v $(pwd)/dp_test:/web \
    -p 8080:8080 \
    halverneus/static-file-server:latest
  1. Install the latest version of Kura on a device (I used a Raspberry Pi for this) and connect to the cloud.
  2. Through the Kapua web UI install the package as per the following screenshot (it leverages DEPLOY-V2 under the hood)

Results develop

  • Kura develop@bbe229b746f54c695f5e218119fd353bfc8fb76f

The package is installed inside /opt/eclipse/kura/packages with the name org.eclipse.kura.wire.script.filter-1.1.0.dp. Upon restarting Kura (using systemctl restart kura) the package is copied to the same location with the name org.eclipse.kura.wire.script.filter_1.1.0.dp and the dpa.properties file updated. The issue here seems to be related to the fact that the old, wrongly named .dp is not removed from the filesystem.

Results for PR#4016

  • Kura kandrej:dp_modifications@9b1963c7ce29f50e369718a3174a3d04b743676b

The package is installed inside /opt/eclipse/kura/packages with the name org.eclipse.kura.wire.script.filter_1.1.0.dp. Upon restarting kura with systemctl restart kura nothing happens and the dpa.properties file is not updated. This is the expected behavior.


Test 5: dpa.properties changed if .dp package is reinstalled

Steps:

  1. Grab a .dp from the marketplace and rename it. For this I used the Wire Script Filter downloaded on my workstation and renamed it from org.eclipse.kura.wire.script.filter_1.1.0.dp to org.eclipse.kura.wire.script.filter.dp
  2. Serve the .dp from a web server. I used the halverneus/static-file-server Docker images for this with the following command (where dp_test containes the .dp):
docker run --rm -it \
    -v $(pwd)/dp_test:/web \
    -p 8080:8080 \
    halverneus/static-file-server:latest
  1. Install org.eclipse.kura.wire.script.filter_1.1.0.dp by passing in the URL to the .dp (in my case http://macbook.local.home:8090/org.eclipse.kura.wire.script.filter.dp) using the Kura UI
  2. Check /opt/eclipse/kura/packages folder for changes
  3. Reinstall org.eclipse.kura.wire.script.filter_1.1.0.dp
  4. Check /opt/eclipse/kura/packages folder for changes

Results develop

  • Kura develop@bbe229b746f54c695f5e218119fd353bfc8fb76f

The dpa.properties file gets updated for every reinstall attempt.

root@raspberrypi:/opt/eclipse/kura/packages# cat dpa.properties
#Wed Nov 02 10:23:48 GMT 2022
org.eclipse.kura.wire.script.filter=file\:/opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp
root@raspberrypi:/opt/eclipse/kura/packages# cat dpa.properties
#Wed Nov 02 10:23:48 GMT 2022
org.eclipse.kura.wire.script.filter=file\:/opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp
root@raspberrypi:/opt/eclipse/kura/packages# cat dpa.properties
#Wed Nov 02 10:24:03 GMT 2022
org.eclipse.kura.wire.script.filter=file\:/opt/eclipse/kura/packages/org.eclipse.kura.wire.script.filter_1.1.0.dp

Results for PR#4016

  • Kura kandrej:dp_modifications@9b1963c7ce29f50e369718a3174a3d04b743676b

The dpa.properties file doesn't get updated for every reinstall attempt.


So... from the functionality point of view I think the PR LGTM.

@kandrej can you please perform a rebase of your branch? This branch looks really far behind the develop branch and would like to give it a test with the latest changes.

mattdibi avatar Nov 02 '22 10:11 mattdibi

@mattdibi Can you please create a PR to fix that duplication issue reported by Sonar? Thanks

MMaiero avatar Nov 08 '22 16:11 MMaiero

Documentation

What was fixed in this PR?

  1. With this PR the dpa.properties file doesn't get overwritten for every deployment package reinstall avoiding unnecessary disk writes.
  2. Upon installation from DEPLOY-V2 the .dp package gets correctly named in the format NAME_VERSION. Before this PR the package would be named NAME-VERSION upon installation, after a Kura restart/reboot a further renaming to the correct format was performed.
  3. If a package was named in an unsupported format (like NAME-VERSION) it was not deleted upon renaming on restart. With this PR the rename ensures the old file is deleted avoiding unnecessary disk space consumption.

mattdibi avatar Nov 09 '22 07:11 mattdibi