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

Allowing the case, tls is false and key/certs vars are configured. #243

Merged
merged 1 commit into from Aug 24, 2021

Conversation

nhosoi
Copy link
Contributor

@nhosoi nhosoi commented Aug 19, 2021

  • The case used to fail in the checking task with emitting an error.
    The error is transformed to a warning.
  • For the test case, TEST CASE 2 in tests_files_elasticsearch.yml
    is added.

In addition, some minor bugs in tests_files_elasticsearch.yml are
fixed.

ref: bz1994580

@nhosoi nhosoi requested a review from richm August 19, 2021 19:10
Copy link
Contributor

@spetrosi spetrosi left a comment

Choose a reason for hiding this comment

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

lgtm

- myprivkey: "{{ __test_key_target }}"
register: __result
changed_when: false
failed_when: __result.rc != 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you use this __result anywhere else? If not, then the default failed condition here is if the grep in the command returns non-zero, so I think you can get rid of the register and the failed_when here.

- "{{ __test_cert_target }}"
- "{{ __test_key_target }}"

- name: Check key/certs in {{ __test_outputfiles_conf }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be "Check key/certs not in {{ __test_outputfiles_conf }}" ?

- The case used to fail in the checking task with emitting an error.
  The error is transformed to a warning.
- For the test case, TEST CASE 2 in tests_files_elasticsearch.yml
  is added.

In addition, some minor bugs in tests_files_elasticsearch.yml are
fixed.

ref: bz1994580
Copy link
Collaborator

@richm richm left a comment

Choose a reason for hiding this comment

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

lgtm

@nhosoi
Copy link
Contributor Author

nhosoi commented Aug 24, 2021

Thank you, @richm. Merging.

@nhosoi nhosoi merged commit 5742b01 into linux-system-roles:master Aug 24, 2021
@nhosoi nhosoi deleted the tls branch June 30, 2022 01:27
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.

None yet

3 participants