search

artefactual / archivematica-storage-service

Archivematica storage service
http://www.archivematica.org
GNU Affero General Public License v3.0
34 stars 43 forks source link

Problem: Storage Service tests run locally are failing #295

Closed ross-spencer closed 5 years ago

ross-spencer commented 6 years ago

Log attached: storage-service-failures.txt

Additional information.

Distributor ID: Ubuntu
Description:    Ubuntu 17.10
Release:    17.10
Codename:   artful

docker-compose version 1.17.1, build 6d101fb

Docker: 

Client:
 Version:      17.11.0-ce
 API version:  1.34
 Go version:   go1.8.3
 Git commit:   1caf76c
 Built:        Mon Nov 20 18:36:36 2017
 OS/Arch:      linux/amd64

Server:
 Version:      17.11.0-ce
 API version:  1.34 (minimum version 1.12)
 Go version:   go1.8.3
 Git commit:   1caf76c
 Built:        Mon Nov 20 18:35:08 2017
 OS/Arch:      linux/amd64
 Experimental: false
sevein commented 6 years ago

IIRC the root cause was that some of our tests attempt to mutate the source code folder but in our Docker image the source code is owned by root while the user running the tests is archivematica hence the permission error.

I filed https://github.com/artefactual/archivematica-storage-service/issues/220 and attempted to fix it via https://github.com/artefactual/archivematica-storage-service/pull/276. Is it possible that your SS git working tree is using an older revision?

ross-spencer commented 6 years ago

I have stable/0.11.x checked out and up to date, so that should be okay right?

On Sun, Dec 17, 2017 at 12:56 PM, Jesús García Crespo < notifications@github.com> wrote:

IIRC the root cause was that some of our tests attempt to mutate the source code folder but in our Docker image the source code is owned by root while the user running the tests is archivematica hence the permission error.

I filed #220 https://github.com/artefactual/archivematica-storage-service/issues/220 and attempted to fix it via #276 https://github.com/artefactual/archivematica-storage-service/pull/276 Is it possible that you SS git working tree is using an older revision?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/artefactual/archivematica-storage-service/issues/295#issuecomment-352276976, or mute the thread https://github.com/notifications/unsubscribe-auth/AByxXERg4-h4HpFrlM4p6xepQUezACSWks5tBWPlgaJpZM4REjiG .

-- Ross Spencer Software Developer, Artefactual Systems Inc. www.artefactual.com

ross-spencer commented 6 years ago

Okay, I see your commits are against qa/0.x. I'll continue to use that one instead.

On Sun, Dec 17, 2017 at 1:15 PM, Ross Spencer rspencer@artefactual.com wrote:

I have stable/0.11.x checked out and up to date, so that should be okay right?

On Sun, Dec 17, 2017 at 12:56 PM, Jesús García Crespo < notifications@github.com> wrote:

IIRC the root cause was that some of our tests attempt to mutate the source code folder but in our Docker image the source code is owned by root while the user running the tests is archivematica hence the permission error.

I filed #220 https://github.com/artefactual/archivematica-storage-service/issues/220 and attempted to fix it via #276 https://github.com/artefactual/archivematica-storage-service/pull/276 Is it possible that you SS git working tree is using an older revision?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/artefactual/archivematica-storage-service/issues/295#issuecomment-352276976, or mute the thread https://github.com/notifications/unsubscribe-auth/AByxXERg4-h4HpFrlM4p6xepQUezACSWks5tBWPlgaJpZM4REjiG .

-- Ross Spencer Software Developer, Artefactual Systems Inc. www.artefactual.com

-- Ross Spencer Software Developer, Artefactual Systems Inc. www.artefactual.com

sevein commented 6 years ago

It's also in stable.

I can't reproduce. Check this out: https://asciinema.org/a/7aoF45uf48VhWSB7gBEMDTNPt.

What permissions do you see when you access the container?

sevein commented 6 years ago

Using Docker for Mac here btw.

ross-spencer commented 6 years ago

Permissions in the container:

archivematica@c1a052228585:/src/storage_service$ ls
__init__.py common  locations  pytest.ini  storage_service
administration  locale  manage.py  static      templates
archivematica@c1a052228585:/src/storage_service$ ls -la
total 52
drwxr-xr-x 9 1001 1001 4096 Dec 12 18:05 .
drwxr-xr-x 9 1001 1001 4096 Dec 17 18:14 ..
-rw-r--r-- 1 1001 1001  144 Dec 12 18:05 .coveragerc
-rw-r--r-- 1 1001 1001   60 Dec 12 18:05 __init__.py
drwxr-xr-x 5 1001 1001 4096 Dec 17 19:29 administration
drwxr-xr-x 4 1001 1001 4096 Dec 17 18:10 common
drwxr-xr-x 6 1001 1001 4096 Dec 12 18:05 locale
drwxr-xr-x 8 1001 1001 4096 Dec 17 18:10 locations
-rwxr-xr-x 1 1001 1001  264 Dec 12 18:05 manage.py
-rw-r--r-- 1 1001 1001   88 Dec 12 18:05 pytest.ini
drwxr-xr-x 6 1001 1001 4096 Dec 12 18:05 static
drwxr-xr-x 4 1001 1001 4096 Dec 17 18:10 storage_service
drwxr-xr-x 5 1001 1001 4096 Dec 12 18:05 templates

Events in Asciinema, https://asciinema.org/a/153168

sevein commented 6 years ago

Ok I see! https://github.com/artefactual/archivematica-storage-service/pull/276 solved the problem to some extent (in our Docker image) but the issue persists when you're injecting the source code from your host into the container because locations/fixtures is owned by your local user.

I think that the right solution here is to update our tests so they create the temporary contents in /var/archivematica/storage_service. Does that make sense?

ross-spencer commented 6 years ago

It makes sense. I can see that /var/archivematica/storage_service is a location that is owned by the archivematica user in the container and so it should be possible to write temporary contents there without issue.

ross-spencer commented 6 years ago

@sevein I've got time today, I can look at modifying those tests to write to the alternate locations. It's good learning.

ross-spencer commented 6 years ago

List of affected tests to modify. 

<test_api.TestPackageAPI testMethod=test_download_file_from_compressed>
<test_api.TestPackageAPI testMethod=test_download_lockss_chunk_incorrect>
<test_api.TestPackageAPI testMethod=test_download_uncompressed_package>
<test_arkivum.TestArkivum testMethod=test_browse>
<test_arkivum.TestArkivum testMethod=test_delete>
<test_arkivum.TestArkivum testMethod=test_has_required_attributes>
<test_arkivum.TestArkivum testMethod=test_post_move_from_ss>
<test_arkivum.TestArkivum testMethod=test_update_package_status_compressed>
<test_arkivum.TestArkivum testMethod=test_update_package_status_uncompressed>
<test_dataverse.TestDataverse testMethod=test_move_to>
<test_dspace.TestDSpace testMethod=test_get_metadata>
<test_dspace.TestDSpace testMethod=test_move_from_ss>
<test_dspace.TestDSpace testMethod=test_split_package_7z>
<test_dspace.TestDSpace testMethod=test_split_package_zip>
<test_duracloud.TestDuracloud testMethod=test_move_from_ss_chunked_file>
<test_duracloud.TestDuracloud testMethod=test_move_from_ss_chunked_resume>
<test_swift.TestSwift testMethod=test_move_from_ss>
<test_swift.TestSwift testMethod=test_move_to_ss>
<test_swift.TestSwift testMethod=test_move_to_ss_bad_etag>
<test_swift.TestSwift testMethod=test_move_to_ss_folder>
ross-spencer commented 6 years ago

I have worked through the first two test files in the code to understand the problem better and can confirm that if we change our fixtures location to /var/archivematica/storage_service/fixtures/ then ostensibly this will work. That being said, I'm not sure that would be a complete solution in and of itself.

Following our Docker container pattern, then injecting these files into the container so that they are also available at /var/archivematica/storage_service/fixtures seems consistent.

So the proposed approach is:

@sevein I don't yet have enough of a grasp to make a decision on the impact of this change on other deployment methods, for example, will all deployment methods enable users to run tests that need access to /var/archivematica/storage_service/fixtures? Same question with the 'read from' location. A guide here would be helpful.

An alternative question is how do we make sure that we accommodate other deployment methods if there is an impact caused by these changes? - Any help, or a pointer to another dev with a view of this is appreciated.

sevein commented 6 years ago

Make a decision on whether to modify the code or not to also change the location of a variable that describes a 'read from' location for fixtures.

Reading from the internal locations/fixtures should be fine, I don't think that needs to be changed unless the separation makes the task much harder. I don't remember off the top of my head if these tests set up a location in which it's expected to both read the existing files and write new ones - if that was the case then I would just not worry and copy the whole thing when the tests start. It won't perform very well but we can worry about performance later.

An alternative question is how do we make sure that we accommodate other deployment methods if there is an impact caused by these changes? - Any help, or a pointer to another dev with a view of this is appreciated.

I suggested /var/archivematica/storage_service because it's the path of the default SS Internal Processing location. We could use /tmp instead, I think it doesn't really matter. Perhaps the latter is less problematic because you're not making assumptions about certain paths being available etc... although it's kind of the standard in AM to have /var/archivematica/storage_service created and be writeable by the archivematica user.

ross-spencer commented 6 years ago

@sevein since I reported this the MakeFile has been updated to run tests as root. Is there still an appetite to see these failures fixed so that they can run okay in the AM user's space?

sevein commented 6 years ago

Yes, I think it would be a good thing but low priority.

sevein commented 5 years ago

Fixed in: