pyinfra icon indicating copy to clipboard operation
pyinfra copied to clipboard

sed replace quoting (server.crontab fails to update crontab entry sometimes)

Open taranlu-houzz opened this issue 4 years ago • 11 comments

Describe the bug I have a feeling this is caused by the cron_name param. Basically, if I modify the command param for server.crontab but leave cron_name as is, when the deploy is run, it will consider the task to already be present resulting in a no-op.

To Reproduce

  • Run on host: crontab -r
  • Run the below deploy once and verify the crontab updated.
  • Change the command, and run the below deploy again.
  • The crontab will not have been updated.
#!/usr/bin/env python
# -*- coding: utf-8 -*-


"""Deploy to illustrate crontab bug.
"""


###############################################################################
# Imports
###############################################################################


# ==Site-Packages==
from pyinfra.operations import server


###############################################################################
# Main
###############################################################################


server.crontab(
    name="Install test cron task.",
    command="echo 'banana'",
    #  command="echo 'yummy'",
    cron_name="My special task",
    minute="*/10",
)

Expected behavior The crontab should be updated to reflect the changes to the command.

Meta

❯ pyinfra --support
--> Support information:

    If you are having issues with pyinfra or wish to make feature requests, please
    check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues .
    When adding an issue, be sure to include the following:

    System: Darwin
      Platform: macOS-10.14.6-x86_64-i386-64bit
      Release: 18.7.0
      Machine: x86_64
    pyinfra: v1.3.6
    Executable: /Users/----/.virtualenvs/05-hz-pyinfra-d0Faa_8y-py3.9/bin/pyinfra
    Python: 3.9.1 (CPython, Clang 10.0.1 (clang-1001.0.46.4))

taranlu-houzz avatar Feb 20 '21 01:02 taranlu-houzz

Nice spot @taranlu-houzz, little bug in checking the command when using cron_name, fixed in https://github.com/Fizzadar/pyinfra/commit/228460d004175ebd539bba641d27773eb955bc40 for release in 1.3.7 shortly!

Fizzadar avatar Feb 28 '21 12:02 Fizzadar

I'm still seeing this issue on 1.3.7. Did the fix not make it into that release?

taranlu-houzz avatar Mar 06 '21 01:03 taranlu-houzz

It did! Will reopen this to investigate :grimacing:

Fizzadar avatar Mar 06 '21 17:03 Fizzadar

So it looks like this happens due to a error handling the quotes in the command, for example the following file does change the command correctly:

server.crontab(
    name="Install test cron task.",
    command="echo banana",
    #command="echo yummy",
    cron_name="My special task",
    minute="*/10",
)

Still looking into fixing this; certainly looks like it boils down to the sed_replace function but have no solution yet!

Fizzadar avatar Mar 07 '21 11:03 Fizzadar

Figured this out!

  • Fix: https://github.com/Fizzadar/pyinfra/commit/62cdd1e21783fff98389980dc584fed09af8945f
  • Test: https://github.com/Fizzadar/pyinfra/commit/62cdd1e21783fff98389980dc584fed09af8945f
  • Cron specific test: https://github.com/Fizzadar/pyinfra/commit/30c4139fe9025974e066ad9d522abbd5b15a9e89

The example code is now working in my tests :)

Fizzadar avatar Mar 19 '21 18:03 Fizzadar

Awesome! Thank you for looking into that! I'll take a look at the next release.

taranlu-houzz avatar Mar 19 '21 18:03 taranlu-houzz

Have now released those fixes in 1.3.9!

Fizzadar avatar Mar 19 '21 20:03 Fizzadar

Hi @Fizzadar, I am still running into this issue. This is the deploy that I am using:

#!/usr/bin/env python
# -*- coding: utf-8 -*-


"""Pyinfra deploy to add cron tasks for test pipeline."""


###############################################################################
# Imports
###############################################################################


# ==Site-Packages==
from pyinfra import host
from pyinfra.facts.server import (
    Hostname,
    Kernel,
)
from pyinfra.operations import server


###############################################################################
# Globals
###############################################################################


P4_CLIENT_NAME = f"{host.get_fact(Hostname)}_test"


###############################################################################
# Main
###############################################################################


if host.get_fact(Kernel) in ("Darwin", "Linux"):
    server.crontab(
        name="Install {P4_CLIENT_NAME} workspace perforce sync cron task.",
        cron_name=f"Sync {P4_CLIENT_NAME} workspace.",
        command=(
            f"export PATH=<modified path to have access to p4 command on host>; "
            f"cd <path to p4 client on host>; "
            "p4 sync >> "
            f'<log dir path on host>/{P4_CLIENT_NAME}_pipeline.log.`date +"\%Y.\%m.\%d"` '
            "2>&1"
        ),
        minute="5",
    )

When I modify the command param, if the entry already exists in the crontab, then nothing is changed, although the operation reports a "success." Also, modifying the cron_name while keeping the command the same does not result in another entry being added (at least, when the modification is to add some text to the end of the previous name).

taranlu-houzz avatar Aug 20 '21 18:08 taranlu-houzz

Thank you for following up on this @taranlu-houzz; I've not got time to investigate now but I will!

Fizzadar avatar Aug 22 '21 17:08 Fizzadar

Hey @taranlu-houzz, thanks for working on this! I just started playing with pyinfra (thanks @Fizzadar!) and ran into an issue with a cron command containing '&'. First run: 45 2 * * * echo 'true' && echo 'true again' Second run: 45 2 * * * echo 'true' 45 2 * * * echo 'true' && echo 'true again'45 2 * * * echo 'true' && echo 'true again' echo 'true again' Is there anything I can do to help your PR move forward?

fbarthez avatar Jun 04 '22 15:06 fbarthez

Hi @fbarthez, thanks for the info. I do still plan to get back to this PR someday, but unfortunately, I probably wont be able to in the near future. I will take note of the issue you mentioned though when I do finally get back to it.

taranlu-houzz avatar Jun 06 '22 16:06 taranlu-houzz