infra.aap_configuration.hub_collection_repository_sync is attempting to sync repos with no remote

[bogdanmuresan]

Assuming we also want to customise the "published" repo, or add an new collection repository with no remote,
when it is added to hub_collection_repositories, since this var is also used by hub_collection_repository_sync, Ah will return "HTTP Error 500: Internal Server Error".

hub_collection_repository_sync role should not attempt to sync repos that don't have a remote

Issue Type

• Bug Report

Ansible, Collection, Controller details

Controller version: 4.6.8

• ansible installation method: one of source, pip, OS package, EE

OS / ENVIRONMENT

Desired Behavior

STEPS TO REPRODUCE

---
hub_collection_repositories:
  - name: rh-certified
    remote: rh-certified
    wait: true
    timeout: 3600
  - name: validated
    remote: validated
    wait: true
    timeout: 3600
  - name: community
    remote: community
    wait: true
    timeout: 3600
  - name: published
    retain_repo_versions: 50
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false
  - name: acme_repo
    retain_repo_versions: 10
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false

  roles:
    - role: infra.aap_configuration.dispatch
      vars:
        aap_configuration_dispatcher_roles: "{{ hub_configuration_dispatcher_roles }}"

hub_configuration_dispatcher_roles:
  - role: hub_collection_remote
    var: hub_collection_remotes
    tags: collectionremote   
  - role: hub_collection_repository
    var: hub_collection_repositories
    tags: collectionsrep
  - role: hub_collection_repository_sync
    var: hub_collection_repositories
    tags: collectionsrepsync

[Tompage1994]

Just thinking about how we could resolve this:

If we add something which only attempts if remote is defined then what if we write a bit of automation which just does the syncing and we only supply the name?

I think we may need to add a variable which is something like sync_if_remote_empty which would force the sync if it is empty, and otherwise would only sync if remote was set.

Either way, I think this would be a breaking change if included.

Thoughts @sean-m-sullivan @djdanielsson

@bogdanmuresan would this solution be acceptable?

[bogdanmuresan]

Hello Tom, thank you for your input.

I guess you could introduce a flag at the repo level - "no_sync", defaulted to false, and add a when statement here: https://github.com/redhat-cop/infra.aap_configuration/blob/devel/roles/hub_collection_repository_sync/tasks/main.yml#L4 (when __hub_collection_repository_sync_item.no_sync: false). I can then set the no_sync to true on the repos without remote.

You could also maybe introduce an new variable for the sync (hub_collection_repositories_sync), that defaults to hub_collection_repositories. I can then split the list into 2:

hub_collection_repositories_with_remotes:
  - name: rh-certified
    remote: rh-certified
    wait: true
    timeout: 3600
  - name: validated
    remote: validated
    wait: true
    timeout: 3600
  - name: community
    remote: community
    wait: true
    timeout: 3600

hub_collection_repositories_without_remotes:
  - name: published
    retain_repo_versions: 50
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false
  - name: acme_repo
    retain_repo_versions: 10
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false

hub_collection_repositories: "{{ hub_collection_repositories_with_remotes + hub_collection_repositories_without_remotes }}"
hub_collection_repositories_sync: "{{ hub_collection_repositories_with_remotes }}"

These are both backwards compatible, but the proper fix would be to only sync if there is a remote, in my opinion.

I don't really have a preference for the solution.

[djdanielsson]

I'm still on leave so I haven't looked super close at this but I do want to jump in and say I would like to try to have it be a non breaking change. Probably next week I will start to be online and have more of an opinion if it hasn't been solved by then

[Tompage1994]

@bogdanmuresan I think I prefer your first idea more here. I would probably reverse it and have an option sync which is defaulted to true, that way we can set sync to false if needed. This way it wouldn't be a breaking change and we have the ability to resolve without being too complicated (which I think the second option might be).

Your original example would then want to be altered like this:

hub_collection_repositories:
  - name: rh-certified
    remote: rh-certified
    wait: true
    timeout: 3600
  - name: validated
    remote: validated
    wait: true
    timeout: 3600
  - name: community
    remote: community
    wait: true
    timeout: 3600
  - name: published
    retain_repo_versions: 50
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false
    sync: false
  - name: acme_repo
    retain_repo_versions: 10
    distribution:
      state: present
    pulp_labels:
      pipeline: "approved" 
    wait: false
    sync: false

[djdanielsson]

so it sounds like we have a plan on how to solve this and now just looking for a PR from someone to fix it