Add recursive-related-bundles endpoint #422
Conversation
|
@release-engineering/exd-guild-hello-operator Can I please request a review on the approach being used to get recursive-related-bundles? Tests and documentation changes are coming up.. |
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
4 times, most recently
from
b5c7698 to
6dbe8b2
Compare
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
3 times, most recently
from
d9a194a to
1abe1ef
Compare
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
from
1abe1ef to
6d22f29
Compare
| @@ -87,6 +87,7 @@ services: | |||
| - ./docker/message_broker/certs:/broker-certs:ro,z | |||
| - request-logs-volume:/var/log/iib/requests:z | |||
| - request-related-bundles-volume:/var/lib/requests/related_bundles:z | |||
| - request-recursive-related-bundles-volume:/var/lib/requests/recursive_related_bundles:z | |||
There was a problem hiding this comment.
Are you planning on getting this merged before or after #421? The latter one will need to be updated after merging the former.
There was a problem hiding this comment.
yes, I am since this one is a bigger change
iib/web/migrations/versions/625fba6081be_add_recursive_related_bundles_endpoint.py
Show resolved
Hide resolved
iib/web/api_v1.py
Outdated
| @login_required | ||
| def recursive_related_bundles(): | ||
| """ | ||
| Submit a request to get nested related bundles of an operator bundle image in a DFS manner. |
There was a problem hiding this comment.
Should we note here that it is a post-order traversal? Somewhere else? We don't want in-order or pre-order traversals as either of these can have the parent before the children.
There was a problem hiding this comment.
hm, actually I implemented a level order traversal. So all child nodes will appear first in the list . And, I agree we should mention it here and somewhere in the docs. On it.
There was a problem hiding this comment.
a reversed level-order traversal? I get lost in all of the possible variants... especially when you diverge from the "named ones" . If you write it, I am sure that it will be sufficient.
| metadata_path = os.path.join(temp_dir, 'metadata') | ||
| _copy_files_from_image(bundle_image_resolved, '/metadata', metadata_path) | ||
| if organization: | ||
| _adjust_operator_bundle(manifests_path, metadata_path, request_id, organization) |
There was a problem hiding this comment.
I assume that you want to do this so that you can modify any pullspecs to ensure that they are actually pullable so that you can then get_related_bundle_images later.
This will process all operations requested for the organization. If an organization has multiple registry_replacements customizations specified, however, intermediate actions to make images pullable might be overruled by later operations to change the registry to a final (and not yet valid) pullspec.
Assuming that the organizations are properly configured, they can specify the related_bundles type at a location where the images can be pulled. We should have that process cache off the related bundles so that we can then read from that cache when we call it again lower.
There was a problem hiding this comment.
Out of band, you commented that we don't need the pullspecs to be resolvable, but we do. As part of the later function call get_related_bundle_images(bundle_metadata), we need to be able to get the image labels to determine whether the related image is a bundle or not:
for related_pullspec_obj in bundle_metadata['found_pullspecs']:
related_pullspec = related_pullspec_obj.to_str()
if yaml.load(
get_image_label(related_pullspec, 'com.redhat.delivery.operator.bundle') or 'false'
):
related_bundle_images.append(related_pullspec)There was a problem hiding this comment.
As discussed on the call, I have made the changes to terminate adjusting the opertor bundle when related_bundles customization is reached.
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
2 times, most recently
from
1fc564e to
6ead456
Compare
|
The code looks good to me. Do we want to release it since you are saying "Tests and documentation changes are coming up.." ... are these unit tests enough or do we want to have iib-api-tests before release? |
|
@lipoja I have now added tests and doc changes too. I think this is good to go |
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
from
6ead456 to
5d931f2
Compare
Refers to CLOUDDST-14678
yashvardhannanavati
force-pushed
the
recursive_regenerate_bundle
branch
from
5d931f2 to
ad2b3fb
Compare
Refers to CLOUDDST-14678