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

Fix linter for Consul #370

Merged
merged 4 commits into from
Jun 10, 2023
Merged

Fix linter for Consul #370

merged 4 commits into from
Jun 10, 2023

Conversation

ThomasSanson
Copy link
Contributor

No description provided.

The ../roles/consul path was previously excluded from ansible-lint checks, but it is no longer necessary as the issue it was addressing has been resolved.
…dead_Servers` to `consul_autopilot_cleanup_dead_servers` for consistency

The variable name `consul_autopilot_cleanup_dead_Servers` has been changed to `consul_autopilot_cleanup_dead_servers` to improve consistency with the naming conventions used in the rest of the codebase.
The jinja2 brackets in the when statement are unnecessary and can be removed. This commit removes them to improve readability and consistency with other tasks in the playbook.
…event silent errors

The `set -o pipefail` option was added to the shell command to prevent silent errors that may occur when the `grep` command fails to find the "encrypt" key in the `config.json` file. This ensures that any errors that occur during the execution of the command are caught and reported.
@ThomasSanson
Copy link
Contributor Author

Hello @vitabaks,

I have a question regarding the Consul role. I was wondering if this role has been tested with Molecule ?

Based on my own investigation, I didn't find indications that this is the case, but I wanted to make sure I'm not missing something. Could you confirm whether this is correct or not ?

Thank you in advance for your time and your help.

@vitabaks
Copy link
Owner

vitabaks commented Jun 9, 2023

@ThomasSanson That's right, I haven't added this scheme to the Molecule tests yet.

I have an idea to add a consul schema test to the daily tests (schedule_pg_xxx.yml).

Where one version of the Linux distribution will be deployed in the scheme with Etcd the second version of Consul.

What do you think?

@ThomasSanson
Copy link
Contributor Author

@vitabaks,

I agree with your suggestion to add a consul schema test to the daily tests. However, I believe we could also benefit from adding Molecule scenarios, just like what has been done for the default and postgrespro scenarios.

Thus, I would lean towards adding a "consul" or Type C scenario. These scenarios would be very helpful in testing and verifying different configurations and environments.

Regarding the Github Action Matrix, I will need to look into it. Without it, maintaining multiple scenarios could become challenging.

We can merge this pull request if you wish. Afterward, we could open a new issue to discuss and work on adding the Type C scenario. This will keep the discussions organized and make progress tracking easier.

@vitabaks vitabaks merged commit 6f4fe12 into vitabaks:master Jun 10, 2023
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