Skip to content

Link to migration bundles#1861

Merged
tkdchen merged 1 commit into
konflux-ci:mainfrom
tkdchen:link-migration-bundles
Feb 19, 2025
Merged

Link to migration bundles#1861
tkdchen merged 1 commit into
konflux-ci:mainfrom
tkdchen:link-migration-bundles

Conversation

@tkdchen

@tkdchen tkdchen commented Jan 27, 2025

Copy link
Copy Markdown
Contributor

STONEBLD-3214

Any task bundles that have migrations are linked by the newer task bundles by a dedicated annotation. See the following example task bundles:

          +-------+------+ +------+-------+
          v       |      | v      |       |
tb0 -->  tb1 --> tb2 --> tb3 --> tb4 --> tb5
          |               |
          M               M

where, tb2 and tb3 point to tb1, and tb4 and tb5 point to tb3.

Linking task bundles helps to reduce the number of HTTP requests made to Quay.io that query which task bundles have migrations.

Terms:

  • tb means task bundle.
  • M means migration.

@tkdchen tkdchen requested a review from a team as a code owner January 27, 2025 05:50
@tkdchen tkdchen force-pushed the link-migration-bundles branch from 667dc90 to 110af20 Compare January 27, 2025 06:04

@chmeliik chmeliik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice idea!

Comment thread hack/build-and-push.sh Outdated
@tkdchen tkdchen force-pushed the link-migration-bundles branch 2 times, most recently from a72139f to bc49d05 Compare January 27, 2025 09:17
@tkdchen tkdchen force-pushed the link-migration-bundles branch from 1f6fbd8 to 70eeb3f Compare February 10, 2025 01:17
@tkdchen

tkdchen commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

Rebased on main branch for fixing code conflict.

@tkdchen

tkdchen commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

/retest build-definitions-pull-request

@tkdchen

tkdchen commented Feb 11, 2025

Copy link
Copy Markdown
Contributor Author

/test build-definitions-pull-request

1 similar comment
@tkdchen

tkdchen commented Feb 12, 2025

Copy link
Copy Markdown
Contributor Author

/test build-definitions-pull-request

@tkdchen tkdchen requested a review from chmeliik February 12, 2025 02:16
Comment thread hack/build-and-push.sh Outdated
Comment thread hack/build-and-push.sh Outdated
Comment thread hack/build-and-push.sh Outdated
@tkdchen tkdchen force-pushed the link-migration-bundles branch 2 times, most recently from 00e2708 to c38b633 Compare February 13, 2025 05:19
@tkdchen

tkdchen commented Feb 13, 2025

Copy link
Copy Markdown
Contributor Author

Added a new commit to address review comment and rebased to fix code conflict between the new commit and the one merged into the main branch.

@tkdchen tkdchen requested review from chmeliik and mmorhun February 13, 2025 07:40
chmeliik
chmeliik previously approved these changes Feb 14, 2025
Comment thread hack/build-and-push.sh
Comment thread hack/build-and-push.sh Outdated
@tkdchen

tkdchen commented Feb 17, 2025

Copy link
Copy Markdown
Contributor Author

Comment thread hack/build-and-push.sh Outdated
Comment on lines +444 to +447
local limit=1
if [ -n "$TEST_REPO_NAME" ]; then
limit=10
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now that we're using the filter_tag_name param, do we still need to list more than 1 digest? If yes, could you add a comment with the reason why?

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.

Now, only list 1 tag.

Comment thread hack/build-and-push.sh
Comment thread hack/build-and-push.sh
Comment thread hack/build-and-push.sh
Comment thread hack/build-and-push.sh Outdated
Comment thread hack/build-and-push.sh
Comment thread hack/build-and-push.sh Outdated
@tkdchen tkdchen force-pushed the link-migration-bundles branch from 6e59333 to 61ebe5d Compare February 18, 2025 01:23
@tkdchen tkdchen requested review from chmeliik and mmorhun February 18, 2025 01:25
STONEBLD-3214

Any task bundles that have migrations are linked by the newer task
bundles by a dedicated annotation. See the following example task
bundles:

            +-------+------+ +------+-------+
            v       |      | v      |       |
  tb0 -->  tb1 --> tb2 --> tb3 --> tb4 --> tb5
           |               |
           M               M

where, tb2 and tb3 point to tb1, and tb4 and tb5 point to tb3.

Linking task bundles helps to reduce the number of HTTP requests made to
Quay.io that query which task bundles have migrations.

Terms:

- tb means task bundle.
- M means migration.
@tkdchen tkdchen force-pushed the link-migration-bundles branch from 61ebe5d to c7e493a Compare February 18, 2025 01:25
Comment thread hack/build-and-push.sh
output=$(build_push_task "$task_dir" "$prepared_task_file" "$task_bundle" "$task_file_sha" "$has_migration")
output=$(
build_push_task "$task_dir" "$prepared_task_file" "$task_bundle" "$task_file_sha" \
"$has_migration" "$previous_migration_bundle_digest"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is previous_migration_bundle_digest used anywhere?

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.

@mmorhun mmorhun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving with agreement to update the annotation name in a follow up PR.
Also, please take a look at the question above.

@tkdchen tkdchen added this pull request to the merge queue Feb 19, 2025
Merged via the queue into konflux-ci:main with commit b261af3 Feb 19, 2025
@tkdchen tkdchen deleted the link-migration-bundles branch February 19, 2025 07:45
@tkdchen tkdchen mentioned this pull request Feb 19, 2025
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