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

Fixes #33748 - correct input reference on duplicate upload #9735

Merged
merged 2 commits into from Nov 5, 2021

Conversation

jlsherrill
Copy link
Member

@jlsherrill jlsherrill commented Oct 20, 2021

What are the changes introduced in this pull request?

Fixing a reference to a variable that doesn't exist, it should be referencing the input

What are the testing steps for this pull request?

From a user perspective, its unclear. It was reported by qe

@theforeman-bot
Copy link

Issues: #33748

Copy link
Member

@jjeffers jjeffers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@jlsherrill
Copy link
Member Author

jlsherrill commented Nov 2, 2021

I've pushed some more updates to fully resolve the issue, and as part of it have tried to simplify the upload class considerably.

Now instead of tons of checking in the orchestration action, it tries to look for duplicates in each action that is attempting to create the duplicates artifact or content unit. Then the action can either return tasks (that will eventually contain an href), or the href itself.

To reproduce the original issue:

  1. create 2 yum repos on a satellite
  2. save the script below and change the first two lines of the script to include the correct filename of the rpm/srpm and the repo id of the repository to upload it to
  3. run this script
  4. change the script to point to the 2nd repository id
  5. run the script again

You might want to also test: #9547 (which worked fine for me with these changes)

ID=2  #repo ID
RPM=createrepo_c-0.17.1-1.el7.src.rpm


SIZE=`stat -c '%s' $RPM`
HOSTNAME=`hostname`
CHECKSUM=`sha256sum $RPM | awk '{print $1}'`
TYPE='srpm'

#  UPLOAD_RESPONSE=`curl -u admin:changeme https://$HOSTNAME/katello/api/v2/repositories/$ID/content_uploads -X POST  -d "{\"checksum\":\"$CHECKSUM\", \"size\": $SIZE}"  -H "Content-Type: application/json"`

UPLOAD_RESPONSE=`curl -u admin:changeme https://$HOSTNAME/katello/api/v2/repositories/$ID/content_uploads -X POST  -d "{\"size\": $SIZE}"  -H "Content-Type: application/json"`

echo $UPLOAD_RESPONSE

UPLOAD_ID=`echo $UPLOAD_RESPONSE | jq .upload_id | awk -F '"' '{print $2}'`

echo $UPLOAD_ID


curl -X PUT -u admin:changeme https://$HOSTNAME/katello/api/v2/repositories/$ID/content_uploads/$UPLOAD_ID -H "Content-Type: multipart/form-data" -F "content=<$RPM"  -F offset=0 -F size=$SIZE


JSON="{\"content_type\":\"$TYPE\", \"uploads\": [{\"id\": $UPLOAD_ID, \"size\":$SIZE, \"checksum\":\"$CHECKSUM\"}   ] }"

echo $JSON

curl -X PUT -u admin:changeme https://$HOSTNAME/katello/api/v2/repositories/$ID/import_uploads/ -H "Content-Type: application/json"  -d "{\"content_type\":\"$TYPE\", \"uploads\": [{\"id\": \"$UPLOAD_ID\", \"size\":$SIZE, \"checksum\":\"$CHECKSUM\", \"name\":\"$RPM\"}   ] }" 

@jjeffers
Copy link
Member

jjeffers commented Nov 3, 2021

Tested this by running the mentioned script, once pre-patch and one post.

Running the script with repo id of first respository:

[vagrant@centos7-katello-devel-stable foreman]$ ./test_9357.sh 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    68    0    52  100    16     91     28 --:--:-- --:--:-- --:--:--    91
{"upload_id":"63bfaee6-dee8-4362-a884-b2d03ce3e348"}
63bfaee6-dee8-4362-a884-b2d03ce3e348
{"content_type":"srpm", "uploads": [{"id": 63bfaee6-dee8-4362-a884-b2d03ce3e348, "size":600482, "checksum":"2bdf1cb71d9d3a785af24f86bdb35bce9fae29feb86cf2c48ad5a2b372441e00"} ] }
  {"id":"4e12c66e-6ebe-4d1d-9c63-16d0408c97e7","label":"Actions::Katello::Repository::ImportUpload","pending":false,"action":"Upload into repository 'zoo1'; product 'test-9735'; organization 'Default Organization'","username":"admin","started_at":"2021-11-02 20:08:12 UTC","ended_at":"2021-11-02 20:08:18 UTC","state":"stopped","result":"success","progress":1.0,"input":{"repository":{"id":2,"name":"zoo1","label":"zoo1"},"product":{"id":2,"name":"test-9735","label":"test-9735","cp_id":"926458147783"},"provider":{"id":1,"name":"Anonymous"},"organization":{"id":1,"name":"Default Organization","label":"Default_Organization"},"services_checked":["pulp3"],"repository_id":2,"sync_capsule":true,"upload_results":[{"pulp_tasks":[{"pulp_href":"/pulp/api/v3/tasks/25d468b5-9514-4331-9b5f-353ac8ea2fa5/","pulp_created":"2021-11-02T20:08:13.063+00:00","state":"completed","name":"pulpcore.app.tasks.upload.commit","logging_cid":"f5a5aadd-16f8-452d-8f6e-88a20d78d3f9","started_at":"2021-11-02T20:08:13.131+00:00","finished_at":"2021-11-02T20:08:13.209+00:00","worker":"/pulp/api/v3/workers/ebb38099-7dc5-4f1c-b1fe-9b7efd457843/","child_tasks":[],"progress_reports":[],"created_resources":["/pulp/api/v3/artifacts/441c7014-5d64-40ab-be96-08c1c0c43c40/"],"reserved_resources_record":["/pulp/api/v3/uploads/63bfaee6-dee8-4362-a884-b2d03ce3e348/"]}],"content_unit_href":"/pulp/api/v3/content/rpm/packages/8cd31093-23de-42d8-a9da-def7a2414acd/"}],"locale":"en","current_request_id":"f5a5aadd-16f8-452d-8f6e-88a20d78d3f9","current_timezone":"UTC","current_organization_id":null,"current_location_id":null,"current_user_id":4},"output":{"upload_results":[{"type":"file"}]},"humanized":{"action":"Upload into","input":[["repository",{"text":"repository 'zoo1'","link":null}],["product",{"text":"product 'test-9735'","link":"/products/2/"}],["organization",{"text":"organization 'Default Organization'","link":"/organizations/1/edit"}]],"output":"","errors":[]},"cli_example":null,"start_at":"2021-11-02 20:08:12 UTC","available_actions":{"cancellable":false,"resumable":false}}

Changed the repo id to second repository.

[vagrant@centos7-katello-devel-stable foreman]$ ./test_9357.sh 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    68    0    52  100    16     97     29 --:--:-- --:--:-- --:--:--    97
{"upload_id":"85fc0cc1-4039-4605-bbbc-3421c6468957"}
85fc0cc1-4039-4605-bbbc-3421c6468957
{"content_type":"srpm", "uploads": [{"id": 85fc0cc1-4039-4605-bbbc-3421c6468957, "size":600482, "checksum":"2bdf1cb71d9d3a785af24f86bdb35bce9fae29feb86cf2c48ad5a2b372441e00"} ] }
{"displayMessage":"Task 1e06b23e-d9e0-46c3-8a13-9535bf19c877: NameError: undefined local variable or method `upload_href' for #\u003cActions::Pulp3::Repository::CommitUpload:0x00007fd899b468f0\u003e\nDid you mean?  uploads_api","errors":["Task 1e06b23e-d9e0-46c3-8a13-9535bf19c877: NameError: undefined local variable or method `upload_href' for #\u003cActions::Pulp3::Repository::CommitUpload:0x00007fd899b468f0\u003e\nDid you mean?  uploads_api"]}

Finally applied the PR and re-ran the script against the 2nd repository:

[vagrant@centos7-katello-devel-stable foreman]$ ./test_9357.sh 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    68    0    52  100    16     21      6  0:00:02  0:00:02 --:--:--    21
{"upload_id":"79675881-796e-4344-b8ca-122b1e7f365d"}
79675881-796e-4344-b8ca-122b1e7f365d
{"content_type":"srpm", "uploads": [{"id": 79675881-796e-4344-b8ca-122b1e7f365d, "size":600482, "checksum":"2bdf1cb71d9d3a785af24f86bdb35bce9fae29feb86cf2c48ad5a2b372441e00"} ] }
  {"id":"d267cd28-de80-422a-9dce-bb8735baad7b","label":"Actions::Katello::Repository::ImportUpload","pending":false,"action":"Upload into repository 'zoo2'; product 'test-9735'; organization 'Default Organization'","username":"admin","started_at":"2021-11-03 11:13:23 UTC","ended_at":"2021-11-03 11:13:27 UTC","state":"stopped","result":"success","progress":1.0,"input":{"repository":{"id":3,"name":"zoo2","label":"zoo2"},"product":{"id":2,"name":"test-9735","label":"test-9735","cp_id":"926458147783"},"provider":{"id":1,"name":"Anonymous"},"organization":{"id":1,"name":"Default Organization","label":"Default_Organization"},"services_checked":["pulp3"],"repository_id":3,"sync_capsule":true,"upload_results":[{"pulp_tasks":[],"content_unit_href":"/pulp/api/v3/content/rpm/packages/8cd31093-23de-42d8-a9da-def7a2414acd/"}],"locale":"en","current_request_id":"6069bb04-7e56-4442-823a-5c5794403f08","current_timezone":"UTC","current_organization_id":null,"current_location_id":null,"current_user_id":4},"output":{"upload_results":[]},"humanized":{"action":"Upload into","input":[["repository",{"text":"repository 'zoo2'","link":null}],["product",{"text":"product 'test-9735'","link":"/products/2/"}],["organization",{"text":"organization 'Default Organization'","link":"/organizations/1/edit"}]],"output":"","errors":[]},"cli_example":null,"start_at":"2021-11-03 11:13:23 UTC","available_actions":{"cancellable":false,"resumable":false}}

@jlsherrill
Copy link
Member Author

i think this should fix the tests

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested several workflows..Works fine for single/ multiple/ duplicate/ duplicate large , remove/add etc..

ACK 👍🏽

@sjha4
Copy link
Member

sjha4 commented Nov 4, 2021

[test katello]

@jlsherrill jlsherrill merged commit 532ce8a into Katello:master Nov 5, 2021
@jlsherrill jlsherrill deleted the 33748 branch November 5, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants