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

Do not check if Port is already in use with --validate-config #3056

Closed
1 task done
mnietz opened this issue May 3, 2024 · 6 comments
Closed
1 task done

Do not check if Port is already in use with --validate-config #3056

mnietz opened this issue May 3, 2024 · 6 comments
Labels

Comments

@mnietz
Copy link

mnietz commented May 3, 2024

What happened?

It's not a Bug but a Feature-Request.

When validating the Patroni-Config on a running cluster it exit with code 1 and the following messages:

$ patroni --validate-config /etc/patroni/patroni.yml ; echo $?
restapi.listen 0.0.0.0:8008 didn't pass validation: Port 8008 is already in use.
postgresql.listen 0.0.0.0:5432 didn't pass validation: Port 5432 is already in use.
1

This makes it not possible to validate the config prior to a restart, for example within an ansible-template using validate:

https://docs.ansible.com/ansible/latest/collections/ansible/builtin/template_module.html#parameter-validate

It would be pretty good to have a check/validate mode like nginx -t or haproxy -c that could be integrated into automation-tools.

How can we reproduce it (as minimally and precisely as possible)?

Just validate the config of an already running cluster

What did you expect to happen?

exit with 0 if config is valid

Patroni/PostgreSQL/DCS version

  • Patroni version: 3.3
  • PostgreSQL version: 17
  • DCS (and its version): etcd 3.5.7

Patroni configuration file

n/a

patronictl show-config

n/a

Patroni log files

n/a

PostgreSQL log files

n/a

Have you tried to use GitHub issue search?

  • Yes

Anything else we need to know?

No response

@mnietz mnietz added the bug label May 3, 2024
@sahilnap06
Copy link
Contributor

sahilnap06 commented Jun 20, 2024

I would like to work on this.

We can have a suppressor flag that can suppress the port-in-use failure.

@mnietz Would you say the first option satisfies the requirement?
CC: @CyberDem0n

@CyberDem0n
Copy link
Member

@sahilnaphade sorry for a late reply.
I think we can just ignore listen parameter in the validate_host_port() function.
Creating a separate argument to modify behavior could be tricky, because of the way how schema validator is implemented.

@sahilnap06
Copy link
Contributor

sahilnap06 commented Aug 20, 2024

@CyberDem0n If I understand correctly, are we considering ignoring the listen parameter for all schema validation cases?

@CyberDem0n
Copy link
Member

@sahilnaphade Passing down parameters to Schema.validate() could be tricky.
Maybe we can create a global variable in the validator.py and change it based on additional parameters passed with patroni --validate-config, either directly, or by using a wrapper function.

What should be default behavior (to check listen or not) is a good question. Now I think we better stick by default with old behavior for backward compatibility.

@sahilnap06
Copy link
Contributor

sahilnap06 commented Aug 27, 2024

@CyberDem0n Sounds good. I have raised the PR for this issue, I tested the code exactly as @mnietz explained, no error is thrown when the flag is specified.

I have used the global variable approach, which, in my opinion would be easily extendable to any other use case we might have.

Please review and let me know if you want any further changes made: #3138

@CyberDem0n
Copy link
Member

Solved in #3138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants