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

Automatically use in-cluster Devfile registries if any #6622

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Feb 27, 2023

Co-authored-by: @valaparthvi @feloy @anandrkskd @ritudes

What type of PR is this:
/kind feature

What does this PR do / why we need it:
If connected to a cluster, odo init, odo preference view, odo registry commands will detect any DevfileRegistriesList and ClusterDevfileRegistriesList resources in the cluster, and use the registries listed there.
The same behavior applies to odo dev and odo deploy when there is source code but no devfile.yaml, as they should start by initializing a Devfile.
Per the acceptance criteria, order of registries is as follows:

  1. current namespace registries
  2. cluster registries
  3. local preferences

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

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

  • If connected to a cluster, you can install the Custom Resource Definitions along with the Custom Resources, and see it in action:
$ kubectl apply -f https://raw.githubusercontent.com/devfile/registry-operator/main/config/crd/bases/registry.devfile.io_clusterdevfileregistrieslists.yaml
$ kubectl apply -f https://raw.githubusercontent.com/devfile/registry-operator/main/config/crd/bases/registry.devfile.io_devfileregistrieslists.yaml
# Cluster-scoped registries list
$ kubectl apply -f https://raw.githubusercontent.com/devfile/registry-operator/main/samples/clusterregistrieslist.yaml
# Namespace-scoped registries list
$ kubectl apply -f https://raw.githubusercontent.com/devfile/registry-operator/main/samples/devfileregistrieslist.yaml

# Now you can use any `odo` command
$ odo preference view
  • If not connected, it should still work with the registries stored in the odo preferences, like so:
$ KUBECONFIG=/dev/null odo preference view
Preference parameters:
 PARAMETER           VALUE           
 ConsentTelemetry    true            
 Ephemeral           false (default) 
 PushTimeout                         
 RegistryCacheTime                   
 Timeout                             
 UpdateNotification                  

Devfile registries:
 NAME                    URL                                SECURE 
 Staging                 https://registry.stage.devfile.io  No     
 DefaultDevfileRegistry  https://registry.devfile.io        No     

@rm3l rm3l added the area/registry Issues or PRs related to Devfile registries label Feb 27, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 27, 2023
@openshift-ci
Copy link

openshift-ci bot commented Feb 27, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Feb 27, 2023
@netlify
Copy link

netlify bot commented Feb 27, 2023

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit 4afb1c7
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/64059826ad5b50000897ddaf
😎 Deploy Preview https://deploy-preview-6622--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

OpenShift Unauthenticated Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

NoCluster Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

Unit Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

Validate Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

Kubernetes Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

Windows Tests (OCP) on commit badd717 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 27, 2023

OpenShift Tests on commit badd717 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 28, 2023

Kubernetes Docs Tests on commit 29b5a38 finished with errors.
View logs: TXT HTML

@rm3l rm3l changed the title [WIP] Automatically use in-cluster Devfile registries if any Automatically use in-cluster Devfile registries if any Mar 2, 2023
@rm3l
Copy link
Member Author

rm3l commented Mar 2, 2023

user-guides/quickstart/docs-mdx/nodejs/nodejs_odo_dev_output.mdx
  Expected
      <string>:   (
        	"""
        	... // 9 identical lines
        	•  Waiting for Kubernetes resources  ...
        	⚠  Pod is Pending
      - 	✓  Pod is Running
        	✓  Syncing files into the container [1s]
        	✓  Building your application in container (command: install) [1s]
        	... // 9 identical lines
        	[Ctrl+c] - Exit and delete resources from the cluster
        	[p] - Manually apply local changes to the application on the cluster
      + 	✓  Pod is Running
        	```
        	"""
        )
      
  to be empty
  In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:104

Addressed in #6545

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

@openshift-ci
Copy link

openshift-ci bot commented Mar 2, 2023

@rm3l: Overrode contexts on behalf of rm3l: Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

In response to this:

user-guides/quickstart/docs-mdx/nodejs/nodejs_odo_dev_output.mdx
 Expected
     <string>:   (
       	"""
       	... // 9 identical lines
       	•  Waiting for Kubernetes resources  ...
       	⚠  Pod is Pending
     - 	✓  Pod is Running
       	✓  Syncing files into the container [1s]
       	✓  Building your application in container (command: install) [1s]
       	... // 9 identical lines
       	[Ctrl+c] - Exit and delete resources from the cluster
       	[p] - Manually apply local changes to the application on the cluster
     + 	✓  Pod is Running
       	```
       	"""
       )
     
 to be empty
 In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:104

Addressed in #6545

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

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.

@rm3l rm3l changed the title Automatically use in-cluster Devfile registries if any [WIP] Automatically use in-cluster Devfile registries if any Mar 2, 2023
@rm3l
Copy link
Member Author

rm3l commented Mar 2, 2023

Still to do:

  • odo init interactive mode: it does detect the in-cluster registry, but was not able to use it
  • odo init non-interactive mode: specifying an in-cluster registry as --devfile-registry returns an error
  • odo registry --details --defile-registry: errors out if we specify an in-cluster registry

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Mar 3, 2023
@rm3l rm3l force-pushed the 5128-odo-should-automatically-use-cluster-registry-if-cluster-has-one-deployed branch from dd817cd to 3ef5a17 Compare March 3, 2023 15:12
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Mar 3, 2023
@rm3l
Copy link
Member Author

rm3l commented Mar 3, 2023

Rebased and force-pushed to fix the conflicts reported.

@rm3l rm3l changed the title [WIP] Automatically use in-cluster Devfile registries if any Automatically use in-cluster Devfile registries if any Mar 3, 2023
@rm3l rm3l marked this pull request as ready for review March 3, 2023 15:51
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 3, 2023
@openshift-ci openshift-ci bot requested review from feloy and rnapoles-rh March 3, 2023 15:52
@rm3l rm3l requested review from valaparthvi and ritudes and removed request for rnapoles-rh March 3, 2023 15:55
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 6, 2023
rm3l and others added 19 commits March 6, 2023 08:36
This is yet to be implemented.

Co-authored-by: Philippe Martin <[email protected]>
This should ideally not happen if the registry operator
 is installed in the cluster (because it validates the
 URL to make sure it is reachable), but you never know ;-)
Registries might be found in the cluster.
Signed-off-by: Parthvi Vala <[email protected]>

Co-authored-by: Armel Soro <[email protected]>
…e cluster or if there are cluster-wide registry lists
CRs are now dynamically created and applied from the tests
@feloy feloy force-pushed the 5128-odo-should-automatically-use-cluster-registry-if-cluster-has-one-deployed branch from 85b9a44 to 4afb1c7 Compare March 6, 2023 07:37
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 6, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy
Copy link
Contributor

feloy commented Mar 6, 2023

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
/override windows-integration-test/Windows-test
Flaky tests

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 6, 2023
@openshift-ci
Copy link

openshift-ci bot commented Mar 6, 2023

@feloy: Overrode contexts on behalf of feloy: Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests, windows-integration-test/Windows-test

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
/override windows-integration-test/Windows-test
Flaky tests

/lgtm

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.

@openshift-merge-robot openshift-merge-robot merged commit 4fb7308 into redhat-developer:main Mar 6, 2023
@rm3l rm3l deleted the 5128-odo-should-automatically-use-cluster-registry-if-cluster-has-one-deployed branch March 6, 2023 08:31
anandrkskd added a commit to anandrkskd/odo that referenced this pull request Mar 7, 2023
…per#6622)

* Add kubeclient as dependency of the registry client

Co-authored-by: Philippe Martin <[email protected]>

* Add GetRegistryList method to the kube client interface

This is yet to be implemented.

Co-authored-by: Philippe Martin <[email protected]>

* Implement GetRegistryList

* adding test if devfileRegistryListCR is present in cluster

Signed-off-by: anandrkskd <[email protected]>

* Unit tests (to be continued)

* Add unit test cases against kclient#GetRegistryList()

Co-authored-by: Philippe Martin <[email protected]>

* Ignore in-cluster registries with an empty URL

This should ideally not happen if the registry operator
 is installed in the cluster (because it validates the
 URL to make sure it is reachable), but you never know ;-)

* Update error message when trying to remove registry

Registries might be found in the cluster.

* Pass isSecure value to the registry handler

* Make it possible to use in-cluster registries when calling 'odo registry --details'

* Remove unused 'preferenceClient' from registry.getRegistryStacks

* Handle in-cluster registries in 'odo init' non-interactive mode

* Handle in-cluster registries in 'odo init' interactive mode

* Add integration test for odo init --devfile-registry

Signed-off-by: Parthvi Vala <[email protected]>

* Use proxy when available

Signed-off-by: Parthvi Vala <[email protected]>

Co-authored-by: Armel Soro <[email protected]>

* Make sure tests work even if the registry operator is installed in the cluster or if there are cluster-wide registry lists

* Add tests for 'odo init' interactive mode

* Remove useless CR file

CRs are now dynamically created and applied from the tests

* fixup! Add tests for 'odo init' interactive mode

---------

Signed-off-by: anandrkskd <[email protected]>
Signed-off-by: Parthvi Vala <[email protected]>
Co-authored-by: Philippe Martin <[email protected]>
Co-authored-by: anandrkskd <[email protected]>
Co-authored-by: Parthvi Vala <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/registry Issues or PRs related to Devfile registries 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.

odo should automatically use cluster registry if cluster has one deployed
5 participants