release-engineering / ubi-population-tool

A tool for populating UBI repositories.
GNU General Public License v3.0
3 stars 14 forks source link

Replace usage of _pulp_client with pubtools-pulplib [RHELDST-141] #223

Closed negillett closed 7 months ago

negillett commented 8 months ago

This commit removes the library's native _pulp_client ("legacy") and replaces it with usage of pubtools-pulplib methods for associating and unassociating content.

Additionally, PulpActions were replaced with simple abstractions for grouping content to be associated/unassociated.

Tests were removed/edited as necessary.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.78%. Comparing base (13ab604) to head (8455d0c).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #223 +/- ## ========================================== - Coverage 98.85% 98.78% -0.08% ========================================== Files 8 7 -1 Lines 960 820 -140 ========================================== - Hits 949 810 -139 + Misses 11 10 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

run tests

negillett commented 8 months ago

I'm at a total loss as to why we're seeing error 111 on '/api/v1/manifest'. I've touched nothing in the ubi client. Any ideas, @rbikar?

rbikar commented 7 months ago

run tests

rbikar commented 7 months ago

I'm at a total loss as to why we're seeing error 111 on '/api/v1/manifest'. I've touched nothing in the ubi client. Any ideas, @rbikar?

I think the ubi-manifest service in the test env was in a bad shape, I re-ran tests and they passed OK.

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

@rbikar, @rohanpm, this is ready for review. I'm still unsure why some modulemd profiles no longer contain "[d]" to signal default but I don't believe the change was introduced here (it was only ever present in the integration tests). It could be a difference between rhel7 and rhel8 content--this is a key difference between the two integration tests that asserted the presence of the mark.

rbikar commented 7 months ago

@rbikar, @rohanpm, this is ready for review. I'm still unsure why some modulemd profiles no longer contain "[d]" to signal default but I don't believe the change was introduced here (it was only ever present in the integration tests). It could be a difference between rhel7 and rhel8 content--this is a key difference between the two integration tests that asserted the presence of the mark.

In realistic usage the ubi7 shouldn't have any modules (but I guess in test the situation can be different).

Anyway you need to examine modulemd_defaults units in the test instance of rhsm-pulp before and after the test.

I checked current real ubi8 image and e.g. for perl module I get this: Output of dnf module list

perl                 5.24       common [d Practical Extraction and Report Language                                                                                                                                                             
                                ], minima 
                                l         
perl                 5.26 [d]   common [d Practical Extraction and Report Language                                                                                                                                                             
                                ], minima 
                                l         
perl                 5.30       common [d Practical Extraction and Report Language                                                                                                                                                             
                                ], minima 
                                l         
perl                 5.32       common [d Practical Extraction and Report Language                                                                                                                                                             
                                ], minima 
                                l   

and corresponding modulemd_defaults unit in production rhsm-pulp data: (Maybe the testing code reads wrongly the output as in terminal the formatting doesn't look very nice)

...
            "name": "perl",
            "profiles": {
                "5.24": [
                    "common"
                ],
                "5.26": [
                    "common"
                ],
                "5.30": [
                    "common"
                ],
                "5.32": [
                    "common"
                ]
            },
            "pulp_user_metadata": {},
            "repo_id": "ubi-8-for-x86_64-appstream-rpms__8",
            "stream": "5.26"
        },
...

which say 2 things:

so it should be dependent on the data in pulp only, can you double-check the removal and copy of modulemd_defaults units? There might some subtle bug (or the formatting problem as I mentioned above).

negillett commented 7 months ago

run tests

negillett commented 7 months ago

run tests

negillett commented 7 months ago

"common [d]" is showing up now for that one tests 🤷🏻

negillett commented 7 months ago

run tests

negillett commented 7 months ago

@rohanpm @rbikar ptal