- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
          🐛🚑️ Make director-v0 work with registry:3.0.0
          #8239
        
          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
  
    🐛🚑️ Make director-v0 work with registry:3.0.0
  
  #8239
              Conversation
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##           master    #8239      +/-   ##
==========================================
- Coverage   88.02%   88.01%   -0.01%     
==========================================
  Files        1916     1916              
  Lines       74053    74053              
  Branches     1301     1301              
==========================================
- Hits        65184    65180       -4     
- Misses       8478     8482       +4     
  Partials      391      391              
 
 Continue to review full report in Codecov by Sentry. 
 🚀 New features to boost your workflow:
  | 
    
          
 | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's see what others think as well
| "application/vnd.oci.image.manifest.v1+json", | ||
| "application/vnd.oci.image.index.v1+json", | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not fully convinced about this approach. What about addressing the issue in the code?
I think it's better to use the default version 2 spec instead of requesting the older one I would expect this one to get deprecated at some point.
log_level=ERROR | log_timestamp=2025-08-20 09:01:21,941 | log_source=simcore_service_director.registry_proxy:log_catch(575) | log_uid=None | log_oec=None | log_trace_id=0 | log_span_id=0 | log_msg=Unhandled exception:
Traceback (most recent call last):
  File "/home/scu/.venv/lib/python3.11/site-packages/servicelib/logging_utils.py", line 570, in log_catch
    yield
  File "/home/scu/.venv/lib/python3.11/site-packages/simcore_service_director/registry_proxy.py", line 449, in get_repo_details
    if image_details := await image_details_future:
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/scu/.venv/lib/python3.11/site-packages/simcore_service_director/registry_proxy.py", line 402, in get_image_details
    labels, image_manifest_digest = await get_image_labels(
                                    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/scu/.venv/lib/python3.11/site-packages/simcore_service_director/registry_proxy.py", line 384, in get_image_labels
    v1_compatibility_key = json_loads(request_result["history"][0]["v1Compatibility"])
                                      ~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'history'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is addressed in the code ;) These are code-changes after all. In case you want to do it differently, feel free to take it over in case ;) I interpret your comment as that I should not quickly get this in (I would have done it this way) but wait for some others. Must I wait for Sylvain, or even Pedro?
          
🧪 CI InsightsHere's what we observed from your CI run for 380e5d3. ❌ Failed Jobs
 ✅ Passed Jobs With Interesting Signals
  | 
    
| 
           @mrnicegyu11 just created #8240 to ensure the dv-0 is compatible with both types. This was adjusted to the newer schema version. Since multi-arch images might become a thing soonish, I guess the dv0 shall become compatible at some point as well. This would be a first step  | 
    
| 
           Closing in favor of #8240 thanks a bunch @sanderegg , appreciated!  | 
    



What do these changes do?
Turns out that in some internal calls the dv0 relies on the docker V1 registry protocol. The
registry:3.0.0seems to by default return v2 specs if the client to the docker registry does not specify what is acceptable. Via the usualAcceptheaders we can inform the docker registry even on version 3.0.0 that we would like to recieve only oci-image v1 data, which is compatible with the dv0 code.Bonus:
Update registry to
3.0.0everywhereRelated issue/s
How to test
Tested locally (
make up-devel)Dev-ops