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: Remove firewalld zone instructions #17389

Merged

Conversation

polarathene
Copy link
Contributor

@polarathene polarathene commented May 23, 2023

Proposed changes

Remove instructions for migrating to the docker zone, which landed in moby releases since 2020H2.

A recent PR commit modified these docs for v23 to remove the 20.10.0 qualifier (which lost that context).


Additional Context

This makes the current information confusing, and implies a user should run similar commands to get integration with firewalld.

  • AFAIK, the feature review discussed a future improvement to support changing the zone, or assigning user-defined networks to custom zones, but this does not appear to have been pursued since.
  • Removing docker0 from the docker zone, will reassign it back, or if assigned to another zone with --permanent cause a failure for dockerd starting due to a conflicting zone already assigned docker0.

Thus given the recent change, these instructions should be dropped, or additional context provided (note: third-party guides created in 2023 can be found advising to assign docker0 to trusted zone).

@netlify
Copy link

netlify bot commented May 23, 2023

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit a452d69
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/64f0015f2a5a540008c5eae4
😎 Deploy Preview https://deploy-preview-17389--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aevesdocker aevesdocker added the area/engine Issue affects Docker engine/daemon label May 23, 2023
@dvdksn
Copy link
Collaborator

dvdksn commented May 23, 2023

@akerouanton can you PTAL? TIA!

@dvdksn dvdksn requested a review from akerouanton May 23, 2023 14:59
@dvdksn dvdksn added area/networking Relates to anything around networking area/security labels May 23, 2023
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

LGTM

@docker-robot
Copy link

docker-robot bot commented Aug 31, 2023

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

These were related to migrating to the `docker` zone support in new moby releases since integration landed in 2020H2.
@polarathene polarathene force-pushed the docs/firewalld-remove-migration-notes branch from 0954c07 to a452d69 Compare August 31, 2023 02:56
@polarathene
Copy link
Contributor Author

Since the PR was raised, the original file was deleted and the content for firewalls relocated.

Nothing has changed since the LGTM beyond adjusting to the new filename.

@polarathene
Copy link
Contributor Author

@dvdksn @neersighted would it be possible to get this merged before it becomes stale again? 😅 Thanks :)

@neersighted
Copy link
Member

neersighted commented Sep 28, 2023

I think we were waiting for @akerouanton to find time/bandwidth; so friendly poke 😀

Copy link
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

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

LGTM

@dvdksn dvdksn merged commit 33c36bc into docker:main Oct 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine Issue affects Docker engine/daemon area/networking Relates to anything around networking area/security lifecycle/frozen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants