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

Initial POC of RabbitMQ transport #60753

Closed
wants to merge 4 commits into from

Conversation

devkits
Copy link
Contributor

@devkits devkits commented Aug 17, 2021

What does this PR do?

Initial POC implementation of RabbitMQ as a new transport layer.
(work-in-progress, but looking for early feedback before the review gets too large).

  • there are a number of TODOs in this review; all should be addressed before review is ready for prime time
  • there are gaps in the implementation that will be addressed. This work is tracked in SSC-978
  • a number of edge cases need to be tested/addressed (e.g. connection error recovery, etc.)
  • added functional tests
  • added integration tests

Testing Done: functional tests with on master and one minion

What issues does this PR fix or reference?

Fixes: VMware SSC-978

Merge requirements satisfied?

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

Commits signed with GPG?

No

(work-in-progress, but looking for early feedback before the review gets too large).

  - there are a number of TODOs in this review; all should be addressed before review is ready for prime time
  - there are gaps in the implementation that will be addressed. This work is tracked in SSC-978
  - a number of edge cases need to be tested/addressed (e.g. connection error recovery, etc.)
  - added functional tests
  - added integration tests

Testing Done: functional tests with on master and one minion
@devkits devkits requested review from s0undt3ch and a team as code owners August 17, 2021 16:24
@welcome
Copy link

welcome bot commented Aug 17, 2021

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 saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!


ZMQ's ipc transport not supported on Windows
"""
opts = dict(salt_master.config.copy(), ipc_mode="ipc", pub_hwm=0)
with PubServerChannelProcess(opts, salt_minion.config.copy()) as server_channel:
send_num = 10000
send_num = 4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should undo this change. This number was reduced to easier testing and comparison of rabbitmq and zeromq.

…ransport is in POC; so we kip RMQ tests (for now) until RMQ dependencies are dealt with in the CI/CD pipeline.
…l". RMQ transport is in POC; so we skip RMQ tests (for now) until RMQ dependencies are dealt with in the CI/CD pipeline.
@devkits devkits marked this pull request as draft August 20, 2021 15:12
@devkits
Copy link
Contributor Author

devkits commented Aug 20, 2021

Closing in favour of #60775

@devkits devkits closed this Aug 20, 2021
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.

1 participant