Project

Profile

Help

Issue #2047

directory creation race condition during publish

Added by mhrivnak almost 5 years ago. Updated about 2 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
2.8.5
Platform Release:
2.9.3
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Easy Fix, Pulp 2
Sprint:
Quarter:

Description

If two repos are published concurrently and share a parent directory, there is a chance they will try to create the parent directory at the same time, and the loser will fail with a traceback like this:

traceback: ! "Traceback (most recent call last):\n  File \"/usr/lib/python2.6/site-packages/celery/app/trace.py\",
    line 240, in trace_task\n    R = retval = fun(*args, **kwargs)\n  File \"/usr/lib/python2.6/site-packages/pulp/server/async/tasks.py\",
    line 328, in __call__\n    return super(Task, self).__call__(*args, **kwargs)\n
    \ File \"/usr/lib/python2.6/site-packages/celery/app/trace.py\", line 437, in
    __protected_call__\n    return self.run(*args, **kwargs)\n  File \"/usr/lib/python2.6/site-packages/pulp/server/managers/repo/publish.py\",
    line 99, in publish\n    transfer_repo, conduit, call_config)\n  File \"/usr/lib/python2.6/site-packages/pulp/server/managers/repo/publish.py\",
    line 127, in _do_publish\n    publish_report = publish_repo(transfer_repo, conduit,
    call_config)\n  File \"/usr/lib/python2.6/site-packages/pulp/server/async/tasks.py\",
    line 495, in wrap_f\n    return f(*args, **kwargs)\n  File \"/usr/lib/python2.6/site-packages/pulp_rpm/plugins/distributors/yum/distributor.py\",
    line 143, in publish_repo\n    return self._publisher.publish()\n  File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 558, in publish\n    return self.process_lifecycle()\n  File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 503, in process_lifecycle\n    super(PluginStep, self).process_lifecycle()\n
    \ File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 127, in process_lifecycle\n    step.process()\n  File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 204, in process\n    self._process_block()\n  File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 246, in _process_block\n    self.process_main()\n  File \"/usr/lib/python2.6/site-packages/pulp/plugins/util/publish_step.py\",
    line 766, in process_main\n    os.makedirs(publish_dir_parent, 0750)\n  File \"/usr/lib64/python2.6/os.py\",
    line 157, in makedirs\n    mkdir(name, mode)\nOSError: [Errno 17] File exists:
    '/var/lib/pulp/published/yum/https/repos/Default_Organization/Library/content_view_name/custom/somename'\n"

Instead of checking for existence, then creating, the code should try to create and handle the exception if it exists.

Here is one such occurrence: https://github.com/pulp/pulp/blob/pulp-2.8.5-1/server/pulp/plugins/util/publish_step.py#L904

And there are two others in the same file.

Associated revisions

History

#1 Updated by amacdona@redhat.com almost 5 years ago

  • Triaged changed from No to Yes

#2 Updated by amacdona@redhat.com almost 5 years ago

This is a little tricky, there are edge cases all over the place.

http://stackoverflow.com/questions/273192/how-to-check-if-a-directory-exists-and-create-it-if-necessary/

Be sure to read the comments, the "correct" answer has race conditions. I just encountered this in the Python plugin, and I have opted for this solution (it also has a race condition, but it is very unlikely:

try:
    os.makedirs(path)
except:
    except OSError:
        if not os.path.isdir(path)
            raise

The best solution (exists_ok=True) is only available in Python 3.

#3 Updated by mhrivnak almost 5 years ago

I think the new race condition you're referring to relates to the secondary stat that happens in the isdir() call. That can be eliminated by just checking the errno attribute of the exception to see if the error is due to the dir already existing. It looks something like this:

try:
    os.makedirs(path)
except OSError as e:
    if e.errno != errno.EEXIST:
        raise

#4 Updated by amacdona@redhat.com almost 5 years ago

That is correct, but that ignores a different problem. See the comments: http://stackoverflow.com/a/5032238/3154930

It may be that the an existing path that is not a directory is not actually possible in this case though.

#5 Updated by pmoravec@redhat.com over 4 years ago

FYI pulp-katello hit an identical issue and they fixed it by https://github.com/Katello/pulp-katello/pull/21/files

#6 Updated by bizhang over 4 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to bizhang

#7 Updated by bizhang over 4 years ago

  • Status changed from ASSIGNED to POST

#8 Updated by bizhang over 4 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#9 Updated by semyers over 4 years ago

  • Platform Release set to 2.9.3

#10 Updated by semyers over 4 years ago

  • Status changed from MODIFIED to 5

#11 Updated by pthomas@redhat.com over 4 years ago

  • Status changed from 5 to 6

verified

1. Created 2 repos with --relative-url sharing a the same parent directory
2. Synced & publish the repo at the same time
3. Both repos synced/published successfully

#12 Updated by semyers over 4 years ago

  • Status changed from 6 to CLOSED - CURRENTRELEASE

#15 Updated by bmbouter about 2 years ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF