salt icon indicating copy to clipboard operation
salt copied to clipboard

Bugfix 67214 minion swarm

Open dnessett opened this issue 9 months ago • 12 comments

What does this PR do?

Fixes bugs in tests/minionswarm.py that were preventing it to run.

What issues does this PR fix or reference?

Fixes #67214

Previous Behavior

minionswarm failed to run

New Behavior

minionswarm now runs correctly

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

  • [ ] Docs
  • [X] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
  • [X] Tests written/updated

Commits signed with GPG?

No

dnessett avatar Mar 01 '25 20:03 dnessett

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

welcome[bot] avatar Mar 01 '25 20:03 welcome[bot]

Looks like there are some pre-commit failures.

twangboy avatar May 14 '25 21:05 twangboy

Looks like there are some pre-commit failures.

OK. I will take a look at it. It may take some time since I am reorganizing my development environment.

dnessett avatar May 14 '25 21:05 dnessett

I am very new to salt development, so I very possibly am missing something, I ran nox -e lint-tests in bugfix-67214, which is what is pushed to https://github.com/dnessett/salt and what is the basis of the PR. The results are:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ nox -e lint-tests
nox > Running session lint-tests
nox > Creating virtual environment (virtualenv) using python3 in .nox/lint-tests
nox > python -m pip install --progress-bar=off -U setuptools pip wheel
nox > python -m pip install --progress-bar=off -r requirements/static/ci/py3.10/linux.txt -r requirements/static/ci/py3.10/lint.txt
nox > pylint --rcfile=.pylintrc --disable=I tests/

------------------------------------
Your code has been rated at 10.00/10

nox > Session lint-tests was successful.

To check I was in the correct branch, I ran:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ git log
commit aeb11b3e615066804d9debd581b8663234a4bffc (HEAD -> bugfix-67214, origin/bugfix-67214)
Author: dnessett <[email protected]>
Date:   Sat Mar 1 13:24:48 2025 -0700

    Eliminated some trailing whitespace errors revealed by nox -e lint-tests

commit ed91f4ceecfb92f773096951dd8a2ac4ee1f73a8
Author: dnessett <[email protected]>
Date:   Sat Mar 1 12:24:11 2025 -0700

    added changelog file

commit 05f5e132d2d90844df3fbc432802a864c42c8dc2
Author: dnessett <[email protected]>
Date:   Fri Feb 28 13:19:32 2025 -0700

    cleaned up some problems with the parser.add_options and added a
    clarificationn to the config-dir help text.

commit 85afd494938c53934e941eb7cb658a9cbcd42062
Author: dnessett <[email protected]>
Date:   Thu Feb 27 11:31:06 2025 -0700

    Fix bugs in minionswarm. Updated help with significant enhancements. Added username to salt-master command. Added ability to turn off open_mode.

commit 2530c8f1d5b1440b70860d4dea453b6657bdf92b
Author: Daniel A. Wozniak <[email protected]>
Date:   Wed May 7 01:33:13 2025 -0700

    Do not fail when cached grains no longer exit

and:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ git branch
* bugfix-67214
  master

I then checked the changelog directory in case the changelog file was faulty, but it seems not:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt/changelog$ cat 67214.fixed.md 
Fixed bugs in minionswarm and added ability turn off open-mode.

So, I am stuck finding out what is causing pre-commit to fail.

dnessett avatar May 18 '25 04:05 dnessett

lint-tests/lint-salt run pylint only. The pre-commit hooks that are failing are black/isort though. You'd run them via

pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py

In general, you should consider installing the pre-commit hook (the git hook) in your local salt checkout for pre-commit (the program) via pre-commit install (or python -m pre_commit install, if you're in a venv and want to ensure you're using the pre-commit that's installed inside the venv). This would ensure you cannot commit before all hooks are fixed, thus avoiding the failing check in CI entirely.

lkubb avatar May 18 '25 11:05 lkubb

lint-tests/lint-salt run pylint only. The pre-commit hooks that are failing are black/isort though. You'd run them via

pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py

In general, you should consider installing the pre-commit hook (the git hook) in your local salt checkout for pre-commit (the program) via pre-commit install (or python -m pre_commit install, if you're in a venv and want to ensure you're using the pre-commit that's installed inside the venv). This would ensure you cannot commit before all hooks are fixed, thus avoiding the failing check in CI entirely.

Thanks.

dnessett avatar May 18 '25 13:05 dnessett

lint-tests/lint-salt run pylint only. The pre-commit hooks that are failing are black/isort though. You'd run them via pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py In general, you should consider installing the pre-commit hook (the git hook) in your local salt checkout for pre-commit (the program) via pre-commit install (or python -m pre_commit install, if you're in a venv and want to ensure you're using the pre-commit that's installed inside the venv). This would ensure you cannot commit before all hooks are fixed, thus avoiding the failing check in CI entirely. pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py Thanks.

Sorry for the delay, I had some emergencies to resolve in another area of my volunteer work.

When I run: pre-commit run black/isort The result is:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py
black................................................(no files to check)Skipped
(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py
isort................................................(no files to check)Skipped
(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ 

There doesn't seem to be any errors from black or isort. Perhaps this is because the two files in question have been committed and so black/isort doesn't regard them as changed? (This is a guess, since I don't know what black/iisort checks for)

dnessett avatar May 21 '25 19:05 dnessett

@dnessett $PWD being ~/bugfix-67214/salt indicates to me you're inside the salt subdirectory of the repository when running the command, not in the repository root, which the paths I gave you are relative to. From pre-commit's perspective, they don't exist, so there's nothing to do. It's not about being committed, this generally works.

lkubb avatar May 21 '25 20:05 lkubb

@dnessett $PWD being ~/bugfix-67214/salt indicates to me you're inside the salt subdirectory of the repository when running the command, not in the repository root, which the paths I gave you are relative to. From pre-commit's perspective, they don't exist, so there's nothing to do. It's not about being committed, this generally works.

I'm in the git source directory when I execute the black/isort functions. I am reorganizing my development environment, so I cloned a copy of my github repository to a directory in my home directory in order to address this problem while the reorganization is in process.

(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ ls
AUTHORS       CODE_OF_CONDUCT.md  doc          noxfile.py   pyproject.toml  rfcs         setup.cfg    tests
changelog     conf                LICENSE      pkg          pytest.ini      salt         setup.py     tools
CHANGELOG.md  CONTRIBUTING.rst    MANIFEST.in  POLICY.rst   README.rst      scripts      SUPPORT.rst
cicd          DEPENDENCIES.md     NOTICE       __pycache__  requirements    SECURITY.md  templates
(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ pre-commit run black --files tests/minionswarm.py,tests/support/runtests.py
black................................................(no files to check)Skipped
(salt) dan@Eclipse-salt:~/bugfix-67214/salt$ pre-commit run isort --files tests/minionswarm.py,tests/support/runtests.py
isort................................................(no files to check)Skipped
(salt) dan@Eclipse-salt:~/bugfix-67214/salt$

Just to demonstrate the files are in the right place:

(salt) dan@Eclipse-salt:~/bugfix-67214/salt/tests$ ls
buildpackage.py      eventlisten.py    __init__.py     modparser.py  runtests.py      support       zypp_plugin.py
committer_parser.py  eventlisten.sh    integration     packdump.py   saltsh.py        unit
conftest.py          filename_map.yml  minionswarm.py  pytests       salt-tcpdump.py  wheeltest.py
(salt) dan@Eclipse-salt:~/bugfix-67214/salt/tests$

(salt) dan@Eclipse-salt:~/bugfix-67214/salt/tests$ cd support
(salt) dan@Eclipse-salt:~/bugfix-67214/salt/tests/support$ ls
case.py         gitfs.py     kernelpkg.py  napalm.py  pkg.py       sminion.py  win_installer.py
cli_scripts.py  helpers.py   mixins.py     netapi.py  pytest       unit.py     xmlunit.py
events.py       __init__.py  mock.py       paths.py   runtests.py  virt.py     zfs.py
(salt) dan@Eclipse-salt:~/bugfix-67214/salt/tests/support$ 


dnessett avatar May 21 '25 21:05 dnessett

OK, sorry for the confusion!

Went back and checked, it seems I messed up and copied the wrong format. Please replace the comma with a space like this:

pre-commit run black --files tests/minionswarm.py tests/support/runtests.py
pre-commit run isort --files tests/minionswarm.py tests/support/runtests.py

lkubb avatar May 22 '25 00:05 lkubb

OK, sorry for the confusion!

Went back and checked, it seems I messed up and copied the wrong format. Please replace the comma with a space like this:

pre-commit run black --files tests/minionswarm.py tests/support/runtests.py
pre-commit run isort --files tests/minionswarm.py tests/support/runtests.py

No problem. Thanks for the help.

dnessett avatar May 22 '25 01:05 dnessett

OK. The changes made by black and isort I have pushed to https://github.com/dnessett/salt.

dnessett avatar May 22 '25 01:05 dnessett