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.

Sign up for GitHub

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

Jump to bottom

odo deploy command #5228

Merged

openshift-merge-robot merged 12 commits into redhat-developer:main from feloy:feature-5151/odo-deploy

Nov 24, 2021

Contributor

feloy commented Nov 12, 2021 •

edited

/kind feature

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #5151

odo deploy should trigger a default deploy command (group.kind: deploy.) and execute it based on how it is defined
during push, do not deploy components referenced in a devfile command

PR acceptance criteria:

Unit test
Integration test
Documentation
I have read the test guidelines

How to test changes / Special notes to the reviewer:

openshift-ci bot added do-not-merge/work-in-progress kind/feature labels

openshift-ci bot requested review from mohammedzee1000 and rnapoles-rh

November 12, 2021 10:12

Contributor Author

feloy commented Nov 12, 2021

Updating devfile/api to the HEAD of main is not possible due to devfile/api#675, I updated to commit before "add annotation to container spec and add validation to it (#661)" instead

feloy force-pushed the feature-5151/odo-deploy branch from 9671ae9 to f9f95ca Compare

November 12, 2021 13:19

netlify bot commented Nov 15, 2021 •

edited

✔️ Deploy Preview for odo-docusaurus-preview ready!

🔨 Explore the source changes: 00d67eb

🔍 Inspect the deploy log: https://app.netlify.com/sites/odo-docusaurus-preview/deploys/619e064c88d4720007e9242a

😎 Browse the preview: https://deploy-preview-5228--odo-docusaurus-preview.netlify.app

feloy force-pushed the feature-5151/odo-deploy branch from d25161a to 558111b Compare

November 15, 2021 13:21

feloy requested review from dharmit and valaparthvi and removed request for mohammedzee1000 and rnapoles-rh

November 15, 2021 14:10

feloy force-pushed the feature-5151/odo-deploy branch 3 times, most recently from 042d170 to 1d82d29 Compare

November 17, 2021 06:59

feloy changed the title ~~[wip] odo deploy command~~ odo deploy command

openshift-ci bot removed the do-not-merge/work-in-progress label

Contributor Author

feloy commented Nov 17, 2021

Updating devfile/api to the HEAD of main is not possible due to devfile/api#675, I updated to commit before "add annotation to container spec and add validation to it (#661)" instead

This problem has been fixed in devfile/api

feloy force-pushed the feature-5151/odo-deploy branch from 1d82d29 to 3bfc49a Compare

November 17, 2021 11:03

dharmit suggested changes

View reviewed changes

Member

dharmit left a comment

Only docs review.

docs/website/docs/command-reference/deploy.md Outdated Show resolved Hide resolved

docs/website/docs/command-reference/deploy.md

+              odo can be used to deploy components in a similar manner they would be deployed by a CI/CD system,
+              by first building the images of the containers to deploy, then by deploying the Kubernetes resources
+              necessary to deploy the components.

Member

dharmit Nov 23, 2021

Should we clarify here that odo deploy works only for devfiles having schemaVersion 2.2.0 or above? Or is that not accurate and odo deploy would work if it finds the required key-values even if the devfile schemaVersion was below 2.2.0?

Contributor Author

feloy Nov 23, 2021

It is accurate. If the devfile revision is lower than 2.2.0, the devfile library will not validate the file:

 ✗  invalid devfile schema. errors :
- commands.6.composite.group.kind: commands.6.composite.group.kind must be one of the following: "build", "run", "test", "debug"

docs/website/docs/command-reference/deploy.md Outdated Show resolved Hide resolved

docs/website/docs/command-reference/deploy.md

+              - a command referencing an `Image` component that, when applied, will build the image of the container to deploy,
+              - a command referencing a `Kubernetes` component that, when applied, will create Kubernetes resources in the cluster.
+              With the following example `devfile.yaml` file, a container image will be built by using the `Dockerfile` present in the directory,

Member

dharmit Nov 23, 2021

IMO, having a fully functional devfile would help here. That way, we can address the point of odo deploy support for devfile 2.2.0 or higher (if we need to.)

Contributor Author

feloy Nov 23, 2021 •

edited

I would prefer not to add a complete devfile. There are a lot of mandatory parts in the devfile that won't be related to this subject, that will make a very long file and will be difficult for the reader to focus on the important parts
I added the schemaVersion to highlight that it should be >= 2.2.0

Member

dharmit Nov 23, 2021

OK. Makes sense. If you think adding a link to a fully functional devfile would make sense, then you could add it. Otherwise, it's OK. We can revisit the discussion if anyone asks for such a devfile. 😄

docs/website/docs/command-reference/deploy.md Outdated Show resolved Hide resolved

openshift-ci bot assigned dharmit

dharmit suggested changes

View reviewed changes

Member

dharmit left a comment

Reviewed only the integration test to figure out how to and what to test for odo deploy.

tests/integration/devfile/cmd_devfile_deploy_test.go Show resolved Hide resolved

feloy added 8 commits

November 23, 2021 08:55


          Update devfile library

7bd39be


          deploy cmd

613ca0a


          Init "odo deploy" command reference

569d1cf


          Integration test

ab11adc


          Fix odo deploy

91a2219


          Filter components to apply during odo push

5834e8f


          Output

c9ef2eb


          Remove redondant level 1 title

5697d6d


          Doc review

da5939f

feloy force-pushed the feature-5151/odo-deploy branch from 64cc361 to da5939f Compare

November 23, 2021 08:14

Contributor Author

feloy commented Nov 23, 2021

@dharmit thanks for the review on doc, I made the changes

dharmit suggested changes

View reviewed changes

Member

dharmit left a comment

Still reviewing the code. But it's a lot of files, so not waiting till I'm done.

pkg/odo/cli/deploy/deploy.go Outdated

+              // RecommendedCommandName is the recommended command name
+              const RecommendedCommandName = "deploy"
+              // LoginOptions encapsulates the options for the odo command

Member

dharmit Nov 23, 2021

s/LoginOptions/DeployOptions/g on this file, maybe?

pkg/devfile/adapters/kubernetes/component/adapter.go Show resolved Hide resolved

pkg/devfile/components.go Outdated

+              	return false, nil
+              }
+              // AddKubernetesComponentToDevfile adds service definition to devfile as an inlined Kubernetes component

Member

dharmit Nov 23, 2021

It's no longer just service definition. You moved the code but forgot to update the comment.

pkg/devfile/components.go Outdated

+              	return addKubernetesComponent(crd, name, componentContext, devfile, devfilefs.DefaultFs{})
+              }
+              // AddKubernetesComponent adds the crd information to a separate file and adds the uri information to a devfile component

Member

dharmit Nov 23, 2021

Suggested change

      
            // AddKubernetesComponent adds the crd information to a separate file and adds the uri information to a devfile component
          
            // addKubernetesComponent adds the crd information to a separate file and adds the uri information to a devfile component

pkg/devfile/components.go Outdated

+              	return devfileData
+              }
+              // GetComponentsToPush returns the list of Kubernetes components to push,

Member

dharmit Nov 23, 2021

Suggested change

      
            // GetComponentsToPush returns the list of Kubernetes components to push,
          
            // GetKubernetesComponentsToPush returns the list of Kubernetes components to push,

pkg/devfile/components.go Outdated Show resolved Hide resolved


          Use server-side apply to apply Kubernetes components

1a9ccda

feloy force-pushed the feature-5151/odo-deploy branch from d7d8b00 to 1a9ccda Compare

November 23, 2021 11:50


          Review

615cb06

feloy requested a review from dharmit

November 23, 2021 15:15

Contributor Author

feloy commented Nov 23, 2021

/test v4.9-integration-e2e

[Fail] odo devfile registry command tests [BeforeEach] When executing registry commands with the registry is not present Should successfully add the registry

Member

kadel commented Nov 23, 2021

/approve

openshift-ci bot commented Nov 23, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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:

~~OWNERS~~ [kadel]

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

openshift-ci bot added the approved label


          Replace PROJECT_ROOT with PROJECTS_ROOT

00d67eb

sonarcloud bot commented Nov 24, 2021

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
3.2% Duplication

Contributor Author

feloy commented Nov 24, 2021

/test psi-kubernetes-integration-e2e

[Fail] odo link command tests for OperatorHub Operators are installed in the cluster when a component and a service are deployed when a storage is added and deployed when a link between the component and the service is created [It] should run odo push successfully

Contributor Author

feloy commented Nov 24, 2021

/test psi-kubernetes-integration-e2e

[Fail] odo link command tests for OperatorHub when one component is deployed when another component is deployed [It] should link the two components successfully without service binding operator

dharmit reviewed

View reviewed changes

pkg/devfile/adapters/common/command_simple.go

@@ @@ -10,8 +10,8 @@ import ( @@
               	"github.com/pkg/errors"
               )
-              // simpleCommand is a command implementation for non-composite commands
-              type simpleCommand struct {
+              // execCommand is a command implementation for non-composite commands

Member

dharmit Nov 24, 2021

Should the file be renamed to command_exec.go as well?

Member

dharmit commented Nov 24, 2021

/lgtm

#5228 (comment) is not a blocker and can be done separately. However, in case you change it in this PR, can you please ask Tomas to lgtm it again if I'm not around tomorrow?

openshift-ci bot added the lgtm label

openshift-merge-robot merged commit ac478e9 into redhat-developer:main

anandrkskd pushed a commit to anandrkskd/odo that referenced this pull request


          odo deploy command (redhat-developer#5228)

bbaa53e

* Update devfile library

* deploy cmd

* Init "odo deploy" command reference

* Integration test

* Fix odo deploy

* Filter components to apply during odo push

* Output

* Remove redondant level 1 title

* Doc review

* Use server-side apply to apply Kubernetes components

* Review

* Replace PROJECT_ROOT with PROJECTS_ROOT

kadel mentioned this pull request

Support image component and deploy devfile command / initial odo deploy implementation #5109

Closed

2 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment