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.

Sign up for GitHub

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

Jump to bottom

Added package tag #1459

Open
wants to merge 1 commit into
base: 7.1.x
Choose a base branch
from
Open

Added package tag #1459

wants to merge 1 commit into from

Conversation

rrbadiani
Copy link
Member

Description

Added package tag

@rrbadiani rrbadiani requested a review from a team September 1, 2023 12:29
@cla-assistant
Copy link

cla-assistant bot commented Sep 1, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

mansisinha
mansisinha reviewed Sep 5, 2023
View reviewed changes
playbooks/tasks/certificate_authority.yml
@@ -30,6 +30,9 @@
ansible_become: "{{ ansible_become_localhost }}"

- name: Apt update
tags:
- certificate_authority
Copy link
Member

Choose a reason for hiding this comment

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

this task is not related to certificate. Package tag looks okay

Copy link
Member Author

Choose a reason for hiding this comment

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

the tasks that have certificate_authority tag in them are mostly using openssl. So if those tasks need to be run separately using --tags certificate_authority then running this task previous to them would ensure when they are run they dont give the error command openssl not found.

mansisinha
mansisinha reviewed Sep 5, 2023
View reviewed changes
playbooks/tasks/certificate_authority.yml
@@ -69,7 +72,9 @@
- '!all'

- name: Install OpenSSL and Rsync - RedHat
tags: certificate_authority
tags:
- certificate_authority
Copy link
Member

Choose a reason for hiding this comment

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

same as above.

mansisinha
mansisinha reviewed Sep 5, 2023
View reviewed changes
Copy link
Member

@mansisinha mansisinha left a comment

Choose a reason for hiding this comment

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

why is this PR raised against 7.1? what about the previous versions? Have we checked thoroughly that tags: package is not required in any other task?

@rrbadiani
Copy link
Member Author

why is this PR raised against 7.1? what about the previous versions? Have we checked thoroughly that tags: package is not required in any other task?

The feature to support non root user was introduced in 7.1.x. Hence to fix issue with non root deployment this Pr is raised against 7.1.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mansisinha mansisinha mansisinha left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rrbadiani @mansisinha