Skip to content

Update devfile tests with OCI-based registry #4679

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

Merged
merged 4 commits into from
May 10, 2021
Merged

Update devfile tests with OCI-based registry #4679

merged 4 commits into from
May 10, 2021

Conversation

josephca
Copy link
Member

@josephca josephca commented Apr 30, 2021

What type of PR is this?
/kind cleanup

What does this PR do / why we need it:
After moving to use OCI-based registry (#4525), devfile tests should be using OCI-based registry as well.

Which issue(s) this PR fixes:

Related issues:
devfile/api#367
#4504
#4525

PR acceptance criteria:

How to test changes / Special notes to the reviewer:
This PR is for changing test codes.

@josephca
Copy link
Member Author

/assign @GeekArthur

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: josephca
To complete the pull request process, please assign prietyc123 after the PR has been reviewed.
You can assign the PR to them by writing /assign @prietyc123 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

@rnapoles-rh rnapoles-rh left a comment

Choose a reason for hiding this comment

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

@mohammedzee1000 Looks like both:

and

have the same content, why use different URLs?

@josephca
Copy link
Member Author

josephca commented May 3, 2021

@mohammedzee1000 Looks like both:

and

have the same content, why use different URLs?

@rnapoles-rh
I intentionally used two registries to test out different test cases against the official OCI-based registry vs. staging registry (contains the latest contents) and also to ensure the availability of both registries.

Here is the detail :

  • Official registry (https://registry.devfile.io) updated weekly vs. staging registry(https://registry.stage.devfile.io) updated with PR merges.
  • odo registry command test uses the official registry as it doesn't cause much overload on the public registry vs. odo catalog test uses the staging registry to avoid potential overload on the official registry.

* 'main' of github.com:openshift/odo:
  Make Telemetry upload faster (#4565)
@josephca
Copy link
Member Author

josephca commented May 4, 2021

@GeekArthur - please review. Thanks,

@girishramnani
Copy link
Contributor

@mohammedzee1000 Looks like both:

and

have the same content, why use different URLs?

@rnapoles-rh
I intentionally used two registries to test out different test cases against the official OCI-based registry vs. staging registry (contains the latest contents) and also to ensure the availability of both registries.

Here is the detail :

  • Official registry (https://registry.devfile.io) updated weekly vs. staging registry(https://registry.stage.devfile.io) updated with PR merges.
  • odo registry command test uses the official registry as it doesn't cause much overload on the public registry vs. odo catalog test uses the staging registry to avoid potential overload on the official registry.

So odo tests would fail if there is breaking changing in the staging registry? wouldn't that block us?

@prietyc123
Copy link
Contributor

prietyc123 commented May 5, 2021

So odo tests would fail if there is breaking changing in the staging registry? wouldn't that block us?

Even I do feel the same. @josephca Currently the breaking changes in devfile registry are bothering us more and now it seems like we need to deal with staging one as well, no?

@josephca
Copy link
Member Author

josephca commented May 5, 2021

So odo tests would fail if there is breaking changing in the staging registry? wouldn't that block us?

Even I do feel the same. @josephca Currently the breaking changes in devfile registry are bothering us more and now it seems like we need to deal with staging one as well, no?

@girishramnani @prietyc123 Thanks for your inputs.
After discussion with devfile service team, it would be sufficient to test only with staging server. In addition, they will try to get some telemetry data in the future to track usage patterns on the community registry. Therefore, it will be good to use the staging server for testing to avoid polluting the data.

That said, I will update the code to use the staging server only.

@josephca josephca requested a review from rnapoles-rh May 5, 2021 17:05
@josephca
Copy link
Member Author

josephca commented May 5, 2021

/retest

1 similar comment
@josephca
Copy link
Member Author

josephca commented May 5, 2021

/retest

Copy link
Contributor

@rnapoles-rh rnapoles-rh left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 6, 2021
@josephca josephca requested a review from rnapoles-rh May 6, 2021 18:01
@josephca
Copy link
Member Author

josephca commented May 6, 2021

/assign @rnapoles-rh
Can you approve? Thanks,

@girishramnani
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented May 10, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani, josephca

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label May 10, 2021
@openshift-merge-robot openshift-merge-robot merged commit a920ccc into redhat-developer:main May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants