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.

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

Update odo readme and docs for the experimental mode #2869

Merged
merged 2 commits into from
Apr 22, 2020
Merged

Update odo readme and docs for the experimental mode #2869

merged 2 commits into from
Apr 22, 2020

Conversation

maysunfaisal
Copy link
Contributor

Signed-off-by: Maysun J Faisal maysun.j.faisal@ibm.com

What type of PR is this?
/kind documentation
[skip ci]

What does does this PR do / why we need it:
This PR

  • updates the readme on the main repo and adds a link to the odo experimental doc
  • updates the odo experimental doc on how to use the various odo commands for experimental mode and also adds a link to the stack creator guide document

Which issue(s) this PR fixes:
N/A

How to test changes / Special notes to the reviewer:
N/A

@maysunfaisal
[skip ci] Update Readme and docs for odo experimental
c939b4d 
Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>
@maysunfaisal
[skip ci] Update experimental doc 2
34e103a 
Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>
dharmit
dharmit suggested changes Apr 13, 2020
View reviewed changes
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.

Besides the changes requested in this PR, I found a few issues that were not related to this PR. So, I've opened #2872 to fix those.

docs/dev/experimental-mode.adoc
```sh
$ odo push --devfile ./devfile.yaml
Error: unknown flag: --devfile
##### Creating a devfile component:
Copy link
Member

Choose a reason for hiding this comment

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

IMO, this section should mention that odo create will create a devfile.yaml in pwd.

docs/dev/experimental-mode.adoc

```
$ odo url create myspring --port 8080 --host <ingress domain>
✓ URL created for component: myspring, cluster host: <ingress domain>
Copy link
Member

Choose a reason for hiding this comment

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

We need a more concrete example here. What's mentioned here is good but an example with a real ingress domain would serve better. It would be awesome if example is based on CRC since it might be used quite extensively by enterprise devs using odo or IDE plugins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, i will put an example of ingress domain with crc

docs/dev/experimental-mode.adoc
By default, `odo push` executes `devbuild` and `devrun` devfile commands. However, custom commands can be also be provided to `odo push` via flags `--build-command` & `--run-command`. `odo push` also provides a `--devfile` flag to execute push with a `devfile.yaml` in a custom path. These flags are only available in the experimental mode.

##### Deleting a devfile component:
Delete a devfile component with `odo delete`. This deletes all the Kubernetes resources created during `odo push`. Use the `-all` flag to delete the Kubernetes resources and the local config at `.odo/env/env.yaml`
Copy link
Member

Choose a reason for hiding this comment

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

Can we change this a little bit? I would like to request for:

This deletes all the Kubernetes resources for the devfile component which were created during odo push.

Copy link
Member

Choose a reason for hiding this comment

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

Also I think it's --all flag instead of -all.

docs/dev/experimental-mode.adoc
✓ Deleting devfile component myspring [339ms]
✓ Successfully deleted component

```
Copy link
Member

Choose a reason for hiding this comment

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

Can we please also add an example of odo delete --all here?

elsony
elsony suggested changes Apr 13, 2020
View reviewed changes
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.

Can we also add info for odo watch?

docs/dev/experimental-mode.adoc
$ odo url create myspring --port 8080 --host <ingress domain>
✓ URL created for component: myspring, cluster host: <ingress domain>

To create URL on the OpenShift Cluster, please use `odo push`
Copy link

Choose a reason for hiding this comment

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

Is this OpenShift specific? Should we say something like the cluster in Kubernetes instead of OpenShift Cluster?

Copy link
Member

Choose a reason for hiding this comment

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

We should make it just cluster, as Tomas suggested earlier.

johnmcollier
johnmcollier suggested changes Apr 13, 2020
View reviewed changes
docs/dev/experimental-mode.adoc
--vmodule moduleSpec Comma-separated list of pattern=N settings for file-filtered logging

##### Listing devfile components:
When the experimental mode is enabled, `odo catalog list components` will list the devfile components alongside the OpenShift components.
Copy link
Member

Choose a reason for hiding this comment

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

We should mention that the OpenShift/s2i components only show up on OpenShift clusters, and only the devfile components will be listed on Kubernetes clusters.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe also highlight that odo catalog list components --all will show all devfile components between both devfile registries (even non-supported ones)

docs/dev/experimental-mode.adoc
To push either of the url sub-command changes, execute `odo push`.

##### Pushing to a devfile component:
When the experimental mode is enabled, `odo push` creates a pod with all the devfile component containers and volumes. Push syncs the local project on disk to the pod container where devfile component has `mountSources: true`. The optional build and the mandatory run commands are executed in order.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: It should be When the experimental mode is enabled and a valid devfile exists because if no devfile exists and experimental mode is enabled, it will fall back to s2i (provided you're on an openshift cluster)

docs/dev/experimental-mode.adoc

#### Developing a devfile:

The devfile registry can be accessed link:https://github.com/elsony/devfile-registry[here].
Copy link
Member

Choose a reason for hiding this comment

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

We should highlight that we also pull devfiles from https://github.com/eclipse/che-devfile-registry, even if none of them are considered supported by odo at the moment

@kadel
Copy link
Member

kadel commented Apr 15, 2020

Could we move docs/dev/experimental-mode.adoc to docs/experimental-mode.adoc?

  • the docs/dev should be documentation for developers working on odo "us"
  • rest of the docs/ folder should be documentation for odo users

@maysunfaisal
Copy link
Contributor Author

@kadel yeah i can do that with this PR!

@girishramnani
Copy link
Contributor 

rpc error: code = 2 desc = oci runtime error: exec failed: container "d4b0151d729f20632bcb9305d8c17208fd87fc9d8e72fb75ed6ff1dcb0afc10b" does not exist

2020/04/13 04:00:52 error: could not wait for pod 'release-latest': it is no longer present on the cluster (usually a result of a race or resource pressure. re-running the job should help)
skipped 5 lines unfold_more

platform error

@girishramnani
Copy link
Contributor

/retest

@cdrage
Copy link
Member

cdrage commented Apr 22, 2020

LGTM.

To be honest, I'm going to merge this in now and we iterate over it later so we can create some separate devfile documentation as well as move the experimental mode document somewhere else.

Mergin'

@cdrage cdrage added approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow. labels Apr 22, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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-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.

@maysunfaisal
Copy link
Contributor Author

@cdrage thx, I was busy with the other PR and didnt' have time to update this. But I can open a follow up with all the suggestions from this PR!

@openshift-bot
Copy link

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 36614ec into redhat-developer:master Apr 22, 2020
@maysunfaisal maysunfaisal mentioned this pull request May 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elsony elsony elsony requested changes

@johnmcollier johnmcollier johnmcollier requested changes

@dharmit dharmit dharmit requested changes

@girishramnani girishramnani Awaiting requested review from girishramnani

Assignees

@dharmit dharmit

@johnmcollier johnmcollier

@elsony elsony

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@maysunfaisal @kadel @girishramnani @cdrage @openshift-ci-robot @openshift-bot @dharmit @johnmcollier @elsony @openshift-merge-robot