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

Dynamic registry support #2940

Conversation

GeekArthur
Copy link
Contributor

@GeekArthur GeekArthur commented Apr 20, 2020

Signed-off-by: jingfu wang jingfu.j.wang@ibm.com

What type of PR is this?
/kind feature

What does does this PR do / why we need it:
Currently we only support static registry, which means user only can create component with devfile that is hosted on our two predefined registries:
Che devfile registry: https://github.com/eclipse/che-devfile-registry/tree/master/devfiles
Private devfile registry: https://github.com/elsony/devfile-registry/tree/master/devfiles

This PR is to implement dynamic registry support, with this PR users can easily configure their own registries with the following operations:

  • Add registry to the registry list
  • List registry in the registry list
  • Delete registry from the registry list
  • Update registry in the registry list
  • List component with corresponding registry
  • Create component that is hosted by specific registry

Which issue(s) this PR fixes:
Fixes #2635

How to test changes / Special notes to the reviewer:

# Add registry to the registry list
odo registry add <registry name> <registry URL>

# List registry in the registry list
odo registry list

# Delete registry from the registry list
odo registry delete <registry name>

# Update registry in the registry list
odo registry update <registry name> <registry URL>

# List component with corresponding registry
odo catalog list components

# Create component that is hosted by specific registry
odo create <component type> --registry <registry name>

Special notes:

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Apr 20, 2020
@GeekArthur
Copy link
Contributor Author

GeekArthur commented Apr 20, 2020

I will add unit tests and integration tests in my following commit of this PR.

@GeekArthur
Copy link
Contributor Author

/assign

@GeekArthur
Copy link
Contributor Author

/area devfile

@openshift-ci-robot openshift-ci-robot added the area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. label Apr 20, 2020
@codecov
Copy link

codecov bot commented Apr 21, 2020

Codecov Report

Merging #2940 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2940   +/-   ##
=======================================
  Coverage   45.50%   45.50%           
=======================================
  Files         109      109           
  Lines       10501    10501           
=======================================
  Hits         4778     4778           
  Misses       5264     5264           
  Partials      459      459           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fc8488...0fc8488. Read the comment docs.

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
Copy link

@elsony elsony left a comment

Choose a reason for hiding this comment

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

We also need to handle gracefully if the user already have a preference.yaml file but have no registry entry, i.e. a preference file created in previous version of odo. In that case, we can just default back to the default list of repos (instead of empty); otherwise, the user will need to workaround the case to manually add the default list of repos before they can get odo catalog list components to work again after they update to the latest version of odo.

@GeekArthur
Copy link
Contributor Author

We also need to handle gracefully if the user already have a preference.yaml file but have no registry entry, i.e. a preference file created in previous version of odo. In that case, we can just default back to the default list of repos (instead of empty); otherwise, the user will need to workaround the case to manually add the default list of repos before they can get odo catalog list components to work again after they update to the latest version of odo.

Yup, my latest commit will handle the following two cases regarding new odo version migration and add default registries for user:

  • User doesn't have preference.yaml file
  • User has preference.yaml file but doesn't use dynamic registry feature before

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@groeges
Copy link
Contributor

groeges commented Apr 22, 2020

If we are adding the capability to add registries, should we still be providing a default that points to a personal git repository (ie Elson's)?

I can see how having this temporarily to provide the newer registry index.json might have been a good thing but with the new functionality I don't think having a personal git repository listed in the code is OK.

@GeekArthur
Copy link
Contributor Author

If we are adding the capability to add registries, should we still be providing a default that points to a personal git repository (ie Elson's)?

I can see how having this temporarily to provide the newer registry index.json might have been a good thing but with the new functionality I don't think having a personal git repository listed in the code is OK.

The original idea regarding default registry was to let it host on personal git repository for tech preview as it's easier to manage and manipulate for both testing and development. Given we have plan for support devfile 2.0, so one option is we can do the default registry migration with devfile 2.0 support together. I can bring this up in our status meeting and discuss:

  • Where we should host default registry publicly
  • Do we need to create corresponding account or we can use the existing one
  • How long it takes to do the default registry migration

@GeekArthur
Copy link
Contributor Author

@groeges I open an issue to track the default registry migration: #2961

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 22, 2020
@GeekArthur GeekArthur changed the title Draft PR for dynamic registry support Dynamic registry support Apr 22, 2020
…amicRegistrySupport

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 23, 2020
Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 24, 2020
…amicRegistrySupport

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 24, 2020
@GeekArthur
Copy link
Contributor Author

/retest

@GeekArthur
Copy link
Contributor Author

/retest

1 similar comment
@GeekArthur
Copy link
Contributor Author

/retest

Copy link
Member

@dharmit dharmit left a comment

Choose a reason for hiding this comment

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

@GeekArthur Thanks for the discussions and accommodating the requested changes.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 8, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@GeekArthur
Copy link
Contributor Author

/retest

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

11 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 9, 2020

@GeekArthur: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.4-integration-e2e-benchmark 6eb07c2 link /test v4.4-integration-e2e-benchmark

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

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. area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation 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.

Support dynamic devfile registry