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(SPSTRAT-465): add task for marketplacesvm #719

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

JAVGan
Copy link
Contributor

@JAVGan JAVGan commented Dec 3, 2024

This commit introduces a new task named marketplaces-push-disk-images which will be used to deliver disk images to various cloud marketplaces using the marketplacesvm_push_wrapper.

Refers to SPSTRAT-465

@JAVGan JAVGan requested a review from a team as a code owner December 3, 2024 13:08
Copy link

openshift-ci bot commented Dec 3, 2024

Hi @JAVGan. 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.

@JAVGan
Copy link
Contributor Author

JAVGan commented Dec 3, 2024

@lslebodn FYI moved here.

@johnbieren is there a way to test that?

@JAVGan JAVGan force-pushed the mkts branch 2 times, most recently from 58d6eb2 to 1bf9eca Compare December 3, 2024 17:56
Copy link

@lslebodn lslebodn left a comment

Choose a reason for hiding this comment

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

Can we somewhere in the code validate staging structure with pushsource-ls
as we discussed in konflux-ci/release-service-utils#326 (comment)?

@JAVGan
Copy link
Contributor Author

JAVGan commented Dec 3, 2024

Can we somewhere in the code validate staging structure with pushsource-ls as we discussed in konflux-ci/release-service-utils#326 (comment)?

Thanks! I forgot it last time. Now it's added 😄

@JAVGan JAVGan force-pushed the mkts branch 2 times, most recently from 9cdbfcf to f3b5002 Compare December 4, 2024 18:25
@JAVGan
Copy link
Contributor Author

JAVGan commented Dec 4, 2024

@mmalina @johnbieren I've added a single test to ensure the task works, but honestly since I'm very new to testing stuff on Konflux I'm really not sure whether I did it correctly or not... What I did was based in the other tasks with some adaptations for our use cases, please let me know what I need to fix/change and I'll do it.

However, by implementing such tests I ended up with the following questions:

  1. As asked earlier do we need to have our task as internal? We're just using oras pull and select-oci-auth to deal with quay.io and for the rest we don't need anything from within the firewall... If you agree the task can be external I'll move it out from the internal directory.
  2. By looking at the tests from push-disk-images I noticed you have 2 different JSON files: snapshot_spec.json and data.json which are combined here. I assumed the snapshot_json was incoming as a whole, like what we discussed before on slack channel and my task assumes it's a single JSON as well... Should I update it to receive two JSONs aside from our starmap mappings? It would end up with 3 JSONs for the parameters if yes...
  3. Is there a way I can "validate" or "run the tests" in some place before asking you guys to run it on CI? I believe it would be faster to catch errors and fix stuff.

@JAVGan JAVGan marked this pull request as draft December 5, 2024 18:00
@JAVGan JAVGan force-pushed the mkts branch 3 times, most recently from 572b908 to 5997329 Compare December 5, 2024 19:09
@JAVGan JAVGan force-pushed the mkts branch 3 times, most recently from f18b2e0 to 4137c66 Compare December 6, 2024 18:04
@JAVGan JAVGan marked this pull request as ready for review December 6, 2024 18:04
@johnbieren
Copy link
Collaborator

/ok-to-test

@johnbieren
Copy link
Collaborator

E2E irrelevant here. Will merge tomorrow unless @mmalina comments something to change

mmalina
mmalina previously approved these changes Dec 12, 2024
johnbieren
johnbieren previously approved these changes Dec 12, 2024
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.

/ok-to-test

@JAVGan
Copy link
Contributor Author

JAVGan commented Dec 12, 2024

@johnbieren @mmalina Fixed an unbound variable

@johnbieren
Copy link
Collaborator

/ok-to-test

johnbieren
johnbieren previously approved these changes Dec 12, 2024
@openshift-ci openshift-ci bot added the lgtm label Dec 12, 2024
@konflux-ci-qe-bot
Copy link

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

PipelineRun Name Status Rerun command Build Log Test Log
konflux-e2e-tests-catalog-xxhfx 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-xxhfx

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.

One last small comment and then it should be ready to merge.

This commit introduces a new task named
`marketplacesvm-push-disk-images` which will be used to
 deliver disk images to various cloud marketplaces
using the `marketplacesvm_push_wrapper`.

Signed-off-by: Jonathan Gangi <[email protected]>
@openshift-ci openshift-ci bot added the lgtm label Dec 12, 2024
@mmalina mmalina merged commit 28b31d8 into konflux-ci:development Dec 12, 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