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

Feature role synchronization. #886

Closed
wants to merge 7 commits into from
Closed

Feature role synchronization. #886

wants to merge 7 commits into from

Conversation

przemkalit
Copy link
Contributor

What does this PR do?

In some ways implement redhat-cop/aap_configuration_extended#9.

How should this be tested?

  1. Create the export with filetree_create
  2. Use filetree_read and dispatch with a new variable
- name: Sync roles
  hosts: all 
  tasks:
    - name: Include vars from control_vars directory
      ansible.builtin.include_vars:
        dir: "{{ dir_orgs_vars }}"
        extensions:
          - "yml"
          - "yaml"

  roles:
    - role: infra.controller_configuration.filetree_read
    - role: infra.controller_configuration.dispatch
      vars:
        sync_roles: true

Is there a relevant Issue open for this?

resolves redhat-cop/aap_configuration_extended#9

Other Relevant info, PRs, etc

N/A

@przemkalit przemkalit requested a review from a team as a code owner August 1, 2024 13:55
@djdanielsson
Copy link
Collaborator

did you mix up this PR and one of your other ones? cuz this talks about filetree but no changes here are within the filetree role

@przemkalit
Copy link
Contributor Author

I don't think that I made mistake with PR, I thought a lot about this topic and this solution allows to synchronize roles between files exported using filetree_create and the target controller. It is not perfect because if someone delete all permissions from the user/team it won't be synchronized.

@djdanielsson
Copy link
Collaborator

@Tompage1994 @sean-m-sullivan thoughts on this one? or do we need filetree team to review?

@Tompage1994
Copy link
Collaborator

Sorry for taking a while on this. I've been back to this 2 or 3 times and I can't really get my head around what you're trying to achieve that the object_diff and filetree stuff doesn't do.

My gut feel is that this isn't the right place for all this logic as it goes against what we have tried to do for all the other 'endpoint' roles where we are simply looping over the module (as well as adding in extra bits to speed up execution etc.) This does stray away from that and that does make me nervous.

Let me know if you have compelling reasons why this needs to be part of this role specifically and not part of one of the filetree roles.

@przemkalit
Copy link
Contributor Author

Okay, I know the object_diff but there is an issue when there are lots of the roles (we have around 20k) and while I was using the object_diff it was killing the job due to memory leak. If you can suggest how to solve it it would be great.

@Tompage1994
Copy link
Collaborator

Okay, I know the object_diff but there is an issue when there are lots of the roles (we have around 20k) and while I was using the object_diff it was killing the job due to memory leak. If you can suggest how to solve it it would be great.

In honesty I've never tried doing object_diff at scale. If you find this works better then maybe adding this logic into the object_diff for roles as a replacement?

@przemkalit
Copy link
Contributor Author

Ok I will try to add this changes to object_diff.

@przemkalit
Copy link
Contributor Author

Hi, after analysing our approach with the object_diff we've decided not to push it to the collection, so I will close this PR.

@przemkalit przemkalit closed this Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tompage1994 Tompage1994 Awaiting requested review from Tompage1994

@sean-m-sullivan sean-m-sullivan Awaiting requested review from sean-m-sullivan

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

Successfully merging this pull request may close these issues.

Roles synchronisation
3 participants
@przemkalit @djdanielsson @Tompage1994