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

Python 3.8 and macOS switching default multi-processing to spawn #55847

Closed
s0undt3ch opened this issue Jan 12, 2020 · 0 comments · Fixed by #57028
Closed

Python 3.8 and macOS switching default multi-processing to spawn #55847

s0undt3ch opened this issue Jan 12, 2020 · 0 comments · Fixed by #57028
Assignees
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Phosphorus v3005.0 Release code name and version severity-high 2nd top severity, seen by most users, causes major problems

Comments

@s0undt3ch
Copy link
Collaborator

s0undt3ch commented Jan 12, 2020

Description of Issue

On macOS and under Python 3.8, the default multi-processing method switched from fork to spawn.

Salt is prepared for spawn but only for windows.
The salt code base needs to be updated to check the default multi-processing method instead of checking where(Windows, Linux, macOS) it's running in order to support non Windows platforms which default to spawn.

Versions Report

All Salt versions up until Salt officially supports Python >= 3.8

Links

https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
https://bugs.python.org/issue33725

@s0undt3ch s0undt3ch added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Jan 12, 2020
@s0undt3ch s0undt3ch added this to the Approved milestone Jan 12, 2020
@sagetherage sagetherage added the Bug broken, incorrect, or confusing behavior label Mar 24, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label Mar 24, 2020
@sagetherage sagetherage assigned s0undt3ch and unassigned Ch3LL May 5, 2020
@sagetherage sagetherage removed the ZRelease-Sodium retired label label May 22, 2020
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label May 22, 2020
@sagetherage sagetherage added the severity-high 2nd top severity, seen by most users, causes major problems label Jul 6, 2020
@sagetherage sagetherage modified the milestones: Approved, Magnesium Jul 14, 2020
@sagetherage sagetherage added Aluminium Release Post Mg and Pre Si and removed Magnesium Mg release after Na prior to Al labels Oct 8, 2020
@sagetherage sagetherage modified the milestones: Magnesium, Aluminium Oct 8, 2020
@sagetherage sagetherage added Silicon v3004.0 Release code name and removed Aluminium Release Post Mg and Pre Si phase-plan labels Mar 24, 2021
@sagetherage sagetherage modified the milestones: Aluminium, Silicon Mar 24, 2021
@sagetherage sagetherage modified the milestones: Silicon, Phosphorus Aug 19, 2021
@sagetherage sagetherage added Phosphorus v3005.0 Release code name and version and removed Silicon v3004.0 Release code name labels Aug 19, 2021
s0undt3ch added a commit to s0undt3ch/salt that referenced this issue May 5, 2022
Fixes saltstack#55847

Signed-off-by: Pedro Algarvio <[email protected]>
garethgreenaway pushed a commit that referenced this issue May 20, 2022
Fixes #55847

Signed-off-by: Pedro Algarvio <[email protected]>
garethgreenaway pushed a commit to garethgreenaway/salt that referenced this issue May 23, 2022
Fixes saltstack#55847

Signed-off-by: Pedro Algarvio <[email protected]>
Ch3LL pushed a commit that referenced this issue May 26, 2022
* Making the retry state system feature available when parallel is set to True.

* swapping out using time.sleep Salt function for Python time.sleep.

* Skipping parallel tests on Windows.

* Fixing items, values, and keys functions in the data module.  Moving integration to functional tests.  Converting unit test over to pytest.

* adding changleog.

* fixes #62044 add ignore_missing to file.comment state

* fixes #61662 fix file.comment reports changes in test mode

* add 61662.fixed changelog

* add final test mode run after comment is successful

* fix uncomment_regex

* fix uncomment_regex

* Try not to trigger the GLIBC race condition.

See https://sourceware.org/bugzilla/show_bug.cgi?id=19329

Fixes #62071

Signed-off-by: Pedro Algarvio <[email protected]>

* Add missing CLI example to ``network.fqdns``

Signed-off-by: Pedro Algarvio <[email protected]>

* Sleep between submitting new jobs to thread pool

Signed-off-by: Pedro Algarvio <[email protected]>

* Sleep before trying to resolve instead

Signed-off-by: Pedro Algarvio <[email protected]>

* Shorter sleeps

* fixes #61944 fixed backslash literal bytestring

* When states are running in parallel, ensure that the total run time produced by the highstate outputter takes that into account.

* removing unrelated change

* Adding changelog.

* Skip parallel test on Windows.

* skip pdbedit unit tests on ubuntu 2204 because md4 is not supported as a hash type

* refresh db at the beginning for destructive aptpkg functional tests

* fix failing test_pkgrepo_managed_absent and actually capture and keep signed-by information in salt/modules/aptpkg.py custom implementation of apt repository management

* Update tests/unit/modules/test_pdbedit.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Disable hack to force allowing forking on macOS

* Use the newer nox platforms file which installs Py3.9

Signed-off-by: Pedro Algarvio <[email protected]>

* Support macOS defaulting to spawn as the default multiprocessing method

* Remove unused attribute

* Switch to ``salt.utils.platform.spawning_platform()``

Signed-off-by: Pedro Algarvio <[email protected]>

* Reconstruct client instances on spawning platforms

* Also skip this test for now on macOS

Signed-off-by: Pedro Algarvio <[email protected]>

* Reconstruct the ``State`` class on spawned processes.

Signed-off-by: Pedro Algarvio <[email protected]>

* Force a reference to ``salt.ext.tornado.iostream.StreamClosedError`` to avoid ``NameError`` issues

Signed-off-by: Pedro Algarvio <[email protected]>

* Skip, for now, problematic tests on spawning platforms

Signed-off-by: Pedro Algarvio <[email protected]>

* DeltaProxy minions do not work on spawning platforms

Signed-off-by: Pedro Algarvio <[email protected]>

* Avoid a ``NameError`` when failing to initialize the process list

Signed-off-by: Pedro Algarvio <[email protected]>

* Use Python 3.9 on macOS

Signed-off-by: Pedro Algarvio <[email protected]>

* Drop requirements on Darwin for Py<3.9

Signed-off-by: Pedro Algarvio <[email protected]>

* Same test run timeout as windows

Signed-off-by: Pedro Algarvio <[email protected]>

* Don't shell out! Access the database directly and properly escape values.

Signed-off-by: Pedro Algarvio <[email protected]>

* Fixes for the new macOS Catalina and BigSur CICD images

Signed-off-by: Pedro Algarvio <[email protected]>

* Migrate ``integration.modules.test_mac_pkgutil`` to PyTest and functional tests.

Fixes #60819

Signed-off-by: Pedro Algarvio <[email protected]>

* Add changelog for #57742

Fixes #57742

Signed-off-by: Pedro Algarvio <[email protected]>

* Add changelog entry.

Fixes #55847

Signed-off-by: Pedro Algarvio <[email protected]>

* initial Proxy Module

* inital pass of restconf states

* adding codeauthor

* adding restconf module

* doc update

* doc update

* doc update

* add requirements chefk to states module

* doc update

* doc update

* doc update

* add changelog entry

* Update salt/proxy/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* Update salt/proxy/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* remove noqa statements

* docs update & opts defaults

* docs update

* error checks + https transport method

* docs update

* docs update

* doing a fix for commit check

* adding test and updating modules

* removing old code

* DRY

* docs update

* output updates

* prepping for deepdiff switch

* states testing

* more tests

* changed changelog from fixed to added

* force pre-commit checks

* new style of diff output that is readable

* updating tests and output

* updating output style to make YAML diff more readable

* add test for restconf module

* adjusting based on pr review

* adjusting based on pr review

* adjusting true/false verify based on pr review

* change uri to path and clean args list

Co-authored-by: Wayne Werner <[email protected]>

* adjusting uri to path based on pr review

* adjusting capabilities path based on pr review

* Update salt/modules/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* adjusting val based on pr review

* caps to confirm to RFC 8040

* WIP: refactor based on feedback

* WIP: changed results to be any kind of falsey val

* linting - pre-commit

* updating docs

* Update salt/states/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* updating style

* fixes for lint and nox pass

* fixing

* blacken

* Update restconf.py

* add cli examples

* change logging type

* add pytest skip for ordereddict issue with legacy python

* add sys library for pytest mark skipif check

* pre-commit fixes

* Update salt/proxy/restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update salt/states/restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/modules/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/states/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/proxy/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* pre-commit fix

* update logging

* Update restconf.py

* some rest apis dont follow the standard correctly (cisco) and need another header

* fixes #61946 sync_after_install immutabledict error

* add configurable sync sleep for create

* add testing around the particular sync command called

* Adds the ability to get file version information on Windows

- Adds file.version that will get just the version
- Adds file.version_details to get additional information
- Adds tests
- Adds changelog

@amalaguti originally submitted a PR for this: #59770, but he has
since deleted his branch. Recognizing his contribution here.

* Update states.chef for version 16.x and 17.x Chef Infra Client output

* Support previous Chef version plus version 16.x and 17.x output.

* Remove legacy chef unit test

* Add unit tests for chef state

These tests cover the entire state, but are only concerned with the
ret["result"] of the outputs. It would be a good idea to test other
outputs as well - these tests could be modified to include assertions
about the other parts of ret. (Obviously they would need to be slightly
renamed. Maybe to `..._return_expected_ret`

* Update salt-bootstrap to 2022.03.15 release

* Update state.py

Fixing lint.

* Running pre-commit manually.

* swap out salt.utils.platform.is_windows for salt.utils.platform.spawning_platform.  Bump up duration for test_retry_option_success_parallel to 30 seconds to account for test running on Mac OS.

Co-authored-by: Megan Wilhite <[email protected]>
Co-authored-by: nicholasmhughes <[email protected]>
Co-authored-by: Pedro Algarvio <[email protected]>
Co-authored-by: emmadionne1 <[email protected]>
Co-authored-by: MKLeb <[email protected]>
Co-authored-by: Caleb Beard <[email protected]>
Co-authored-by: Pedro Algarvio <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Jamie (Bear) Murphy <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Wayne Werner <[email protected]>
Co-authored-by: Cassandra Durnford <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Jamie (Bear) Murphy <[email protected]>
Co-authored-by: Twangboy <[email protected]>
Co-authored-by: Eric Ham <[email protected]>
Co-authored-by: Wayne Werner <[email protected]>
Co-authored-by: krionbsd <[email protected]>
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 Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Phosphorus v3005.0 Release code name and version severity-high 2nd top severity, seen by most users, causes major problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants