-
Notifications
You must be signed in to change notification settings - Fork 297
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 #38072 - add host bootc_images endpoint #11257
base: master
Are you sure you want to change the base?
Conversation
Rubocop errors are unrelated & on master. |
dd43f60
to
7bbefa8
Compare
Does it make sense to add filtering params to this API endpoint? Like filter for hosts or digest etc? |
I think it makes sense, it would enable there to be a search bar in the UI. Question is, which params should we support? Perhaps:
|
I'm thinking I might need to make some changes to how the returned data is structured to better suit UI development. Seems like more keys might be expected. I'd also like to follow the general standard we have where "name" before the image path might be one key I should add. |
I've added full support for scoped_search searching via the Host::Managed model. My only regret is that I have to load up the host IDs first and then create the content facets query, but I don't see a good way yet to combine the two queries into one. |
29a8e15
to
723f2e8
Compare
content_facets = ::Katello::Host::ContentFacet.where(host_id: ::Host::Managed.joins(:content_facet).search_for(host_search).pluck(:id)) | ||
aggregate_bootc_data = content_facets.where.not(bootc_booted_image: nil, bootc_booted_digest: nil). | ||
select(:bootc_booted_image, :bootc_booted_digest, 'COUNT(hosts.id) as host_count'). | ||
joins(:host).group(:bootc_booted_image, :bootc_booted_digest).order(:bootc_booted_image) |
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.
This order would need to be parameterized based on API param.
573a441
to
c1b3695
Compare
Sorting support is in! |
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.
APJ..Code looks good..Everything works well in testing..👍
c1b3695
to
81a551d
Compare
What are the changes introduced in this pull request?
Adds a new
/hosts/bootc_images
endpoint that returns an overview of all bootc images that hosts are using.The return data looks like the following:
This data will power a page that shows a list of image paths and each image mode host that is using that image path. Since each host could use the same image but a different digest, each image path holds any number of digests with the count of hosts that are using that digest. More digests per image path means more drift from the latest container content under that container tag.
Considerations taken when implementing this change?
Choosing where this goes was difficult -- it's related to hosts so I decided to stick it under the hosts endpoint. I use facts as a similar paradigm: host facts and bootc images are both aggregate information from hosts that are available at a single API endpoint.
What are the testing steps for this pull request?
curl "https://`hostname`/api/hosts/bootc_images?per_page=5&page=7" -uadmin:pass