Skip to content
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

feat(RELEASE-1310): add update-cr-status to push-disk-images #717

Merged

Conversation

maorfr
Copy link
Contributor

@maorfr maorfr commented Dec 1, 2024

this commit adds the update-cr-status task to the push-disk-images pipeline.

related to: https://issues.redhat.com/browse/RELEASE-1310

@maorfr maorfr requested a review from a team as a code owner December 1, 2024 09:12
Copy link

openshift-ci bot commented Dec 1, 2024

Hi @maorfr. Thanks for your PR.

I'm waiting for a konflux-ci member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@gbenhaim
Copy link
Member

gbenhaim commented Dec 1, 2024

/ok-to-test

@konflux-ci-qe-bot
Copy link

@maorfr: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name Status Rerun command Build Log Test Log
konflux-e2e-tests-catalog-gj242 Failed /retest View Pipeline Log View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

  1. Install ORAS (see the ORAS installation guide).
  2. Download artifacts with the following commands:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/konflux-team/release-service-catalog:konflux-e2e-tests-catalog-gj242

@maorfr maorfr changed the title feat(RELEASE-1310) add update-cr-status to push-disk-images pipeline feat(RELEASE-1310) add update-cr-status to push-disk-images Dec 2, 2024
@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch 6 times, most recently from fc65c0a to 855066c Compare December 2, 2024 11:46
@maorfr maorfr changed the title feat(RELEASE-1310) add update-cr-status to push-disk-images feat(RELEASE-1310): add update-cr-status to push-disk-images Dec 2, 2024
@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch from 855066c to 7ec35ea Compare December 2, 2024 11:47
Copy link
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

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

I am fine with adding the task to the pipeline although it will do nothing for now. I don't agree with putting the snapshot in release artifacts. What we really need is a story like the ones on this epic https://issues.redhat.com/browse/RELEASE-878 for the push-disk-images task

tasks/push-disk-images/README.md Outdated Show resolved Hide resolved
@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch 2 times, most recently from 575f7fb to d5b0a36 Compare December 2, 2024 21:21
Copy link
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

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

One small thing but otherwise lgtm

tasks/push-disk-images/push-disk-images.yaml Outdated Show resolved Hide resolved
@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch 2 times, most recently from 5ab484c to 4d4fd83 Compare December 2, 2024 21:37
@maorfr
Copy link
Contributor Author

maorfr commented Dec 2, 2024

/retest

1 similar comment
@maorfr
Copy link
Contributor Author

maorfr commented Dec 2, 2024

/retest

@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch 3 times, most recently from 5668f17 to cebc888 Compare December 2, 2024 22:41
Copy link
Contributor

@mmalina mmalina left a comment

Choose a reason for hiding this comment

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

It looks good to me, except for the failing test - as Johnny also pointed out in Slack:

the -production test pipeline failed now because no staged.files exist in the dummy snapshot created for it. All of the tests will need it now (or make the jq not fail if the fields are missing)

@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch from cebc888 to fa6e6e4 Compare December 3, 2024 14:06
Signed-off-by: Maor Friedman <mafriedm@redhat.com>
@maorfr maorfr force-pushed the push-disk-images-update-cr-status branch from fa6e6e4 to 9d59dfe Compare December 3, 2024 14:06
@johnbieren
Copy link
Collaborator

No e2e test for this pipeline so merging without it

@johnbieren johnbieren merged commit 63710b7 into konflux-ci:development Dec 3, 2024
6 of 7 checks passed
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.

5 participants