Skip to content

Conversation

@kalexand-rh
Copy link
Contributor

@kalexand-rh kalexand-rh added this to the Future Release milestone May 29, 2020
@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 May 29, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@kalexand-rh
Copy link
Contributor Author

@patrickdillon, will you PTAL?

Copy link
Contributor

@patrickdillon patrickdillon left a comment

Choose a reason for hiding this comment

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

I left a few comments. we also need to update upi docs for 4.5 & 4.4:

@kalexand-rh
Copy link
Contributor Author

Thank you @patrickdillon! I've made the updates that you requested in-line and will address the other changes that you pointed out on https://bugzilla.redhat.com/show_bug.cgi?id=1849434.

@gpei, will you PTAL?

@gpei
Copy link

gpei commented Jun 22, 2020

@jinyunma could you help to check this ?

Choose a reason for hiding this comment

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

The default storage for each node is 120G, plus 1 template with 16G, so the minimum size of storage required is 856 GB.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is technically correct. for vanilla ipi install bootstrap node will be torn down before worker creation so 800 gb is safe. I am fine with bumping as suggested.

@jinyunma
Copy link

I left a few comments. we also need to update upi docs for 4.5 & 4.4:

* folder name - https://github.com/openshift/installer/pull/3738/files

* machinesets - [openshift/installer#3619](https://github.com/openshift/installer/pull/3619)

@patrickdillon About "folder name", only need to be added into 4.5 upi doc, right? Or you also did some change on 4.4?

@jinyunma
Copy link

Another comment is after installing a ipi cluster on vsphere, also need the step of Image registry storage configuration, like we do in vsphere upi,
https://docs.openshift.com/container-platform/4.4/installing/installing_vsphere/installing-vsphere.html#registry-removed_installing-vsphere
@xiuwang Could you please help to check and confirm this?

@patrickdillon
Copy link
Contributor

@patrickdillon About "folder name", only need to be added into 4.5 upi doc, right? Or you also did some change on 4.4?

only 4.5. folder name still == cluster name in 4.4

@kalexand-rh kalexand-rh force-pushed the osdocs988 branch 2 times, most recently from 808cd2e to 2f07bda Compare July 2, 2020 00:01
@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 2, 2020
@kalexand-rh kalexand-rh changed the title [WIP] OSDOCS-988 vSphere IPI OSDOCS-988 vSphere IPI Jul 2, 2020
@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 2, 2020
@kalexand-rh kalexand-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 2, 2020
Copy link

@lamek lamek left a comment

Choose a reason for hiding this comment

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

Just a handful of nits. This looks really great.

@xiuwang
Copy link

xiuwang commented Jul 3, 2020

Another comment is after installing a ipi cluster on vsphere, also need the step of Image registry storage configuration, like we do in vsphere upi,
https://docs.openshift.com/container-platform/4.4/installing/installing_vsphere/installing-vsphere.html#registry-removed_installing-vsphere
@xiuwang Could you please help to check and confirm this?

Acked

@jinyunma
Copy link

jinyunma commented Jul 3, 2020

For new added section "Installing a cluster on vSphere with customization", missed the networking customization like vsphere upi in section "Installing a cluster on vSphere with user-provisioned infrastructure and network customization", since vsphere ipi installation also supports customized network.

@kalexand-rh
Copy link
Contributor Author

For new added section "Installing a cluster on vSphere with customization", missed the networking customization like vsphere upi in section "Installing a cluster on vSphere with user-provisioned infrastructure and network customization", since vsphere ipi installation also supports customized network.

@jboxman, are you planning on making an assembly for the network config for vSphere IPI?

@jboxman
Copy link
Contributor

jboxman commented Jul 6, 2020

@kalexand-rh, I can.

@kalexand-rh
Copy link
Contributor Author

@jinyunma, do you see any other changes to the documents that are currently in the scope of this PR?

@jinyunma
Copy link

jinyunma commented Jul 7, 2020

@jinyunma, do you see any other changes to the documents that are currently in the scope of this PR?

I checked and all are lgtm, except below two installation configuration parameters in topic "Installing a cluster on vSphere with customizations"[1][2] and removing machinset on all upi on vpshere installation guide.

  1. platform.vsphere.folder: the example should be /<datacenter_name>/vm/<folder_name>/<subfolder_name>

2.metadata.name: it describes that "A string that contains uppercase or lowercase letters, such as dev.". I tried to set uppercase letters for this parameter, error is shown me as below, need to reword it:
“X Sorry, your reply was invalid: a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is 'a-z0-9?(.a-z0-9?)*')”

  1. In all upi on vpshere installation topics, need to add the section to remove machinesets, which qe has already been verified on ocp4.5.
    Bug 1834966: update vSphere UPI docs to remove machinesets installer#3619

@kalexand-rh kalexand-rh force-pushed the osdocs988 branch 2 times, most recently from be1e2fd to 2d2a788 Compare July 7, 2020 21:31
@kalexand-rh
Copy link
Contributor Author

@jinyunma, will you please take another look?

@jinyunma
Copy link

jinyunma commented Jul 8, 2020

@jinyunma, will you please take another look?
no more comments. lgtm

@jinyunma
Copy link

jinyunma commented Jul 8, 2020

@kalexand-rh vsphere ipi installation has one known issue bz#1852545 . If vsphere server on customer site have multiple datacenter and clusters, then it will have multiple default root resource pool, the worker nodes will not be provisioned successfully during installation. Do you think it is good way to add the issue into known issue list in 4.5 release notes or add a note in ipi on vsphere installation doc? If need workaround, @patrickdillon may help to provide it.

@kalexand-rh
Copy link
Contributor Author

@jinyunma, I will check on bz#1852545 and tag you in whatever follow-up PR I open.

@kalexand-rh kalexand-rh force-pushed the osdocs988 branch 2 times, most recently from 49f47fd to 057d80e Compare July 8, 2020 14:59
@kalexand-rh kalexand-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jul 8, 2020
@kalexand-rh kalexand-rh merged commit 6240ff7 into openshift:master Jul 8, 2020
@kalexand-rh
Copy link
Contributor Author

/cherrypick enterprise-4.5

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #23552

Details

In response to this:

/cherrypick enterprise-4.5

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.

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

Labels

branch/enterprise-4.5 peer-review-done Signifies that the peer review team has reviewed this PR 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.