Skip to content

Create admin guide#3526

Closed
abrennan89 wants to merge 13 commits into
knative:mainfrom
abrennan89:3525
Closed

Create admin guide#3526
abrennan89 wants to merge 13 commits into
knative:mainfrom
abrennan89:3525

Conversation

@abrennan89
Copy link
Copy Markdown
Contributor

@abrennan89 abrennan89 commented May 3, 2021

Part of #3525

@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 3, 2021
@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label May 3, 2021
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 3, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from abrennan89 after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Details 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

Comment thread docs/eventing/getting-started.md
Comment thread docs/eventing/getting-started.md
@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 4, 2021
@abrennan89 abrennan89 changed the title [WIP] Create admin guide Create admin guide May 4, 2021
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2021
@abrennan89
Copy link
Copy Markdown
Contributor Author

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2021
@omerbensaadon
Copy link
Copy Markdown

/assign

Copy link
Copy Markdown
Contributor

@vaikas vaikas left a comment

Choose a reason for hiding this comment

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

Thanks!! Couple of small things.

Comment thread docs/admin/metrics/eventing-metrics-api.md Outdated
- /docs/serving/metrics/#admin
---

Administrators can monitor the Knative Serving control plane by looking at the metrics exposed by each Knative Serving component.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto. data plane.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

well :) reading through these, maybe both control and dataplane? Which makes me think if they should be grouped then differently? Thinks like Controller / Webhook vs. Activator. I think some of these are more applicable for the Application developer vs. cluster admin, but just jotting it down and get somebody from serving to help :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, always jot it down, this is all good context 😄

Is there a description of control vs dataplane anywhere that we can reuse, or can we create a blurb about each of those to explain the difference in the metrics topics? @skonto is explaining the difference between these something you could help with?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated these to say data plane and control plane, can you open a follow up issue to improve these docs maybe please and loop in SMEs so we're not blocking the PR on this?

Comment thread docs/admin/metrics/eventing-metrics-api.md Outdated
Copy link
Copy Markdown

@omerbensaadon omerbensaadon left a comment

Choose a reason for hiding this comment

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

/lgtm

Peppered in a few references to what things would look like in mkDocs 👍🏼

This is an awesome start to make your Miro 🍝 vision into a reality.

Comment thread docs/admin/install/check-install-version.md
Comment thread docs/admin/logs/_index.md

The default configuration will classify logs into:

- Knative apps, or pods with an `app=Knative` label
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should apps be "services" here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it's the same difference but I didn't write this, I'd imagine @skonto did maybe, I just cleaned it up, so let's get his opinion before we change any wording relating to components?

Copy link
Copy Markdown
Contributor

@skonto skonto May 10, 2021

Choose a reason for hiding this comment

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

I didnt write this part but I think using services is more precise, although this label is used in different places and seems a bit confusing to me eg. serving controller.

Comment thread docs/admin/logs/_index.md
Comment thread docs/admin/logs/_index.md
@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2021
Copy link
Copy Markdown
Member

@evankanderson evankanderson 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 doing this!

There are a couple places where our docs weren't in optimal places before (that you couldn't have known about, because the doc didn't say); I've left some suggestions on content reorganization that this PR made clear because of that.

Other than that, this looks great, thank you very much!

Comment thread docs/admin/_index.md

* [Knative Eventing](../eventing/): Easily route events between on-cluster and off-cluster components by exposing event routing as configuration rather than embedded in code.

These components are delivered as Kubernetes custom resource definitions (CRDs), which can be configured by a cluster administrator to provide default settings for developer-created applications and event workflow components.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there's also some common configuration options for administrators around telemetry (config-logging, config-observability) that should probably move to the admin guide:

https://knative.dev/docs/install/collecting-metrics/
https://knative.dev/docs/install/collecting-logs/

(I'm not quite sure what to do with the "here's a local-logs-collection solution" at the end of the second; probably turn it into something real or go back and delete it.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nevermind, it looks like you did this. Do you want to mention "common configuration options" as another section here, or just rely on the left-hand nav?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't want to add a bunch of links to maintain really until things settle down a bit with moving stuff, but for now I've added the contents list back to the page; does that work for you?

Comment thread docs/admin/install/_index.md
Comment thread docs/admin/install/installing-istio.md
Comment thread docs/admin/logs/_index.md
weight: 50
type: "docs"
aliases:
- /docs/install/collecting-logs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to make "logs" a top-level next to "install", add a "configuration" sub-tree to hold all the configuration, or have a "common" sub-tree to hold configuration that's common to Serving and Eventing (off the top of my head, the high-availability configuration, logging, and monitoring)?

(I'm trying to avoid the "too many items at one level in the LHS navigation" problem we have today.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can you add this as a suggestion to the Miro doc please so I can see better what you're saying / how this fits in?
We can follow it up in another PR if we want to move this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@evankanderson tracing too?

Comment thread docs/admin/serving/config-ha.md
Comment thread docs/admin/serving/webhook-customizations.md
Comment thread docs/admin/upgrade/upgrade-installation.md
Comment thread docs/eventing/getting-started.md
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label May 5, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@omerbensaadon
Copy link
Copy Markdown

/lgtm 👍🏼

@RichieEscarez RichieEscarez added Toggle me for site "Branch Deploy" build See site build at https://app.netlify.com/sites/knative/deploys/ (search PR # in log) and removed Toggle me for site "Branch Deploy" build See site build at https://app.netlify.com/sites/knative/deploys/ (search PR # in log) labels May 7, 2021
@RichieEscarez
Copy link
Copy Markdown
Contributor

FYI: This PR build is failing with:

1:45:27 PM: Error: Error building site: "/opt/build/repo/content/en/community/contributing/elections/2021-TOC/candidate-dprotaso.md:2:1": failed to unmarshal YAML: yaml: line 2: mapping values are not allowed in this context

@RichieEscarez
Copy link
Copy Markdown
Contributor

RichieEscarez commented May 10, 2021

1:45:27 PM: Error: Error building site: ....2021-TOC/candidate-dprotaso.md

@abrennan89 This should fix the broken builds: knative/website#297

@abrennan89 abrennan89 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

@abrennan89: PR needs rebase.

Details

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.

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2021
@abrennan89
Copy link
Copy Markdown
Contributor Author

A lot of this work will be negated by the new docs structure and things like the getting started, so I'm just going to start working on the mkdocs branch instead and close this PR.

@abrennan89 abrennan89 closed this May 17, 2021
@abrennan89 abrennan89 deleted the 3525 branch August 26, 2022 17:54
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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants