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

ci: parameterize registry and add zot as another candidate #709

Closed
wants to merge 1 commit into from

Conversation

rchincha
Copy link

@rchincha rchincha commented Dec 1, 2022

Signed-off-by: Ramkumar Chinchani rchincha@cisco.com

@rchincha
Copy link
Author

rchincha commented Dec 1, 2022

Looks like there is very docker-specific env setup. Should probably standardize on that also.

@qweeah
Copy link
Contributor

qweeah commented Dec 2, 2022

Looks like there is very docker-specific env setup. Should probably standardize on that also.

The env setup now is based on mount files in distribution storage layout. ZOT storage is designed based on OCI layout so it's not compatible. We will definitely standardize the setup process when CLI supports OCI layout target.

@qweeah
Copy link
Contributor

qweeah commented Dec 2, 2022

Also specs related to docker images will be covered in ORAS E2E tests, @rchincha how to run those based on ZOT?

@FeynmanZhou
Copy link
Member

FeynmanZhou commented Dec 2, 2022

@rchincha Hi Ramkumar, it seems this PR make the E2E testing fail. It has some tech limitations to adding Zot to the CI.

BTW, I would like to add the Zot registry to the ORAS quick start, see oras-project/oras-www#71.

@rchincha
Copy link
Author

rchincha commented Dec 2, 2022

@rchincha Hi Ramkumar, it seems this PR make the E2E testing fail. It has some tech limitations to adding Zot to the CI.

BTW, I would like to add the Zot registry to the ORAS quick start, see oras-project/oras-www#71.

Trying to understand the test setup a bit more. It appears data is manually loaded, which is unlikely to work with every registry.

@rchincha
Copy link
Author

rchincha commented Dec 2, 2022

@qweeah
Copy link
Contributor

qweeah commented Dec 3, 2022

Trying to understand the test setup a bit more. It appears data is manually loaded, which is unlikely to work with every registry.

Yes, once oras CLI supports OCI layout, we can store test data in OCI layout. I created an issue for that.

@rchincha
Copy link
Author

rchincha commented Dec 5, 2022

Trying to understand the test setup a bit more. It appears data is manually loaded, which is unlikely to work with every registry.

Yes, once oras CLI supports OCI layout, we can store test data in OCI layout. I created an issue for that.

You can also do:

skopeo copy --format=oci docker://busybox:latest oci:/tmp/vol/busybox:latest

docker run -v /tmp/vol:/var/lib/registry -p 5000:5000 ghcr.io/project-zot/zot-minimal-linux-amd64:latest

Basically two parts:

  1. registry-specific - storage, authN, etc
  2. registry-agnostic - OCI conformance really

@shizhMSFT
Copy link
Contributor

@rchincha Thanks for creating this PR for proposing zot as a candidate registry for oras E2E testing.

In our current plan, we are using github.com/oras-project/distribution, which is a specially crafted fork of github.com/distribution/distribution, for the E2E testing.

To achieve the best quality of E2E testing, we can't use oras to populate the data to be tested since oras itself can be flawed. It is also not a good idea to use other tools to populate data since the other tools may not support oras specific features and they also need to be well tested. That's why we manually prepare test data for the distribution, which is unfortunately distribution specific. Maybe in the future, we can use the older version of oras to populate data to test the latest version of oras just like compiler bootstrapping.

zot is a good candidate for oras E2E testing. However, it is an OCI-only registry. If it can support docker manifests or even custom manifests, we can definitely use zot for oras E2E testing.

@rchincha
Copy link
Author

rchincha commented Dec 6, 2022

To achieve the best quality of E2E testing, we can't use oras to populate the data to be tested since oras itself can be flawed. It is also not a good idea to use other tools to populate data since the other tools may not support oras specific features and they also need to be well tested.

@shizhMSFT, behavior of oras wrt a registry should only be concerned with HTTP API compatibility right? So the registry itself should be allowed to be setup in any which way. Otherwise oras' contract expands to say that only registries which support docker media-types work with oras.

As the e2e stands today, understood that it is tightly coupled to docker registry and sounds like you want to leave the e2e alone.

My suggestion (also see above) would be to break the workflow into two parts - a registry-specific setup, after which oras makes its HTTP API calls. Updating the PR accordingly. See if this works for you.

@codecov-commenter
Copy link

Codecov Report

Merging #709 (f3e9d30) into main (b89736e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #709   +/-   ##
=======================================
  Coverage   72.12%   72.12%           
=======================================
  Files          14       14           
  Lines         513      513           
=======================================
  Hits          370      370           
  Misses        114      114           
  Partials       29       29           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shizhMSFT
Copy link
Contributor

Otherwise oras' contract expands to say that only registries which support docker media-types work with oras.

This statement is not true. oras is designed to be compatible with OCI-compliant registries and Docker-compliant registries. In other words,

  • If a registry is docker-only, oras is compatible with it.
  • If a registry is OCI-only, oras is compatible with it.
  • If a registry is compliant with docker and OCI, oras is compatible with it.
  • If a registry is partially compliant with docker or OCI specs, oras may still be compatible with it. For example, if we want to experiment a new manifest type, oras can still work with that manifest type even if it is not defined by oci or docker specs.

That's why we made a testing registry (github.com/oras-project/distribution) and only for testing, which supports OCI and Docker. Although it is not suitable for production, we can easily modify it to test all scenarios. Again, we cannot use Docker-only registries since they don't support OCI. Similarly, we cannot use OCI-only registries since they don't support Docker.

@rchincha
Copy link
Author

rchincha commented Dec 7, 2022

@shizhMSFT thanks for the clarification. Once this PR passes CI, maybe it can parameterized to included other registries - docker or OCI.

@rchincha
Copy link
Author

rchincha commented Dec 9, 2022

Fixed the PR. Hopefully should pass the CI checks this time.

@rchincha
Copy link
Author

rchincha commented Dec 9, 2022

rchincha#1
^ test this action here, including the attach example. Pls wait till this passes.

@rchincha
Copy link
Author

rchincha commented Dec 9, 2022

rchincha#1
^ now ready, pls note oras temporarily pinned to v0.16.0 due to #719 so remove that section later.

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
@rchincha
Copy link
Author

PR updated after the oras-go bump.

@rchincha
Copy link
Author

notaryproject/notation#493
^ do you plan to do the same wrt this PR?

@shizhMSFT
Copy link
Contributor

notaryproject/notation#493 ^ do you plan to do the same wrt this PR?

/cc @qweeah

@shizhMSFT shizhMSFT added the stale Inactive issues or pull requests label Jun 28, 2023
@qweeah qweeah removed the stale Inactive issues or pull requests label Jul 24, 2023
run: |
# upload images, zot can serve OCI image layouts directly like so
mkdir /tmp/zot
skopeo copy --format=oci docker://busybox:latest oci:/tmp/zot/busybox:latest
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we could do this later with oras

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale Inactive issues or pull requests label Sep 12, 2023
@qweeah
Copy link
Contributor

qweeah commented Sep 12, 2023

Closing as ZOT is already used as E2E backend since v1.1.0

@qweeah qweeah closed this Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Inactive issues or pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants