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

Adds basic workflow #99

Merged
merged 1 commit into from
May 14, 2019
Merged

Conversation

maximilien
Copy link
Contributor

  • deploy a function (list, update, curl, describe, and delete)

Completes more of issue #51

@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 9, 2019
@sixolet
Copy link
Contributor

sixolet commented May 9, 2019

/retest

Copy link
Contributor

@sixolet sixolet left a comment

Choose a reason for hiding this comment

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

Moslty nits

In this basic worflow we show the CRUD (create, read, update, delete) operations on a service. We use a well known [simple Hello World service](https://github.com/knative/docs/tree/master/docs/serving/samples/hello-world/helloworld-go) that reads the environment variable `TARGET` and prints it as output.

1. Create a service from image in `default` namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Your markdown renders the numbered list as 1. 1. 1, rather than numbering properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, was afraid of that. I'll use bullet points. Thx.

## Basic (get your feet wet)

In this basic worflow we show the CRUD (create, read, update, delete) operations on a service. We use a well known [simple Hello World service](https://github.com/knative/docs/tree/master/docs/serving/samples/hello-world/helloworld-go) that reads the environment variable `TARGET` and prints it as output.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/basic//

The purpose of this section of the Kn documentation is to list what we believe are common workflows or use-cases for the Knative CLI. Obviously, as we release the CLI in the wild and get users in production, we might discover novel workflows. As we do we will improve this document.

## Basic (get your feet wet)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Basic/Introductory/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

# Workflows

The purpose of this section of the Kn documentation is to list what we believe are common workflows or use-cases for the Knative CLI. Obviously, as we release the CLI in the wild and get users in production, we might discover novel workflows. As we do we will improve this document.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/what we believe are//

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx

# Workflows

The purpose of this section of the Kn documentation is to list what we believe are common workflows or use-cases for the Knative CLI. Obviously, as we release the CLI in the wild and get users in production, we might discover novel workflows. As we do we will improve this document.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Obviously....end of paragraph/This is a live document, meant to be updated as we learn more about good ways to use kn/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More concise. I like it. Thx


```bash
$ kn service list
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we agreed that this will be knative service get (same semantics as in kubectl)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK let me see if in latest code I can do this. Otherwise, I'll omit until ready.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rhuss the service get issue #48 is implemented in #90 but seems that it's on hold waiting on feedback and perhaps some code changes.

I added a comment to include a section for * Get service when that's ready.

```bash
$ kn service describe hello
apiVersion: knative.dev/v1alpha1
Copy link
Contributor

Choose a reason for hiding this comment

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

The output of kn service describe will be always human readable, not yaml. Its kn service get hello -o yaml which returns a machine readable representation (with the same -o support as for kubectl as this is reused from Kubernete's cli-runtime).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd agree with that. However, current implementation yielded YAML. I simply cut and pasted and edited to redact.

I'll try with latest code and if not ready I'll remove (for now).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rhuss no issue on this. Want to open it so we can see any feedback and I can address both.

```bash
$ kn service delete hello3
$ Deleted hello3 in default namespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for nit-picking on high-level (pardonnez moi), but I would use here as a response:

Service 'hello3' in namespace 'default' deleted

(i.e. to make clear what is used provided data and mentioning early that's about a service)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that. I'll submit a PR. Again output is as is in current code.

```

You can then issue the same `$ kn service list` command to verify that the service was deleted.
Copy link
Contributor

Choose a reason for hiding this comment

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

kn service get

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup yup.


```bash
curl '-sS' '-H' 'Host: hello.default.example.com' 'http://xxx.xx.xxx.xx:80'

Choose a reason for hiding this comment

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

let's remove :80 from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, fair enough.

```

Where `http://xxx.xx.xxx.xx:80` is your Knative installation ingress.

Choose a reason for hiding this comment

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

and here

@cppforlife
Copy link

i think it may also be useful to include output for all the commands ran.

@maximilien
Copy link
Contributor Author

i think it may also be useful to include output for all the commands ran.

That's what I did. Some have no output (like service create) but that will change. There is an issue on that.

@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 10, 2019
@cppforlife
Copy link

/lgtm
/approve

service get will be added when it's merged.

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cppforlife, maximilien

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2019
@knative-prow-robot knative-prow-robot merged commit 5703254 into knative:master May 14, 2019
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
maximilien added a commit to maximilien/client that referenced this pull request May 14, 2019
@maximilien maximilien deleted the patch-1 branch May 14, 2019 19:31
coryrc pushed a commit to coryrc/client that referenced this pull request May 14, 2020
dsimansk added a commit to dsimansk/client that referenced this pull request Apr 27, 2023
* Update spec file to release 1.6

* Cherry-pick test fixes, bump to client v1.6.1

---------

Co-authored-by: David Simansky <dsimansk@redhat.com>
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. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants