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

Document how odo translates container component into Kubernetes resources #6145

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Sep 20, 2022

What type of PR is this:
/kind documentation
/area documentation

What does this PR do / why we need it:
This adds a new 'Architecture' section under 'Developer Reference' that documents in detail how odo works, for users to understand what odo does in their clusters.

Let me know if I've missed anything, or if you think the structure or style should be different.

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

PR acceptance criteria:

How to test changes / Special notes to the reviewer:
Visit https://deploy-preview-6145--odo-docusaurus-preview.netlify.app/docs/development/architecture/how-odo-works

@netlify
Copy link

netlify bot commented Sep 20, 2022

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit a72ed69
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/632c58518ade810008236642
😎 Deploy Preview https://deploy-preview-6145--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.

@openshift-ci openshift-ci bot added kind/documentation area/documentation Issues or PRs related to documentation or the 'odo.dev' website labels Sep 20, 2022
@openshift-ci openshift-ci bot requested review from cdrage and feloy September 20, 2022 16:00
@rm3l rm3l force-pushed the 5174-document-how-odo-translates-container-component-to-deployment branch from 76a774d to eab60a7 Compare September 20, 2022 16:06
@rm3l rm3l requested a review from kadel September 20, 2022 16:06
@odo-robot
Copy link

odo-robot bot commented Sep 20, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 20, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 20, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 20, 2022

OpenShift Tests on commit 7ac889e finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 20, 2022

Windows Tests (OCP) on commit 7ac889e finished successfully.
View logs: TXT HTML

@rm3l rm3l force-pushed the 5174-document-how-odo-translates-container-component-to-deployment branch from 7d9c44e to fdc1dfd Compare September 21, 2022 08:00
Copy link
Contributor

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

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

This is an excellent article, great work, Armel!

I have a few nitpicks that I have suggested. Apart from them, can you ensure you add a period '.' to all the sentences in the tables? I have pointed a few and but not all.

running containers.
Note that synchronization and push to the cluster can also be triggered on demand by pressing `p` at any time.
See [this page](../../command-reference/dev#applying-local-changes-to-the-application-on-the-cluster) for more details.
6. `odo` optionally restarts the running application (if the command is not marked as `hotReloadCapable` in the Devfile).
Copy link
Contributor

Choose a reason for hiding this comment

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

This point is not very clear to me. How and when does it restart a running application? Or if this point is a part of point 5, then it makes sense, but then I would merge them both.

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding more clarification on this.


By default, `odo` adds the following labels to the Service:

| Key | Description | Example Value |
Copy link
Contributor

Choose a reason for hiding this comment

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

These labels are the same as Deployments, no? Perhaps you don't need to redefine them and simply point to that section?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, they are the same. I'll list them under a dedicated section.

@dharmit
Copy link
Member

dharmit commented Sep 22, 2022

Amazing work, @rm3l! Thanks for putting this together.

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharmit

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Sep 22, 2022
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

Great work!


If the Devfile contains any [`Kubernetes` components](../../development/devfile#components) not referenced by any [`apply` command](https://devfile.io/docs/2.2.0-alpha/adding-apply-commands),
`odo dev` automatically applies them.
This is the mechanism used by [`odo add binding`](../../command-reference/add-binding) to have `ServiceBinding` resources created by `odo dev`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the mechanism used by odo add binding to have ServiceBinding resources created by odo dev.

You are saying too much, or not enough: you could point to a specific paragraph where this is explained in more details, or remove this part, as it is not clearly understandable as is.

Copy link
Member Author

@rm3l rm3l Sep 22, 2022

Choose a reason for hiding this comment

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

I was hesitant to add a dedicated section about how odo handles Devfile kubernetes components. But you're right, it is currently a bit confusing. Removing this sentence for now - this could be the scope of a separate PR.

This is the mechanism used by [`odo add binding`](../../command-reference/add-binding) to have `ServiceBinding` resources created by `odo dev`.
3. Once the resources are ready, `odo` executes any `build` (optional) and `run` (or `debug`) commands defined in the Devfile into the right containers.
4. It reacts to events occurring in the cluster and that might affect the resources managed.
5. Unless told otherwise (when running `odo dev --no-watch`), `odo` watches for local changes
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move the execption (Unless told otherwise...) later in the sentence, to start with the important thing: odo watches for local changes


</details>

#### Volumes
Copy link
Contributor

Choose a reason for hiding this comment

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

I thin kyou should also tell somewhere in the page that odo can also create PersistentVolumeClaim resources when Volume are defined in Devfile or when Ephemeral is false.

Co-authored-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Philippe Martin <phmartin@redhat.com>
@sonarcloud
Copy link

sonarcloud bot commented Sep 22, 2022

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 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@feloy
Copy link
Contributor

feloy commented Sep 22, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 22, 2022
@rm3l
Copy link
Member Author

rm3l commented Sep 22, 2022

• [FAILED] [395.351 seconds]
odo add binding command tests
/go/odo_1/tests/integration/cmd_add_binding_test.go:13
  when the component is bootstrapped
  /go/odo_1/tests/integration/cmd_add_binding_test.go:153
    when adding a binding (current namespace)
    /go/odo_1/tests/integration/cmd_add_binding_test.go:219
      when odo dev is run [BeforeEach]
      /go/odo_1/tests/integration/cmd_add_binding_test.go:260
        when odo dev command is stopped
        /go/odo_1/tests/integration/cmd_add_binding_test.go:265
          should have successfully delete the binding
          /go/odo_1/tests/integration/cmd_add_binding_test.go:271
...
 ✓  Pod is Running
      Error occurred on Push - watch command was unable to push component: some servicebindings are not injected
      
      Error occurred on Push - watch command was unable to push component: some servicebindings are not injected

/override OpenShift-Integration-tests/OpenShift-Integration-tests

This is a doc-only PR.

@openshift-ci
Copy link

openshift-ci bot commented Sep 22, 2022

@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:

• [FAILED] [395.351 seconds]
odo add binding command tests
/go/odo_1/tests/integration/cmd_add_binding_test.go:13
 when the component is bootstrapped
 /go/odo_1/tests/integration/cmd_add_binding_test.go:153
   when adding a binding (current namespace)
   /go/odo_1/tests/integration/cmd_add_binding_test.go:219
     when odo dev is run [BeforeEach]
     /go/odo_1/tests/integration/cmd_add_binding_test.go:260
       when odo dev command is stopped
       /go/odo_1/tests/integration/cmd_add_binding_test.go:265
         should have successfully delete the binding
         /go/odo_1/tests/integration/cmd_add_binding_test.go:271
...
✓  Pod is Running
     Error occurred on Push - watch command was unable to push component: some servicebindings are not injected
     
     Error occurred on Push - watch command was unable to push component: some servicebindings are not injected

/override OpenShift-Integration-tests/OpenShift-Integration-tests

This is a doc-only 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.

@openshift-merge-robot openshift-merge-robot merged commit 1c0ec66 into redhat-developer:main Sep 22, 2022
kadel pushed a commit to kadel/odo that referenced this pull request Oct 3, 2022
…esources (redhat-developer#6145)

* Document how `odo` translates `container` component into Kubernetes resources

* Use different background color for highlighted code line

The default one makes the highlighted line hardly readable

* fixup! Document how `odo` translates `container` component into Kubernetes resources

* Apply code review suggestions

Co-authored-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Philippe Martin <phmartin@redhat.com>

Co-authored-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Philippe Martin <phmartin@redhat.com>
@rm3l rm3l deleted the 5174-document-how-odo-translates-container-component-to-deployment branch December 1, 2022 16:34
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/documentation Issues or PRs related to documentation or the 'odo.dev' website 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.

Document how odo translates container component to Deployment
5 participants