Don't perform index image verification when overwrite_from_index is False #166
Conversation
|
No risk of data loss but IIB may report incorrect from index resolved image data. @shawn-hurley tells me that we can now use a digest for the index image pull spec. If we do that we can safely remove this check for all cases. Can we try that instead? |
iib/workers/tasks/build.py
Outdated
| prebuild_info['from_index_resolved'], from_index, overwrite_from_index_token | ||
| ) | ||
| if overwrite_from_index: | ||
| _verify_index_image( |
There was a problem hiding this comment.
Maybe in the future, but it would be great if this was done as part of the _overwrite_from_index function.
querti
force-pushed
the
remove-sanity-check
branch
2 times, most recently
from
c305302 to
44affa0
Compare
|
@yashvardhannanavati @shawn-hurley @mprahl check pls |
iib/workers/tasks/build.py
Outdated
| @@ -617,10 +629,13 @@ def _overwrite_from_index( | |||
| :param str output_pull_spec: the pull specification of the manifest list for the index image | |||
| that IIB built. | |||
| :param str from_index: the pull specification of the image to overwrite. | |||
| :param str resolved_prebuild_from_index: resolved index image before starting the build. | |||
| :param str overwrite_from_index_token: the user supplied token to use when overwriting the | |||
| ``from_index`` image. If this is not set, IIB's configured credentials will be used. | |||
| :raises IIBError: if one of the skopeo commands fails. | |||
There was a problem hiding this comment.
This needs to also reflect that _verify_index_image can raise an exception.
querti
force-pushed
the
remove-sanity-check
branch
from
44affa0 to
bd88a9d
Compare
|
@mprahl @lcarva @yashvardhannanavati Can you merge this? I don't have the rights |
|
One thing I just thought of is that the resolved from index is marked in the database at the beginning of the request, but if it changed during the request, then what is recorded as the resolved from index may be different than what was used. iib/iib/workers/tasks/build.py Lines 551 to 553 in 99bbcf8 Once the resolved from index pullspec can be used, then we can actually just remove the verification code altogether. It looks like podman 1.9.2 allows us to specify a digest for the from index: If you'd still like to pursue with this approach, perhaps you can update the resolved from index image in the database after the opm command. |
I've actually already filed a story for implementing what you're describing. Luiz and I have agreed that this PR is an okay short-term solution (as some teams seem affected by the verification failures). Also, I think that the verification will still be required for overwriting the index image in order to prevent data loss. |
|
@querti could you please resolve conflicts and update the PR? |
…alse There is no risk of data loss when IIB doesn't overwrite the original index image. Thus, to prevent unnecessary failures, this check is disabled when overwrite_from_index is False. The check has been moved to overwrite_from_index function, as it is now only performed before overwrite_from_index is attempted.
querti
force-pushed
the
remove-sanity-check
branch
from
bd88a9d to
42975ba
Compare
|
@yashvardhannanavati done. |
There is no risk of data loss when IIB doesn't overwrite the original index image. Thus, to prevent unnecessary failures, this check is disabled when overwrite_from_index is False.
More info in CLOUDDST-2783