Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

makedir bug in the file module #34478

Closed
hujunya opened this issue Jul 6, 2016 · 5 comments
Closed

makedir bug in the file module #34478

hujunya opened this issue Jul 6, 2016 · 5 comments
Labels
Bug broken, incorrect, or confusing behavior Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix P4 Priority 4 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@hujunya
Copy link
Contributor

hujunya commented Jul 6, 2016

https://github.com/saltstack/salt/blob/develop/salt/modules/file.py#L4599

def makedirs_(path,
              user=None,
              group=None,
              mode=None):
    '''
    Ensure that the directory containing this path is available.
    .. note::
        The path must end with a trailing slash otherwise the directory/directories
        will be created up to the parent directory. For example if path is
        ``/opt/code``, then it would be treated as ``/opt/`` but if the path
        ends with a trailing slash like ``/opt/code/``, then it would be
        treated as ``/opt/code/``.
    CLI Example:
    .. code-block:: bash
        salt '*' file.makedirs /opt/code/
    '''
    path = os.path.expanduser(path)

    # walk up the directory structure until we find the first existing
    # directory
    dirname = os.path.normpath(os.path.dirname(path))

    if os.path.isdir(dirname):
        # There's nothing for us to do
        msg = 'Directory \'{0}\' already exists'.format(dirname)
        log.debug(msg)
        return msg

    if os.path.exists(dirname):
        msg = 'The path \'{0}\' already exists and is not a directory'.format(
            dirname
        )
        log.debug(msg)
        return msg

    directories_to_create = []
    while True:
        if os.path.isdir(dirname):
            break

        directories_to_create.append(dirname)
        dirname = os.path.dirname(dirname)

    # create parent directories from the topmost to the most deeply nested one
    directories_to_create.reverse()
    for directory_to_create in directories_to_create:
        # all directories have the user, group and mode set!!
        log.debug('Creating directory: %s', directory_to_create)
        mkdir(directory_to_create, user=user, group=group, mode=mode)

if the directory containing the path is not available, like path = '/notexist/' on Linux or path = 'N:' on Windows, then the program will run into the while loop, the length of the directories_to_create list will increse, until a MemoryError is raised.

while True:
        if os.path.isdir(dirname):
            break

        directories_to_create.append(dirname)
        dirname = os.path.dirname(dirname)
 [salt.state       ][ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "C:\salt\bin\lib\site-packages\salt\state.py", line 1542, in call
    **cdata['kwargs'])
  File "C:\salt\bin\lib\site-packages\salt\states\file.py", line 1660, in directory
    name, user=user, group=group, mode=dir_mode
  File "C:\salt\bin\lib\site-packages\salt\modules\file.py", line 3445, in makedirs_
    directories_to_create.append(dirname)
MemoryError
@Ch3LL
Copy link
Contributor

Ch3LL commented Jul 6, 2016

@hujunya i'm not able to replicate this behavior with the following test case:

 ch3ll@localhost  /tmp/notsrvsalt  sudo salt-call --local file.makedirs '/doesnotexist/'
local:
    None

Maybe im misunderstanding something. Thanks in advance for any clarification

@Ch3LL Ch3LL added info-needed waiting for more info cannot-reproduce cannot be replicated with info/context provided labels Jul 6, 2016
@Ch3LL Ch3LL added this to the Blocked milestone Jul 6, 2016
@hujunya
Copy link
Contributor Author

hujunya commented Jul 7, 2016

@Ch3LL Could you please try this on Windows server ?
file.makedirs on a not exist disk, like salt-call.bat --local file.makedirs "N:"

@Ch3LL
Copy link
Contributor

Ch3LL commented Sep 9, 2016

@hujunya would you mind trying the fix in #35849 ?

@Ch3LL Ch3LL added Execution-Module Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P4 Priority 4 Platform Relates to OS, containers, platform-based utilities like FS, system based apps and removed cannot-reproduce cannot be replicated with info/context provided info-needed waiting for more info labels Sep 9, 2016
@Ch3LL Ch3LL modified the milestones: Approved, Blocked Sep 9, 2016
@Ch3LL Ch3LL added the fixed-pls-verify fix is linked, bug author to confirm fix label Sep 9, 2016
@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 11, 2016

ping @hujunya any chance to attempt using the proposed PR?

@hujunya
Copy link
Contributor Author

hujunya commented Oct 12, 2016

@Ch3LL the PR #35849 can fix this bug.
Thank you so much !

@hujunya hujunya closed this as completed Oct 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix P4 Priority 4 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

2 participants