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

Add an action to restart the plesk-firewalld service and disable the conflicting firewalld service #70

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

SandakovMM
Copy link
Contributor

No description provided.

@SandakovMM SandakovMM self-assigned this Sep 4, 2024
# Don't do startup because the services will be started up after reboot at the end of the script anyway.
return action.ActionResult()

def _revert_action(self) -> action.ActionResult:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe remember initial firewalld state and restore it on revert too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not obvious, because firewalld and plesk-firewall are not meant to operate at the same time. If both are enabled, something is likely wrong.
On the other hand centos2alma was not designed to fix such issues by reverting operations. Therefore, it might be better to return the server to its original state, even if that state is incorrect 🤔

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 have checked it and found a problem. When we start firewalld during the revert process, we lose the SSH connection, causing the revert to stop. I suggest not starting firewalld during the revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, wait a moment. Plesk-firewall already performs the same function. Therefore, this approach may not be the best. I will need to revise the change

Copy link
Contributor

@kpushkaryov kpushkaryov left a comment

Choose a reason for hiding this comment

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

Please rebase before merge.

…vice is enabled

These services conflict with each other, so we need to choose one.
If plesk-firewall is enabled, we assume it has been chosen by the customer, so we keep it enabled
@SandakovMM SandakovMM merged commit 7a13418 into main Sep 6, 2024
3 checks passed
@SandakovMM SandakovMM deleted the disable-firewalld branch September 6, 2024 07:12
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.

2 participants