Fixes #13987 - Capsule Sync Optimization

[johnpmitsch]

Capsule sync should check if importers or distributors in repos need updating before updating them ( as opposed to blindly updating all of them as it does now). This will speed up capsule syncs and use less dynflow actions.

[ehelms]

Pushing these three lines into a function that returns repositories needing update could help readability and hide away those details to keep the plan function succinct.

[ehelms]

Please update to point at 13987 since you marked 13989 as a duplicate

[ehelms]

This is needed due to the format that generated_distributors is in?

[johnpmitsch]

yes that is correct, so I can compare hash object to hash object

[johnpmitsch]

If anyone has any idea where all these methods could potentially live, I am all 👂 👂

[ehelms]

Spit balling on this one as an example of how you might think about it, if it doesn't make sense to attach to the repo object itself, at least take a look at the logic updates to see if they match expectation:

       def repos_needing_distributor_updates(capsule, environment, content_view)
          repos = capsule.repos_available_to_capsule(environment, content_view)
          repos.select do |repo|
            repo_details = capsule.pulp_repo_facts(repo.pulp_id)
            next unless repo_details          
            capsule_distributors = repo_details["distributors"]
            repo.distributors_changed?(repo_details["distributors"])
          end
       end

in glue/pulp/repo.rb:

      def distributors_changed?(distributors)
           generated_distributors = repo.generate_distributors.map { |dist| object_to_hash(dist) }
           distributors.each do |dist|
             dist.merge!(dist["config"])
             dist.delete("config")
           end

           generated_distributors.any? do |gen_dist|
             capsule_distributors.any? do |cap_dist|
               (gen_dist.to_a - cap_dist.to_a).empty?
             end
           end
         end  
    end

[johnpmitsch]

@ehelms refactored according to your suggestions

[jlsherrill]

should this be distributors_changed? to be consistent with importer_changed?

[johnpmitsch]

I'll double check what it was, but there was an issue with naming it that, not sure if there is some sort of magic method thing going on. Any objection to having them both end with _change? if I run into that issue changing it back to _changed? ?

[jlsherrill]

ahhhh probably due to distributors being a lazy accessor. What about:

distributors_match?()

importer_matches?()

[jlsherrill]

that's the best i could come up with :)

[johnpmitsch]

I think that is fairly clear, I'll try that

[johnpmitsch]

@jlsherrill updated here (importer matches)

[jlsherrill]

Could you just use .as_json() here instead of the object_to_hash method?

2.2.4 :013 > Katello::Repository.first.generate_distributors[0].as_json
 => {"relative_url"=>"Default_Organization/Library/custom/testproduct/tester", "http"=>true, "https"=>true, "auto_publish"=>true, "id"=>"Default_Organization-testproduct-tester", "protected"=>true, "checksum_type"=>"sha256"} 

[jlsherrill]

Other than the as_json comment, everything looks good and seems to be working well!

[johnpmitsch]

@ehelms @jlsherrill updated to use as_json and tested a sync, which ran fine. (wouldn't hurt to double check though)

[jlsherrill]

Looks good to me, just one test failure and this is good to go.

[jlsherrill]

ACK