Skip to content

Conversation

@abrennan89
Copy link
Contributor

Fixes #978

Proposed Changes

  • Added new section for 'Using Knative Services'
  • Removed getting started with app deployment section and section on cluster-local services and pulled all of this information into one section

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 8, 2019
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 8, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: abrennan89
To complete the pull request process, please assign evankanderson
You can assign the PR to them by writing /assign @evankanderson in a comment when ready.

The full list of commands accepted by this bot can be found 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

@RichieEscarez
Copy link
Contributor

RichieEscarez commented Nov 8, 2019

Awesome! Thanks @abrennan89! I dont have time to get to this today but will soon.

Initial items ive noticed:

  • the markdown indentation issues for nested content (ie. need to indent code samples (use spaces, not tabs). [see section below about the auto-one-off staging builds]

  • our audience for these tasks are not operators so the meta about what underlies a Knative service is unnecessary and might distract users (ie developers who want to deploys services to Knative)

    • we should move the "Knative services = K8s service" details down into a separate concept topic (our Nav proposal currently has that at the bottom of the Serving section)

image

  • The title should also refocus towards that developer audience (ie. they dont need to know about the system, just how to get their services running in it). For example, from their point of view, they will always be "creating [user-created] Knative services" so we should avoid qualifying it because they might distract them from the task and cause them to ask "do I need to know what a non-user-created service is?".

A hard to use but convenient staging option

Also, we have an in-progress "docs-contributor" option of viewing a staged version of the PR. Today, the build runs and creates a unique "staged-version" of a PR (ie every commit will trigger and run a new build -> therefore the URL to this changes each time).

That said, you can preview that unique instance of your content if you look at the build page and find that build (usually a recent build near the top of the list - You click on one of those builds and then searching by your branch name works well to determine if its your content. Then when the build is successful, you click the big green [ Preview ] button and navigate to your topic): https://app.netlify.com/sites/knative/deploys

Here is the latests (remember that by committing more, it builds those changes to a different URL):
https://5dc5b121252e1800072510b0--knative.netlify.com/docs/serving/using-knative-services/

(sorry its so difficult right now but i will eventually get back to refining this process and documenting how it works)

Copy link

@dgerd dgerd left a comment

Choose a reason for hiding this comment

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

Thanks for updating this and making a straight-forward easy to follow guide for Services.

`svc.cluster.local` by
[editing the `config-domain` config map](./using-a-custom-domain.md). This
will change all services deployed through Knative to only be published to the
cluster, none will be available off-cluster.
Copy link

Choose a reason for hiding this comment

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

Might be too much information for this guide, but you can also add this domain as part of a selector statement to apply this to some of the services.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RichieEscarez wdyt with regards to document scope?

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 it can work here if we remove/revert combining all that "deploying an application" content in this file.

@dgerd Question: is creating "cluster-local services" not a common task? the idea here is that we teach users how to create a Knative service with this task and the provide the option for configuring that task to be "private"? If its not going to be a typical task, we could instead use "progressive disclosure" and only provide a "statement and link" to point users to a separate sub-topic (ie revert the "cluster-local-route" topic).

@abrennan89
Copy link
Contributor Author

* our audience for these tasks are not operators so the meta about what underlies a Knative service is unnecessary and might distract users (ie developers who want to deploys services to Knative)

@RichieEscarez do you mean the section Modifying Knative Services? I think this came from the links you provided initially about Services so I had included it, but maybe it's not required? Let me know if you want me to remove it or if you meant a different part 🙂

@abrennan89 abrennan89 force-pushed the SRVKS-335 branch 2 times, most recently from 62597d1 to 2d012cf Compare November 11, 2019 18:11
@abrennan89
Copy link
Contributor Author

@dgerd @RichieEscarez thanks so much for all the feedback and input!
I've implemented most comments now but there are a couple of comments @RichieEscarez for you just to clarify.
Lmk if there are any additional changes required.

@knative-prow-robot
Copy link
Contributor

@abrennan89: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-knative-docs-markdown-link-check 03009c3 link /test pull-knative-docs-markdown-link-check

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@samodell
Copy link
Contributor

Hey @abrennan89 ! Richie and I were discussing this, and we're not sure this is the direction we want to take the getting started content. Are you able to come to tomorrow or next week's Documentation WG to discuss? Tomorrow is the 4:30pm PT meeting, and next week is the 9:30am PT one.

Copy link
Contributor

@RichieEscarez RichieEscarez left a comment

Choose a reason for hiding this comment

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

Nice work @abrennan89! There is a lot of content here, which I think we can scope down so that we achieve our goal of "task-oriented content". I've added a few comments and questions but I think it would be good to chat about these files (as Sam suggested) in our next WG meeting.

@@ -1,136 +0,0 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not get rid of this file. If there are updates that need to be pushed into this file, we should make or at least open an issue.

We need this doc to provide the end-to-end Serving details that touch on the main concepts and quickly gets someone up and running. While this file (in its current state) is light on the conceptual details, its better as-is than without (there is a todo to get this up to the level that the "Eventing Getting Started" is at).


## Creating a private cluster-local Service
By default services deployed through Knative are published to an external IP
address, making them public services on a public IP address and with a
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 that if we use active voice here, we can address @dgerd's point too, for example:

Suggested change
address, making them public services on a public IP address and with a
address. After you configure your [custom domain](./using-a-custom-domain.md), you can publicly access your services.

## Creating a private cluster-local Service
By default services deployed through Knative are published to an external IP
address, making them public services on a public IP address and with a
[public URL](./using-a-custom-domain.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[public URL](./using-a-custom-domain.md).

Knative provides two ways to enable private services which are only available
inside the cluster:

1. To make all services only available only from within the cluster, change the default domain to
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the comment above. Since these are two options and not a set of linear steps, 'bulleted lists' work better here.

`svc.cluster.local` by
[editing the `config-domain` config map](./using-a-custom-domain.md). This
will change all services deployed through Knative to only be published to the
cluster, none will be available off-cluster.
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 it can work here if we remove/revert combining all that "deploying an application" content in this file.

@dgerd Question: is creating "cluster-local services" not a common task? the idea here is that we teach users how to create a Knative service with this task and the provide the option for configuring that task to be "private"? If its not going to be a typical task, we could instead use "progressive disclosure" and only provide a "statement and link" to point users to a separate sub-topic (ie revert the "cluster-local-route" topic).

[editing the `config-domain` config map](./using-a-custom-domain.md). This
will change all services deployed through Knative to only be published to the
cluster, none will be available off-cluster.
1. To make an individual service cluster-local, the service or route can be
Copy link
Contributor

Choose a reason for hiding this comment

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

"cluster-local" appears to be a visibility label that is Knative specific but Kubernetes refers to "both "cluster-private-IP" and "cluster-internal IP" terms". For consistency we should try to use the same terminology (versus saying "to be cluster-local")

For example: You can configure a Knative service to use a "cluster-private-IP"/"cluster-internal IP" if you add the cluster-local label to that service.

The service returns the a URL with the `svc.cluster.local` domain, indicating
the service is only available in the cluster local network.

## Deploying an application using Knative Services
Copy link
Contributor

Choose a reason for hiding this comment

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

From here down i think ALL but "Modifying Knative Services", is out of scope for this "Creating a Knative service" task and should move back into the "Getting Started" guide.

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

Labels

cla: yes Indicates the PR's author has signed the CLA. kind/serving 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.

Can't find docs on how to create cluster internal-only Knative service

6 participants