docs/user: Standardize install-config property documentation #2162

wking · 2019-08-05T22:13:51Z

@abhinavdahiya, does this look like a reasonable path?

patrickdillon

I do think an example install config would be helpful.

I also provided some minor nitpicky copy edit suggestions. The changes probably need to also be copied to the originating comments.

docs/user/customization.md

wking · 2019-09-01T21:44:46Z

Ok, I've pushed 96b16e9d4 -> 211796a93 filling in most of the rest of this (enough to remove the WIP tag from the PR subject). I'm not clear on a bunch of the vSphere and OpenStack stuff though; can I get some help there, @abhinavdahiya, @tomassedovic, @Fedosin?

abhinavdahiya · 2019-09-03T16:50:55Z

docs/user/customization.md

gcp, none, baremetal ?

gcp, none, baremetal ?

Can we grow those in follow-up work? We weren't linking to them before this PR, and I'm not all that familiar with whether parameters are required or not. Seems like it would be easier to review if they had platform-specific PRs.

docs/user/customization.md

jcpowermac · 2019-09-04T19:59:01Z

docs/user/vsphere/customization.md

Looks good!

docs/user/openstack/customization.md

wking · 2019-09-09T18:53:47Z

docs/user/openstack/customization.md

It's also strange that we use compute in a parameter that is configuring the control plane machines.

docs/user/openstack/customization.md

This is a bit more accessible than pointing folks at Godocs, since it allows us to focus on the YAML property names (while Godocs understandably focus on Go property names) and YAML renderings. Also break up our old "one big example" install-config.yaml into a minimal per-platform example and a series of small extentions excercising groups of properties. The vSphere docs are based heavily on [1]. Also drop proxy.md. It was added in e7edbf7 (Add proxy configuration to bootstrap node, 2019-06-24, openshift#1832), but: * Proxy testing and Squid configuration information belongs in openshift/release, not in the installer repository. * docs/user/customization.md now contains a more complete proxy-config fragment. OpenStack computeFlavor precedence is based on [2]. [1]: https://github.com/openshift/openshift-docs/blob/enterprise-4.2/modules/installation-vsphere-config-yaml.adoc Last touched by commit openshift/openshift-docs@25afc76 , 2019-08-19 [2]: openshift#2162 (comment)

Fedosin · 2019-09-09T20:03:33Z

/lgtm

openshift-ci-robot · 2019-09-09T20:03:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Fedosin, wking

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

abhinavdahiya · 2019-09-09T22:17:17Z

docs/user/aws/customization.md

+    Resources created by the cluster itself may not include these tags.
+* `defaultMachinePlatform` (optional object): Default [AWS-specific machine pool properties](#machine-pools) which applies to [machine pools](../customization.md#machine-pools) that do not define their own AWS-specific properties.
+
+## Machine pools


nit: match with something similar to Cluster-scoped properties

nit: match with something similar to Cluster-scoped properties

Do you mean "Machine pools" -> "Machine-pool properties"? Or something else?

abhinavdahiya · 2019-09-09T22:18:14Z

docs/user/aws/customization.md

- `platform.aws.userTags` - a map of keys and values that the installer will add as tags to all resources it creates
+## Cluster-scoped properties
+
+* `amiID` (optional string): The AMI that should be used to boot machines for the cluster.


should string be surrounded with back ticks?

should string be surrounded with back ticks?

Like:

* `amiID` (optional `string`): ...

? I think it's fine without that; if we wanted to be formal we could link to the JSON RFC for string or the YAML string spec. But the YAML spec is pretty dense; I'm fine just leaving it as an informal "string" and expecting folks to figure it out. I'm also fine linking to the specs if you want me to file that as follow-up work.

abhinavdahiya · 2019-09-09T22:19:56Z

docs/user/aws/customization.md

+* `amiID` (optional string): The AMI that should be used to boot machines for the cluster.
+    If set, the AMI should belong to the same region as the cluster.
+* `region` (required string): The AWS region where the cluster will be created.
+* `userTags` (optional object): Additional keys and values that the installer will add as tags to all resources that it creates.


hmm, object is probably not accurate here because it is map string->string, where objects are more like rootVolume ie has known fields and values.

hmm, object is probably not accurate here because it is map string->string...

That's still a JSON object (YAML mapping), we just leave the set of valid keys more open-ended than for JSON objects backed by Go structs.

openshift-bot · 2019-09-09T22:40:07Z

/retest

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

openshift-ci-robot · 2019-09-10T00:50:32Z

@wking: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-openstack	`a947609`	link	`/test e2e-openstack`
ci/prow/e2e-libvirt	`a947609`	link	`/test e2e-libvirt`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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

Typo is from a947609 (docs/user: Standardize install-config property documentation, 2019-08-05, openshift#2162).

This is a bit more accessible than pointing folks at Godocs, since it allows us to focus on the YAML property names (while Godocs understandably focus on Go property names) and YAML renderings. Also break up our old "one big example" install-config.yaml into a minimal per-platform example and a series of small extentions excercising groups of properties. The vSphere docs are based heavily on [1]. Also drop proxy.md. It was added in e7edbf7 (Add proxy configuration to bootstrap node, 2019-06-24, openshift#1832), but: * Proxy testing and Squid configuration information belongs in openshift/release, not in the installer repository. * docs/user/customization.md now contains a more complete proxy-config fragment. OpenStack computeFlavor precedence is based on [2]. [1]: https://github.com/openshift/openshift-docs/blob/enterprise-4.2/modules/installation-vsphere-config-yaml.adoc Last touched by commit openshift/openshift-docs@25afc76 , 2019-08-19 [2]: openshift#2162 (comment)

Typo is from a947609 (docs/user: Standardize install-config property documentation, 2019-08-05, openshift#2162).

It's been vCenter since the property landed in 3e73413 (types/vsphere: simplify vsphere platform, 2019-04-10, openshift#1591). This commit fixes "vcenter" typos from 0ec07d0 (docs: vSphere installation docs, 2019-04-07, openshift#1545) and a947609 (docs/user: Standardize install-config property documentation, 2019-08-05, openshift#2162).

openshift-ci-robot requested review from jcpowermac and patrickdillon August 5, 2019 22:14

patrickdillon reviewed Aug 8, 2019

View reviewed changes

docs/user/customization.md Outdated Show resolved Hide resolved

docs/user/customization.md Outdated Show resolved Hide resolved

docs/user/customization.md Outdated Show resolved Hide resolved

wking commented Aug 8, 2019

View reviewed changes

docs/user/customization.md Outdated Show resolved Hide resolved

markmc reviewed Aug 21, 2019

View reviewed changes

docs/user/customization.md Outdated Show resolved Hide resolved

docs/user/customization.md Outdated Show resolved Hide resolved

wking mentioned this pull request Aug 29, 2019

Documentation is not friendly #2285

Closed

wking force-pushed the install-config-markdown-docs branch 2 times, most recently from 96b16e9 to 211796a Compare September 1, 2019 21:42

wking changed the title ~~WIP: docs/user: Standardize install-config property documentation~~ docs/user: Standardize install-config property documentation Sep 1, 2019

abhinavdahiya reviewed Sep 3, 2019

View reviewed changes

jcpowermac reviewed Sep 4, 2019

View reviewed changes

docs/user/customization.md Outdated Show resolved Hide resolved

jcpowermac reviewed Sep 4, 2019

View reviewed changes

docs/user/vsphere/customization.md Outdated

Copy link

Contributor

jcpowermac Sep 4, 2019

Choose a reason for hiding this comment

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

Looks good!

wking force-pushed the install-config-markdown-docs branch from 211796a to 91c8e95 Compare September 4, 2019 22:17

wking commented Sep 9, 2019

View reviewed changes

docs/user/openstack/customization.md Outdated Show resolved Hide resolved

wking force-pushed the install-config-markdown-docs branch from 91c8e95 to 1d3e2ea Compare September 9, 2019 18:53

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2019

wking commented Sep 9, 2019

View reviewed changes

wking force-pushed the install-config-markdown-docs branch from 1d3e2ea to ea6f75e Compare September 9, 2019 18:57

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2019

iamemilio reviewed Sep 9, 2019

View reviewed changes

docs/user/openstack/customization.md Outdated Show resolved Hide resolved

Fedosin suggested changes Sep 9, 2019

View reviewed changes

docs/user/openstack/customization.md Outdated Show resolved Hide resolved

docs/user/openstack/customization.md Outdated Show resolved Hide resolved

docs/user/openstack/customization.md Outdated Show resolved Hide resolved

wking force-pushed the install-config-markdown-docs branch from ea6f75e to 715a847 Compare September 9, 2019 19:36

wking force-pushed the install-config-markdown-docs branch from 715a847 to a947609 Compare September 9, 2019 19:37

openshift-ci-robot assigned Fedosin Sep 9, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2019

abhinavdahiya reviewed Sep 9, 2019

View reviewed changes

openshift-merge-robot merged commit edbfd3e into openshift:master Sep 9, 2019

wking deleted the install-config-markdown-docs branch September 10, 2019 04:10

mandre mentioned this pull request Sep 10, 2019

OpenStack: clarify doc around computeFlavor property #2339

Merged

wking added a commit to wking/openshift-installer that referenced this pull request Sep 26, 2019

docs/user/customization: Fix "utalize" -> "utilize" typo

f58d49a

Typo is from a947609 (docs/user: Standardize install-config property documentation, 2019-08-05, openshift#2162).

wking mentioned this pull request Sep 26, 2019

docs/user/customization: Fix "utalize" -> "utilize" typo #2418

Merged

jhixson74 pushed a commit to jhixson74/installer that referenced this pull request Dec 6, 2019

docs/user/customization: Fix "utalize" -> "utilize" typo

ac47880

Typo is from a947609 (docs/user: Standardize install-config property documentation, 2019-08-05, openshift#2162).

wking mentioned this pull request Jan 10, 2020

docs/user/vsphere: Fix "vcenter" -> "vCenter" #2902

Merged

docs/user: Standardize install-config property documentation #2162

docs/user: Standardize install-config property documentation #2162

Uh oh!

Conversation

wking commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wking commented Sep 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fedosin commented Sep 9, 2019

Uh oh!

openshift-ci-robot commented Sep 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking Sep 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Sep 9, 2019

Uh oh!

openshift-ci-robot commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

wking commented Aug 5, 2019 •

edited

Loading

wking Sep 9, 2019 •

edited

Loading