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

odo catalog list components fails if registry is not set up properly #3297

Closed
GeekArthur opened this issue Jun 3, 2020 · 7 comments · Fixed by #3341
Closed

odo catalog list components fails if registry is not set up properly #3297

GeekArthur opened this issue Jun 3, 2020 · 7 comments · Fixed by #3341
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. area/registry Issues or PRs related to Devfile registries kind/bug Categorizes issue or PR as related to a bug.
Projects

Comments

@GeekArthur
Copy link
Contributor

/kind bug

What versions of software are you using?

Operating System:
MacOS

Output of odo version:
Latest master

How did you run odo exactly?

odo registry add Fake https://fake                                                                                      ✔  14:29:21 
New registry successfully added
odo catalog list components                                                                                             ✔  14:29:35 
 ⚠  Registry  is not set up properly with error: <nil>
 ✗  Get "https://fake/devfiles/index.json": dial tcp: lookup fake: no such host

Actual behavior

odo catalog list components fails if registry is not set up properly.

Expected behavior

We should use warning instead of error, because we need to support the case where user has multiple registries but only part of the registries is not set up properly, for that case we shouldn't block user from listing registry that is set up properly and listing s2i components.

Any logs, error output, etc?

N/A

Special Notes:

This regression starts from this PR #3112 gets merged, we need to do the following things to fix the issue:

  1. Use warning instead of error for registry that is not set up properly
  2. Display the registry name properly when throwing warning
  3. Add tests to cover the case to avoid regression in the future
@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 3, 2020
@kadel
Copy link
Member

kadel commented Jun 4, 2020

/area registry

@openshift-ci-robot openshift-ci-robot added the area/registry Issues or PRs related to Devfile registries label Jun 4, 2020
@johnmcollier
Copy link
Member

I think there's a couple things that need to be done as part of this:

  1. odo catalog list shouldn't error out if one of the registries is invalid or cannot be reached. A warning should be displayed instead
  2. odo registry add should not allow users to add invalid registries.

@GeekArthur
Copy link
Contributor Author

  1. odo catalog list shouldn't error out if one of the registries is invalid or cannot be reached. A warning should be displayed instead

Do you mean odo registry list instead of odo catalog list? Because odo catalog list issue is already under this issue description. If you mean odo registry list, I think that error is unlikely as odo registry add already validates for the error so that the error won't be moved to odo registry list

odo registry add should not allow users to add invalid registries.

We already have that validation in existing code base

@johnmcollier
Copy link
Member

johnmcollier commented Jun 5, 2020

Do you mean odo registry list instead of odo catalog list? Because odo catalog list issue is already under this issue description. If you mean odo registry list, I think that error is unlikely as odo registry add already validates for the error so that the error won't be moved to odo registry list

Yes, I mean odo catalog list

We already have that validation in existing code base

We only validate that what's being given to odo registry add is a url. We don't currently validate that the URL points to something or that the URL + devfiles/index.json points to something. I think an error should occur for the following (which it does not do today)

Johns-MacBook-Pro-3:odo johncollier$ odo registry add Fake https://Fake
New registry successfully added

@GeekArthur
Copy link
Contributor Author

We already have that validation in existing code base

We only validate that what's being given to odo registry add is a url. We don't currently validate that the URL points to something or that the URL + devfiles/index.json points to something. I think an error should occur for the following (which it does not do today)

Johns-MacBook-Pro-3:odo johncollier$ odo registry add Fake https://Fake
New registry successfully added

Yes, that behavior is expected and is by design. The design is that we divide the validation into two stages:
Early validation stage: Only validate URL basic format, happens when user adds registry URL
Validation stage: Validate URL accessibility, happens when user uses registry URL

That design is based on yum-config-manager, design proposal with details under this thread: #2940 (comment)

@GeekArthur
Copy link
Contributor Author

/assign

@elsony elsony added this to For consideration in Sprint 185 via automation Jun 11, 2020
@elsony
Copy link

elsony commented Jun 11, 2020

/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 Jun 11, 2020
@elsony elsony moved this from For consideration to In progress in Sprint 185 Jun 11, 2020
@elsony elsony moved this from In progress to For review in Sprint 185 Jun 16, 2020
Sprint 185 automation moved this from For review to Done Jun 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. area/registry Issues or PRs related to Devfile registries kind/bug Categorizes issue or PR as related to a bug.
Projects
No open projects
Sprint 185
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants