On start, update git_pillar on second loop

[sathieu]

What does this PR do?

Before this patch, the master would wait git_pillar_update_interval before doing the first update. When this setting is big (we set it to one week at work), this can lead to unfetched repositories.

What issues does this PR fix or reference?

PR #53621

Fixes #56356.

Previous Behavior

The first git_pillar update would run at start time + git_pillar_update_interval.

New Behavior

The first git_pillar update would run at start time.

Tests written?

No

Commits signed with GPG?

No

[Ch3LL]

can we get an associated issue open alongside this PR, so its clear what issue we are fixing and in case anyone else is running into the issue its easily discoverable?

[sathieu]

@Ch3LL Issue created: #56356.

[Ch3LL]

any chance we can get a test case here to ensure it does not regress in the future?

[sathieu]

@Ch3LL I have no idea of how to test this.

[Ch3LL]

I would suggest a unit test here tests/unit/test_master.py and add a new test ckass for the maintenance class and add a test for the run function with git_pillar_update_interval set to different values.

[xeacott]

Hey @sathieu any follow up on this? I can help you write a test if you need for this PR. 😄

[sathieu]

@xeacott Yes, please help me with a test.

[sathieu]

I would suggest a unit test here tests/unit/test_master.py and add a new test ckass for the maintenance class and add a test for the run function with git_pillar_update_interval set to different values.

@Ch3LL OK but how to escape the while True loop?

[xeacott]

I would suggest something like this to get you started...

class MaintenanceTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
    """
    TestCase for salt.master.Maintenance class
    """

    def setUp(self):
        self.opts = salt.config.master_config(None)
        self.main_class = salt.master.Maintenance(self.opts)

    def tearDown(self):
        del self.main_class

    def test_run_func(self):
        """
        Test the run function inside Maintenance class.
        """
        with patch("salt.utils.process", autospec=True):
            with patch("salt.master", autospec=True):
                self.main_class.run()

[sathieu]

@Ch3LL Please review again. There are (passing 😄) tests now!

[sathieu]

Also @xeacott please review ...

[xeacott]

Oh yes will do, thanks for writing those tests!

[waynew]

@xeacott Tests look good to me - I'd double check the docs to make sure we don't specify a different behavior (I'm guessing not).

[sathieu]

@xeacott You've marked this PR Merge Ready. I've now rebased and fixed pre-commit and Pylint. Hope you merge it now ...