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

Failing attempt to configure an overriding default controller definitions feature in filetree_read #558

Closed
nodje opened this issue Mar 29, 2023 · 12 comments
Labels
enhancement New feature or request filetree/object_diff inactive No movement has happened in 30 days question Further information is requested

Comments

@nodje
Copy link

nodje commented Mar 29, 2023

If I'm not mistaken the directory convention to organize controllers' definition is as follow:

orgs_var/{{ orgs }}/env/common
orgs_var/{{ orgs }}/env/dev
orgs_var/{{ orgs }}/env/prod

and with this comes the default filetree_read configuration which reads some controller def in common some others in named env (dev, prod, etc.)

I needed something more flexible, a system where all controllers' definition could be read from common and overriden in any named env, should the need arise.

With this in mind and my shallow understanding of this collection, I have implemented the playbook to read and dispath like this:

roles:
    # run once for common values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_hosts.d/"
    # run a second time to override values by env
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_hosts.d/"
#    - redhat_cop.controller_configuration.object_diff
    - redhat_cop.controller_configuration.dispatch

In essence, I call filetree_read twice:

  • a first time with controller not read from common by default, but configured to be read from common to provide a common base
  • a second time with the default config (where specific controllers are reads from a named env), which I have written down to make the playbook more explicit
    Then dispatch is called (with objec_diff usage as a 3rd step still in the working)

It all seemed to work flawlessly for a while but i have started to notice something that makes it unsuitable:
the two sets of config filetree_read processes are not applied in the read order by dipatch. Indeed, they are applied randomly.

I could call dispatch twice to make sure the read order is maintained but I thought I'd ask here if there wasn't a better way to do this.

I think I've attempted to ask if this could be a feature in the past.
To me, it's a must have in order to avoid a dreaded code duplication.
And particularly here since the Yaml code configuration quickly becomes very verbose.

I know it's not a bug report per se, but other sections still seem seldomly visited.
Let me know if I should repost.

@nodje nodje added bug Something isn't working new New issue, this should be removed once reviewed labels Mar 29, 2023
@nodje
Copy link
Author

nodje commented Apr 3, 2023

It is apparently not possible to execute dispatch twice.

roles:
    # run once for common values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_hosts.d/"
    - redhat_cop.controller_configuration.dispatch
    # run a second time to override values by env
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_hosts.d/"
    - redhat_cop.controller_configuration.dispatch

Here the first dispatch is run but the second time the role is not executed for some reason.

@ivarmu
Copy link
Contributor

ivarmu commented Apr 4, 2023

Hi @nodje. If I understand you well, I think what you are trying to do is just the following:

roles:
  # run once for common and env values
  - role: redhat_cop.controller_configuration.filetree_read
    vars:
      filetree_controller_settings:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_settings.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_settings.d/"
      filetree_controller_user_accounts:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_users.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_users.d/"
      filetree_controller_credentials:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_credentials.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_credentials.d/"
      filetree_controller_execution_environments:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_execution_environments.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_execution_environments.d/"
      filetree_controller_inventory_sources:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_inventory_sources.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_inventory_sources.d/"
      filetree_controller_instance_groups:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_instance_groups.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_instance_groups.d/"
      filetree_controller_hosts:
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_hosts.d/"
        - "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_hosts.d/"

  - redhat_cop.controller_configuration.dispatch

@ivarmu ivarmu added question Further information is requested and removed bug Something isn't working labels Apr 4, 2023
@nodje
Copy link
Author

nodje commented Apr 4, 2023

Hi @ivarmu ,

sounds great thank you.
Are you yourself using this? Cause I'd like to confirm that the filetree_read reading order will be the kept the same when dispatch implement the settings.
Your solution is neater than my original implementation, but it should boil down to the same in the end.
Hence, logically, I should have the same issue: no control on the order of settings dispatch

@ivarmu
Copy link
Contributor

ivarmu commented Apr 4, 2023

We are using this approach to have common and environment-related users. They are, in fact, all different users, so there's no duplicate in our usecase. Anyway, the file loading is done here, so the module ansible.builtin.find is determining the order the files will be read, any kind of ordering is not guarranteed.

Maybe you can try the following instead:

roles:
    # read the common values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_hosts.d/"
    # read the env values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_hosts.d/"
    # run all the gathered objects
    - redhat_cop.controller_configuration.dispatch

@nodje
Copy link
Author

nodje commented Apr 4, 2023

What you just posted is my original configuration.
And it doesn't work since the ordering is not guaranteed.

Take settings for instance.
The idea is hat I set a common default value for a setting, which can then be overriden by a specific env value.

But there a notion of precedence (the specific env config must always take precedence over the default) that cannot be implemented.

@ivarmu
Copy link
Contributor

ivarmu commented Apr 4, 2023

You are right, is like your first example, sorry for my duplication. I think there are two things here to have in consideration:

  1. Reading the files:
    1.1. First filetree_read should read all the objects from common into the corresponding variables (let's say controller_user_accounts)
    1.2. Second filetree_read should read all the objects from {env} into the same corresponding variables
    1.3. In that point, the contents of the controller_user_accounts variable should look as desired, with the duplicated values been overwritten by the second read, so values from {env} should persist if duplicated, and values from common if not

  2. Applying the objects to the Tower/Controller: that should work as expected, if point 1 is as exposed (that's not)

So, the problem here is that filetree_read is setting a new value for each variable each time it reads from files, so the second time, the controller_user_accounts is completely overwritten by the contents of {env}. So... one possible workaround could be something like this (only schematic, as it needs major changes, but to give an idea):

roles:
    # read the common values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/common/controller_hosts.d/"
    # Save first values
    - set_fact:
        controller_user_accounts_1: "{{ controller_user_accounts }}"
        controller_..._1: "{{ controller_... }}"
    # read the env values
    - role: redhat_cop.controller_configuration.filetree_read
      vars:
        filetree_controller_settings: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_settings.d/"
        filetree_controller_user_accounts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_users.d/"
        filetree_controller_credentials: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_credentials.d/"
        filetree_controller_execution_environments: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_execution_environments.d/"
        filetree_controller_inventory_sources: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_inventory_sources.d/"
        filetree_controller_instance_groups: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_instance_groups.d/"
        filetree_controller_hosts: "{{ dir_orgs_vars }}/{{ orgs }}/env/{{ env }}/controller_hosts.d/"
    # Join both variables
    - set_fact: .....
    # run all the gathered objects
    - redhat_cop.controller_configuration.dispatch

I'm not sure if it would be interesting for that role to treat an already populated input variable, so the new reads should be added instead of completely overwritten...

What are your thoughts @sean-m-sullivan @djdanielsson @Tompage1994 @silvinux ?

@ivarmu ivarmu added the enhancement New feature or request label Apr 4, 2023
@djdanielsson djdanielsson removed the new New issue, this should be removed once reviewed label Apr 6, 2023
@github-actions github-actions bot added the inactive No movement has happened in 30 days label May 7, 2023
@nodje
Copy link
Author

nodje commented May 12, 2023

Hi,

can we reactivate this enhancement request?

@github-actions github-actions bot removed the inactive No movement has happened in 30 days label May 12, 2023
@github-actions github-actions bot added the inactive No movement has happened in 30 days label Jun 12, 2023
@github-actions github-actions bot removed the inactive No movement has happened in 30 days label Jun 21, 2023
@sean-m-sullivan
Copy link
Collaborator

@ivarmu ? Assigning to ypu

@adonisgarciac
Copy link
Contributor

I think to try to join both list could be very complicated. Let's say controller_user_accounts again: It's possible to have the same user defined in both paths (common and env). If I'm not wrong, you can't join both list directly. It has to go item by item comparing them and if it has the same name, it will keep the env one. So, this could complicate the code and increase exponentially the execution time.

IMHO, we should allow, as much, for example, persist values in common but if the object type is defined in any way in the env path, the common object definition should be totally overwritten.

Anyway, common paths were intended to avoid duplicate object definition which are common between environments. However, one of gitops principles to follow is implement the environment-per-folder approach because is a much better way to organize your GitOps applications. Not only it is much simpler to implement and maintain, but it is also the optimal method for promoting releases between different GitOps environments. So, best practices tell us that is a better idea to duplicate object definition even they are the same between environments.

@nodje
Copy link
Author

nodje commented Jul 21, 2023

Hi @adonisgarciac ,

thank you for taking the time to investigate this.
I'm not in a position to judge on the complexity or feasibility of this feature with the actual code base.
It might well be that it is too complex to implement.

However, I think I disagree with your conclusion :)
My experience tells me that duplicating code is never a good thing. In the end, someone is always going to modify one occurrence and forgetting others. If not yourself then the one taking over your code.
And i believe that is true with yaml config for filetree_read as well.

One env-per-folder is good, but each env could contain only the part from common that it needs to override.

In the end we had to modify our code base to have exactly what you suggest: common env ONLY for what is exactly common to all env, and one conf per env if one single env needs a different config.
That works, but it duplicates code, and doesn't scale well with the number of env.

@adonisgarciac
Copy link
Contributor

You're able to have common and env folder working together. The conflicting point happens when you want to override common objects with env folder.

I mean, you're able to have JT in common and JT in env folder. However, the code are not ready to manage the definition of same JT in common and in env because, as you said before, the order of each object creation is not being managed. To achieve that behavior, in my opinion, the code will become unmanageable and the compute time of each execution will increase exponentially because it has to compare each object from common with each object from env.

One folder-per-env is a GitOps principle, for that reason I suggested it.

You can do it by yourself in the playbook instead of in the roles with something similar as @ivarmu proposed you. But, IMHO, if there are many objects, it will take a long time to process everything and compare list1 (from common) with list2 (from env) to keep the env ones.

@github-actions github-actions bot added the inactive No movement has happened in 30 days label Aug 24, 2023
@sean-m-sullivan
Copy link
Collaborator

Closing due to inactive, there are always multiple ways of doing things. If you find that changes are needed to the roles to facilitate something please bring it up. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request filetree/object_diff inactive No movement has happened in 30 days question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants