Avoid registration and update request handling failure due to profiles handling errors by rtamalin · Pull Request #1522 · SUSE/rmt

rtamalin · 2026-07-28T22:23:52Z

Description

Rework the announce_system request handling workflow to create the system record first, without including any profiles, and then, if profiles are provided, update the system record with any provided complete profiles. As before, if any issues are identified while processing the profiles the response header will be set. Additionally any failures that occur while updating the system record will be logged, but request processing will continue as normal.

As part of update request handling log any errors that occur during the system profiles updates, set the response header indicating that clients should send complete profiles next time, and continue with normal processing.

Refactored the update request handler to split out the processing of system profiles to a separate routine to address rubocop complexity concerns.

Updated spec tests for both announce_system and update requests to exercise the modified processing scenarios.

Created placeholder v3.2 entry in the rmt-server.changes to hold a changelog entry for these changes.

AI-Assistance-By: Qwen 3.6 35B MoE (A3B)

Fixes: SCC-822

How to test

To assist in testing this scenario I've SUSE/connect-ng's public-api-demo to add support for specifying profiles via a PROFILES_DIR env var. See public-api-demo/README.md for more details.

However, there seems to be a limitation in the public-api-demo command, or possibly the SUSE/connect-ng client request setup where profiles larger than approx 1MiB in size cause an error when creating the client HTTP request. Therefore test this I had to temporarily reduce the max size of the data field in my local RMT's profiles table to a smaller value to allow me to submit profiles that were too large.

Change Type

Please select the correct option.

Bug Fix (a non-breaking change which fixes an issue)
New Feature (a non-breaking change which adds new functionality)
Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

I have reviewed my own code and believe that it's ready for an external review.
I have provided comments for any hard-to-understand code.
I have documented the MANUAL.md file with any changes to the user experience.
If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Review

Please check out our review guidelines
and get in touch with the author to get a shared understanding of the change.

rtamalin · 2026-07-28T22:25:12Z

Created as a draft for now while waiting to see if feature tests pass

rtamalin · 2026-07-29T12:34:41Z

Feature tests passed so draft has been removed for now...

rtamalin · 2026-07-31T13:32:27Z

Have been able to test using enhanced public-api-demo command as proposed in SUSE/connect-ng#418 but also needed to temporarily reduce the size of the profile table's data field due to sizing issues encountered when creating client requests with large profile blobs in public-api-demo.

jeremy-moffitt · 2026-07-31T13:35:22Z

but also needed to temporarily reduce the size of the profile table's data field due to sizing issues encountered when creating client requests with large profile blobs in public-api-demo.

reduce from what to what? what makes it temporary?

rtamalin · 2026-07-31T13:54:32Z

reduce from what to what? what makes it temporary?

When the PCT team identified the original bsc#1268305 issue @jesusbv proposed #1494 (corresponding #1506 for rmt_3) to increase the size of the data column to 16MiB from 64KiB.

It is temporary because it was only to be used for testing purposes; I don't want to reduce the profiles table data column size permanently as part of implementing this fix.

jeremy-moffitt · 2026-07-31T14:05:24Z

reduce from what to what? what makes it temporary?

When the PCT team identified the original bsc#1268305 issue @jesusbv proposed #1494 (corresponding #1506 for rmt_3) to increase the size of the data column to 16MiB from 64KiB.

It is temporary because it was only to be used for testing purposes; I don't want to reduce the profiles table data column size permanently as part of implementing this fix.

@gbuenodevsuse do we have good data on the package profile size? might be relevant to this

rtamalin · 2026-07-31T14:11:39Z

@gbuenodevsuse do we have good data on the package profile size? might be relevant to this
What may be more relevant is that I hit issues with the public-api-demo testing when a profile was approx 1.1MiB in size; that may indicate issues with large profile/payload sizes in SUSE/connect-ng in general, i.e. impacting suseconnect...

rtamalin · 2026-07-31T14:16:32Z

@gbuenodevsuse do we have good data on the package profile size? might be relevant to this
What may be more relevant is that I hit issues with the public-api-demo testing when a profile was approx 1.1MiB in size; that may indicate issues with large profile/payload sizes in SUSE/connect-ng in general, i.e. impacting suseconnect...

That may possibly be an nginx config issue in the RMT deployment...

digitaltom · 2026-07-31T16:02:05Z

    process_system_profiles(create_params)

-    @system = System.create!(**create_params)
+    # Retry system creation without profiles if initial attempt fails


In pubcloud RMTs, the engine overwrites the announce_system methods with it's own version: https://github.com/SUSE/rmt/blob/master/engines/scc_proxy/lib/scc_proxy/engine.rb#L291
So the retry here would not cover it. Maybe extract the retry'ed system create to a shared method?

rtamalin · 2026-08-06T15:14:37Z

Switched this back to draft as I need to work on the issue with the PCT hooking that @digitaltom identified.

jeremy-moffitt

looks ok to me, but someone with more Ruby familiarity should approve before merging

mssola

LGTM in general, just a question on the workflow on system creation.

mssola · 2026-08-12T10:21:58Z

Small comment out of curiosity. I see that you are using tags on your git commit messages like AI-Assistance-By, which is cool. But you are not signing off your commits, which I believe is more important to denote that:

The usual meaning for signing off: You are fully responsible for each commit.
In conjunction with the AI tag: you have thoroughly reviewed the output of the LLM.

I'm not telling you to suddenly sign off your commits or anything, just curious by the whole thing 😉

rtamalin · 2026-08-12T14:00:52Z

Small comment out of curiosity. I see that you are using tags on your git commit messages like AI-Assistance-By, which is cool. But you are not signing off your commits, which I believe is more important to denote that:
1. The usual meaning for signing off: You are fully responsible for each commit.

2. In conjunction with the AI tag: you have thoroughly reviewed the output of the LLM.
I'm not telling you to suddenly sign off your commits or anything, just curious by the whole thing 😉

So in this regard I was following the recommendation to indicate if AI assistance was used in generating the code in the PR. I avoid letting the AI assistant commit or submit anything though, and review and fix things up before I commit and submit... But yeah I can see the benefit of including a signed off by entry to clearly indicate that I have reviewed the code. So a good practice to assume going forward.

rtamalin · 2026-08-12T14:05:50Z

Switching back to draft as my discussion with @mssola identified a viable, likely simpler, alternative approach for the announce_system handling.

As part of annonce_system request handling add support for retrying system record creation without including any complete profiles if an error occurs and profiles were present. Avoid infinite retry loops by skipping retry if profiles weren't provided. Set the reponse header to tell the client to send full profiles next time. As part of update request handling log any errors that occur during the system profiles updates, set the response header indicatingh that clients should send complete profiles next time, and continue with normal processing. Refactored the update request handler to split out the processing of system profiles to a separate routine to address rubocop complexity concerns. Updated spec tests for both announce_system and update requests to exercise the modified processing scenarios. Created placeholder v3.2 entry in the rmt-server.changes to hold a changelog entry for these changes. AI-Assistance-By: Qwen 3.6 35B MoE (A3B) Fixes: SCC-822

@digitaltom

This allows the PubCloud engine overrides for the announce_system request handling to leverage it to do the retry without profiles if profiles were provided in the original request. This addresses @digitaltom's review feedback.

Re-work the announce_system request handling for both general and PubCloud deployment scenarios to avoid the need to retry the system entry creation. Instead create!() the system entry without including any detected complete profiles, and then use update!() to update the system entry with any detected complete profiles. If any incomplete or invalid profiles are detected, or if update!() fails to update the profiles in the DB, the response header will be set to indicate that a client needs to send complete profiles next time. Update recently added test descriptions to reflect these changes.

rtamalin · 2026-08-14T20:32:04Z

Removed draft status as generally happy with the updated solution, working on testing the PubCloud aspect of this.

rtamalin requested review from brett060102, chajain, digitaltom, felixsch, gbuenodevsuse, jeremy-moffitt, jesusbv, mssola and ngetahun July 28, 2026 22:23

rtamalin marked this pull request as ready for review July 28, 2026 22:50

rtamalin force-pushed the avoid_profile_related_request_failures branch from 1b4ed3d to 51b2cd2 Compare July 30, 2026 14:34

digitaltom reviewed Jul 31, 2026

View reviewed changes

rtamalin force-pushed the avoid_profile_related_request_failures branch from 51b2cd2 to d00dd3e Compare August 6, 2026 12:44

rtamalin marked this pull request as draft August 6, 2026 15:13

rtamalin force-pushed the avoid_profile_related_request_failures branch 3 times, most recently from 3cf0cea to 39a882a Compare August 11, 2026 14:42

rtamalin marked this pull request as ready for review August 11, 2026 15:01

rtamalin force-pushed the avoid_profile_related_request_failures branch 2 times, most recently from e2d0fc5 to 7249884 Compare August 11, 2026 17:51

jeremy-moffitt approved these changes Aug 11, 2026

View reviewed changes

rtamalin requested a review from digitaltom August 11, 2026 19:56

mssola reviewed Aug 12, 2026

View reviewed changes

Comment thread app/controllers/api/connect/v3/subscriptions/systems_controller.rb Outdated

Comment thread app/controllers/api/connect/v3/subscriptions/systems_controller.rb Outdated

rtamalin marked this pull request as draft August 12, 2026 14:01

rtamalin force-pushed the avoid_profile_related_request_failures branch from b7da50b to ef8beb8 Compare August 12, 2026 14:07

jeremy-moffitt self-requested a review August 12, 2026 16:21

rtamalin force-pushed the avoid_profile_related_request_failures branch from ef8beb8 to c1a5af9 Compare August 12, 2026 20:21

rtamalin requested a review from mssola August 12, 2026 20:55

rtamalin marked this pull request as ready for review August 12, 2026 20:55

rtamalin added 3 commits August 14, 2026 16:29

Move system.create!() retry to separate subroutine

e4320e9

This allows the PubCloud engine overrides for the announce_system request handling to leverage it to do the retry without profiles if profiles were provided in the original request. This addresses @digitaltom's review feedback.

rtamalin force-pushed the avoid_profile_related_request_failures branch from c1a5af9 to f92d0c9 Compare August 14, 2026 20:29

Uh oh!

Conversation

rtamalin commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How to test

Change Type

Checklist

Review

Uh oh!

rtamalin commented Jul 28, 2026

Uh oh!

rtamalin commented Jul 29, 2026

Uh oh!

rtamalin commented Jul 31, 2026

Uh oh!

jeremy-moffitt commented Jul 31, 2026

Uh oh!

rtamalin commented Jul 31, 2026

Uh oh!

jeremy-moffitt commented Jul 31, 2026

Uh oh!

rtamalin commented Jul 31, 2026

Uh oh!

rtamalin commented Jul 31, 2026

Uh oh!

digitaltom Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

rtamalin Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

rtamalin commented Aug 6, 2026

Uh oh!

jeremy-moffitt left a comment

Choose a reason for hiding this comment

Uh oh!

mssola left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mssola commented Aug 12, 2026

Uh oh!

rtamalin commented Aug 12, 2026

Uh oh!

rtamalin commented Aug 12, 2026

Uh oh!

rtamalin commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rtamalin commented Jul 28, 2026 •

edited

Loading