Skip to content

Conversation

@zmc
Copy link
Member

@zmc zmc commented Nov 6, 2025

  • At schedule time, actually check for the existence of a container image rather than for a completed package build
  • Provide some helper functions so that tasks can locate images based on sha1/distro/flavor

@zmc zmc requested a review from a team as a code owner November 6, 2025 03:18
@zmc zmc requested review from amathuria and kamoltat and removed request for a team November 6, 2025 03:18
adk3798
adk3798 previously approved these changes Nov 6, 2025
Copy link

@adk3798 adk3798 left a comment

Choose a reason for hiding this comment

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

Seems like it builds all the names and things properly. I assume we'll have to have the cephadm task make use of this? Right now it looks like we just get the sha1 from the config and append it as the image tag to some default image base

@zmc zmc force-pushed the container-images branch 2 times, most recently from 343c239 to 0a6690b Compare November 6, 2025 18:28
@zmc zmc requested a review from adk3798 November 6, 2025 22:34
As opposed to checking for a specific completed package build, and inferring the
status of the container based on the result. This check will only affect jobs
which use the cephadm task.

Signed-off-by: Zack Cerza <zack@cerza.org>
@zmc zmc force-pushed the container-images branch from 0a6690b to bf8ca42 Compare November 6, 2025 23:01
Signed-off-by: Zack Cerza <zack@cerza.org>
@zmc
Copy link
Member Author

zmc commented Nov 6, 2025

Seems like it builds all the names and things properly. I assume we'll have to have the cephadm task make use of this? Right now it looks like we just get the sha1 from the config and append it as the image tag to some default image base

I've just pushed a commit that causes teuthology to inspect any cephadm task invocations it finds, and check images mentioned there as well. This is in addition to It checking for the existence of the default image.

@functools.lru_cache()
def container_image_exists(image: str):
"""
Use the Quay API to check for the existence of a container image.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this code work for any container registry api? If so, the description is better to change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should it? I'm not sure that is the case, but if you have a reference I would love to see it

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this method supposed to be working with other registry as well as ceph itself. For example, downstream teuthology deploys fine cephadm using third party container registry, but if we merge this quay only specific check it might break this. So, we either support some universal api or we need to check if we actually can check the existence of the container.

Copy link
Member Author

Choose a reason for hiding this comment

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

Which downstream product an which non-quay registry?

I can tweak this so that it skips the check if, for example, the host portion of the image locator doesn't contain 'quay', and then interested parties can contribute checks using APIs they need to query

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants