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

docs: Add note about using previous: replaced and temporary service failures #141

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

richm
Copy link
Contributor

@richm richm commented Apr 25, 2023

Add a note to the README about the use of previous: replaced and that it can
cause temporary service outages to the node being managed.
#138

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3abd32d) 53.39% compared to head (f86e074) 53.39%.

❗ Current head f86e074 differs from pull request most recent head bb9acf3. Consider uploading reports for the commit bb9acf3 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #141   +/-   ##
=======================================
  Coverage   53.39%   53.39%           
=======================================
  Files           2        2           
  Lines         796      796           
=======================================
  Hits          425      425           
  Misses        371      371           
Flag Coverage Δ
sanity ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link

@myllynen myllynen left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

README.md Outdated
*WARNING*: When using this option, there's a small time window when firewall is
being reset and all new connections to the system are rejected. Applying changes
with this option in production might cause temporary service failures during the
operation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth adding that existing connections will still pass traffic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please see the new text

…ailures

Add a note to the README about the use of `previous: replaced` and that it can
cause temporary service outages to the node being managed.
linux-system-roles#138
@richm richm force-pushed the readme-reset-loses-connection branch from f86e074 to bb9acf3 Compare April 26, 2023 13:35
Copy link
Collaborator

@erig0 erig0 left a comment

Choose a reason for hiding this comment

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

lgtm. thanks!

@richm richm merged commit 02d524b into linux-system-roles:main Apr 26, 2023
@richm richm deleted the readme-reset-loses-connection branch April 26, 2023 13:50
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.

3 participants