Skip to content

ROX-34063: Fix v1tov2 image migration logic to set correct image name#19883

Open
charmik-redhat wants to merge 2 commits intomasterfrom
charmik/fix-v1tov2-image-migration-logic-to-set-correct-image-name
Open

ROX-34063: Fix v1tov2 image migration logic to set correct image name#19883
charmik-redhat wants to merge 2 commits intomasterfrom
charmik/fix-v1tov2-image-migration-logic-to-set-correct-image-name

Conversation

@charmik-redhat
Copy link
Copy Markdown
Contributor

Description

Bug summary:

After the deployment_containers are migrated to have imageV2Id populated, image reprocessing is used to migrate images from images table to images_v2 table. This is done using reprocessImageV2 function in the reprocessor as follows:

  • Get all deployed container images by using GetImageContainerViews
  • For each container image, get the V1 image from images table using GetImageMetadata on imageV1 datastore
  • Convert V1 image to V2 using ConvertToV2 function
  • Write the V2 image to images_v2 table using imageV2 datastore

The conversion in ConvertToV2 copies the legacy image's name directly. However, images table only stores one image per digest whereas images_v2 can store multiple images per digest with different names. So the name of the deployed container image can be different than the V1 image obtained by GetImageMetadata . Using ConvertToV2 directly on V1 image can result in migrated V2 image to have mismatched name and ID.

Fix

Pass the deployed image's name (obtained from GetContainerImageViews) through to the conversion function so the migrated V2 image gets the correct name and derived ID.

  • ConvertToV2WithNameOverride (pkg/images/utils/convert_utils.go): New function that accepts a *storage.ImageName override, so the converted V2 image uses the deployed image's name (and
    correct derived ID) instead of the legacy image's name.
    • ContainerImageView (central/deployment/views/container_images.go): Added image name fields (Registry, Remote, Tag, FullName) and a GetImageName() getter.
    • GetContainerImageViews (central/deployment/datastore/internal/store/postgres/full_store.go): Updated the SQL query to select and group by the image name fields.
    • reprocessImageV2 (central/reprocessor/reprocessor.go): Now receives the deployed image's name from the caller and passes it to ConvertToV2WithNameOverride during legacy-to-V2 migration.
    • Tests updated with image name assertions for both FlattenImageData enabled and disabled paths.

User-facing documentation

  • CHANGELOG update is not needed
  • documentation is not needed

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • Unit tests
  • Will validate this manually in the PR enabling the feature flag and adding migration for populating imageIDV2 in deployments_containers

charmik-redhat and others added 2 commits April 7, 2026 18:57
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines -8 to -10
// Each imageIDV2 will map to a single digest, but SQL wants us to apply an aggreate func to all selected fields
// when grouing by imageIDV2. So we need to select the digest with a min aggregate,
// which will be a no-op for a single value.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comment was no longer relevant

@charmik-redhat charmik-redhat requested a review from a team April 8, 2026 04:53
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

🚀 Build Images Ready

Images are ready for commit 0899fa6. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-588-g0899fa6cdd

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.58%. Comparing base (6a9c388) to head (0899fa6).

Files with missing lines Patch % Lines
...nt/datastore/internal/store/postgres/full_store.go 0.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19883      +/-   ##
==========================================
- Coverage   49.58%   49.58%   -0.01%     
==========================================
  Files        2766     2766              
  Lines      208540   208551      +11     
==========================================
  Hits       103408   103408              
- Misses      97460    97471      +11     
  Partials     7672     7672              
Flag Coverage Δ
go-unit-tests 49.58% <20.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 8, 2026

@charmik-redhat: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-qa-e2e-tests 0899fa6 link false /test gke-qa-e2e-tests
ci/prow/gke-ui-e2e-tests 0899fa6 link true /test gke-ui-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants