Skip to content

add base documentation for all operator+config resources #15806

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
Closed
wants to merge 1 commit into from
Closed

add base documentation for all operator+config resources #15806

wants to merge 1 commit into from

Conversation

@bparees
Copy link
Contributor

@bparees bparees commented Jul 9, 2019

No description provided.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 9, 2019
@bparees
Copy link
Contributor Author

bparees commented Jul 9, 2019

@kalexand-rh @bmcelvee ptal

this is my attempt to straighten out/provide a base for documenting our various cluster config + operator config resources which a cluster admin would interact with in the course of configuring their cluster.

deads2k
deads2k reviewed Jul 10, 2019
View reviewed changes
installing/install_config/customizations.adoc Outdated
Copy link
Contributor

@deads2k deads2k Jul 10, 2019

Choose a reason for hiding this comment

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

We do not wish to document this for cluster-admins. This is not an area where they are allowed to alter configuration and status is reported back through our clusteroperator, so they do not need to view this resource for status either.

Copy link
Contributor Author

@bparees bparees Jul 10, 2019

Choose a reason for hiding this comment

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

will remove

deads2k
deads2k reviewed Jul 10, 2019
View reviewed changes
installing/install_config/customizations.adoc Outdated
Copy link
Contributor

@deads2k deads2k Jul 10, 2019

Choose a reason for hiding this comment

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

looks like it has leaked here too. @bergerhoffer please remove references to

  1. kubeapiserver.operator.openshift.io
  2. kubecontrollermanager.operator.openshift.io
  3. kubescheduler.operator.openshift.io
  4. openshiftapiserver.operator.openshift.io
  5. authentication.operator.openshift.io
  6. serviceca.operator.openshift.io

These are not resources for cluster-admins. This is not an area where they are allowed to alter configuration and status is reported back through our clusteroperator, so they do not need to view this resource for status either.

Copy link
Contributor Author

@bparees bparees Jul 10, 2019

Choose a reason for hiding this comment

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

this is commented out and doesn't appear in the rendered doc. not sure why it was put here in the first place but i left it as a reference of the "global set of stuff".

@bparees bparees changed the title add base documentation for all operator+config resources [WIP]add base documentation for all operator+config resources Jul 10, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2019
@bparees bparees changed the title [WIP]add base documentation for all operator+config resources add base documentation for all operator+config resources Jul 10, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2019
@bparees
Copy link
Contributor Author

bparees commented Jul 10, 2019

@deads2k updated w/ the outcomes from our discussion

deads2k
deads2k reviewed Jul 11, 2019
View reviewed changes
installing/install_config/customizations.adoc Outdated
Copy link
Contributor

@deads2k deads2k Jul 11, 2019

Choose a reason for hiding this comment

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

minor question: components or features? I think of them as features.

Copy link
Contributor Author

@bparees bparees Jul 11, 2019

Choose a reason for hiding this comment

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

i think of them as components (the bits of code implementing the feature) but you're probably right that to a user it's a feature. i'll change it.

deads2k
deads2k reviewed Jul 11, 2019
View reviewed changes
installing/install_config/customizations.adoc Outdated
Copy link
Contributor

@deads2k deads2k Jul 11, 2019

Choose a reason for hiding this comment

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

can you add a comment (not visible in rendered docs I hope), that say that if you find yourself writing a help page that requires coordinated changes on two resources, they should tag api-approvers and get some help rethinking their API?

Copy link
Contributor Author

@bparees bparees Jul 11, 2019

Choose a reason for hiding this comment

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

i.....can...... but i would imagine by the time someone is writing that doc the ship has already sailed.

your comment makes me think this has already happened? :)

Copy link
Contributor Author

@bparees bparees Jul 11, 2019

Choose a reason for hiding this comment

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

comment added.

@deads2k
Copy link
Contributor

deads2k commented Jul 11, 2019

minor comments and question

/lgtm
/hold
holding for others to have a shot.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Jul 11, 2019
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2019
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 11, 2019

New changes are detected. LGTM label has been removed.

@adambkaplan
Copy link
Contributor

adambkaplan commented Jul 12, 2019

Devex components look good. Allowing other teams to weigh in.

@bparees
Copy link
Contributor Author

bparees commented Jul 15, 2019

@bmcelvee @kalexand-rh i think this has gotten all the engineering attention it's going to, ptal/merge.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019
@bparees
Copy link
Contributor Author

bparees commented Jul 16, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2019
@bparees
Copy link
Contributor Author

bparees commented Jul 16, 2019

remade this PR against master: #15905

will cherry-pick that back to 4.1 and 4.2.

@bparees bparees closed this Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deads2k deads2k deads2k left review comments

Assignees

@deads2k deads2k

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bparees @deads2k @openshift-ci-robot @adambkaplan