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

[develop, neon] fix(cron.py): ensure get_entry includes the special entries as well #55016

Closed

Conversation

myii
Copy link
Contributor

@myii myii commented Oct 16, 2019

What does this PR do?

CC: @mchugh19.

Follow-up to #52441, which was introduced due to a PR in SaltStack Formulas: saltstack-formulas/cron-formula#4.

While working through that cron-formula PR, found that special entries are not included by get_entry.

So the formula creates the following crontab for root:

# Lines below here are managed by Salt, do not edit
# comment1 SALT_CRON_IDENTIFIER:task1
0 0 7 1 6 echo test > /tmp/test
# comment4 SALT_CRON_IDENTIFIER:task4
*/5 * * * * echo task4 > /tmp/test4
# comment3 SALT_CRON_IDENTIFIER:task3
@hourly echo task3 > /tmp/test3

task3 isn't being found with the current version of cron.get_entry:

$ sudo salt-call --config-dir=/tmp/kitchen/etc/salt cron.get_entry root task3
local:
    False

Found only task1 and task4 accessible by get_entry:

[
    {
        "minute": "0",
        "hour": "0",
        "daymonth": "7",
        "month": "1",
        "dayweek": "6",
        "identifier": "task1",
        "cmd": "echo test > /tmp/test",
        "comment": "comment1",
        "commented": false
    },
    {
        "minute": "*/5",
        "hour": "*",
        "daymonth": "*",
        "month": "*",
        "dayweek": "*",
        "identifier": "task4",
        "cmd": "echo task4 > /tmp/test4",
        "comment": "comment4",
        "commented": false
    }
]

list_tab(user) contains it under the special key:

{
    "pre": [],
    "crons": [
        {
            "minute": "0",
            "hour": "0",
            "daymonth": "7",
            "month": "1",
            "dayweek": "6",
            "identifier": "task1",
            "cmd": "echo test > /tmp/test",
            "comment": "comment1",
            "commented": false
        },
        {
            "minute": "*/5",
            "hour": "*",
            "daymonth": "*",
            "month": "*",
            "dayweek": "*",
            "identifier": "task4",
            "cmd": "echo task4 > /tmp/test4",
            "comment": "comment4",
            "commented": false
        }
    ],
    "special": [
        {
            "spec": "@hourly",
            "cmd": "echo task3 > /tmp/test3",
            "identifier": "task3",
            "comment": "comment3",
            "commented": false
        }
    ],
    "env": []
}

So after applying the fix:

$ sudo salt-call --config-dir=/tmp/kitchen/etc/salt cron.get_entry root task3
[
    {
        "minute": "0",
        "hour": "0",
        "daymonth": "7",
        "month": "1",
        "dayweek": "6",
        "identifier": "task1",
        "cmd": "echo test > /tmp/test",
        "comment": "comment1",
        "commented": false
    },
    {
        "minute": "*/5",
        "hour": "*",
        "daymonth": "*",
        "month": "*",
        "dayweek": "*",
        "identifier": "task4",
        "cmd": "echo task4 > /tmp/test4",
        "comment": "comment4",
        "commented": false
    },
    {
        "spec": "@hourly",
        "cmd": "echo task3 > /tmp/test3",
        "identifier": "task3",
        "comment": "comment3",
        "commented": false
    }
]
local:
    ----------
    cmd:
        echo task3 > /tmp/test3
    comment:
        comment3
    commented:
        False
    identifier:
        task3
    spec:
        @hourly

The saltcheck tests using this fixed version of cron.py are all working as expected:

What issues does this PR fix or reference?

Tests written?

No

Commits signed with GPG?

Yes

@myii myii requested a review from a team as a code owner October 16, 2019 00:48
@ghost ghost requested a review from Ch3LL October 16, 2019 00:48
@myii
Copy link
Contributor Author

myii commented Nov 13, 2019

Note, this depends on the merge of #52441 into master.

@mchugh19
Copy link
Contributor

Referenced Neon PR has master port waiting in #54985

@bryceml
Copy link
Contributor

bryceml commented Nov 19, 2019

@myii
Copy link
Contributor Author

myii commented Nov 19, 2019

@bryceml Sure, this is based on #54985 (which is #52441 being ported to master), so as soon as that's done, I'll rebase this accordingly.

@dwoz
Copy link
Contributor

dwoz commented Dec 2, 2019

@myii I'm closing this since we're no longer accepting PRs to develop and you will port it after #54985 is merged.

@dwoz dwoz closed this Dec 2, 2019
@myii myii mentioned this pull request Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants