Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Feb 1, 2020

This is part of decoupling our docs from the assumption that a bastion will exist. For installation, we just care that there is a registry with the mirrored content that the cluster can pull from. Having that
registry on a bastion makes creating the mirrored images easier, but as our existing docs hint, there are other ways you can get the mirrored images in to your mirror repository. This commit borrows its motivation and some of its wording from @smarterclayton's here.

Also push the port information into <mirror_registry>, because we want to make it clear that we don't require a port for these entries. The previous content was not consistent about whether placeholder were
referenced with or without their wrapping <>. In my new text, I've included the <> to make it more clear that they are part of the placeholder, and not literal characters that should remain after placeholder substitution.

CC @kalexand-rh. There's a lot of duplicate-ish content in here. Any ideas for DRYing it up?

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 1, 2020
@wking wking force-pushed the rename-bastion-to-mirror-host branch from 9c3b13e to 91dbff4 Compare February 1, 2020 05:24
@wking wking changed the title installing/install_config/installing-restricted-networks-preparations: Rename bastion -> mirror host *: Rename bastion -> mirror host Feb 1, 2020
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

Our style guide forbids the use of "may," and that's really the only issue that i see. Most of the repetition is either due to the fact that there's a separate assembly for each disconnected installation or the fact that there are separate yaml samples for each installation type. (If I see a good opportunity to clean up the latter, I will.)

Copy link
Contributor

Choose a reason for hiding this comment

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

That was a reminder for myself that I didn't need to install the CLI as part of the installation process. You install from the mirror host and already have it installed. (Also, "may" doesn't translate well, so I wouldn't add it.)

Copy link
Contributor

Choose a reason for hiding this comment

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

may

Copy link
Contributor

Choose a reason for hiding this comment

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

I get that you're trying to step back from the bastion, but do you have a particular workflow in mind? "May implies permission, and that's not really what we mean here.

I'd either you may or, if you want to back it off more and still want them to use the installer that they generated from the mirror repo, say something like "Because the installation media is on the mirror host, the following steps assume that you use that computer to complete all installation steps. If you cannot complete the {product-title} installation from the mirror host, use the method that best fits your restrictions to ensure that you use the installation program that you generated from the mirrored content."

wking and others added 2 commits June 22, 2020 11:32
This is part of decoupling our docs from the assumption that a bastion
will exist.  For installation, we just care that there is a registry
with the mirrored content that the cluster can pull from.  Having that
registry on a bastion makes creating the mirrored images easier, but
as our existing docs hint, there are other ways you can get the
mirrored images in to your mirror repository.  This commit borrows its
motivation and some of its wording from Clayton's [1].

Also push the port information into <mirror_registry>, because we want
to make it clear that we don't require a port for these entries.  The
previous content was not consistent about whether placeholder were
referenced with or without their wrapping <>.  In my new text, I've
included the <> to make it more clear that they are part of the
placeholder, and not literal characters that should remain after
placeholder substitution.

[1]: openshift#17896 (comment)
@kalexand-rh kalexand-rh force-pushed the rename-bastion-to-mirror-host branch from 91dbff4 to 6973df0 Compare June 22, 2020 15:44
@kalexand-rh
Copy link
Contributor

@wking, will you PTAL at 6973df0? I rebased and made a few style edits.

@kalexand-rh
Copy link
Contributor

@jianlinliu,will you PTAL?

@wking, I want to merge this as soon as Jianlin blesses it so I can resolve any conflicts with Clayton's PR.

@jianlinliu
Copy link

LGTM

@wking
Copy link
Member Author

wking commented Jun 23, 2020

Don't block on me. I can always file further suggestions in the off chance I notice anything post-merge.

@kalexand-rh kalexand-rh merged commit 0818562 into openshift:master Jun 23, 2020
@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.2

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.3

@openshift-cherrypick-robot

@kalexand-rh: #19435 failed to apply on top of branch "enterprise-4.2":

Using index info to reconstruct a base tree...
M	installing/installing_aws/installing-restricted-networks-aws.adoc
M	installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
M	installing/installing_vsphere/installing-restricted-networks-vsphere.adoc
M	modules/installation-about-restricted-network.adoc
M	modules/installation-adding-registry-pull-secret.adoc
M	modules/installation-bare-metal-config-yaml.adoc
M	modules/installation-generate-aws-user-infra-install-config.adoc
M	modules/installation-mirror-repository.adoc
M	modules/installation-obtaining-installer.adoc
M	modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
M	modules/installation-vsphere-config-yaml.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-vsphere-config-yaml.adoc
CONFLICT (content): Merge conflict in modules/installation-vsphere-config-yaml.adoc
Auto-merging modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
CONFLICT (content): Merge conflict in modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
Auto-merging modules/installation-obtaining-installer.adoc
CONFLICT (content): Merge conflict in modules/installation-obtaining-installer.adoc
Auto-merging modules/installation-mirror-repository.adoc
Auto-merging modules/installation-generate-aws-user-infra-install-config.adoc
Auto-merging modules/installation-bare-metal-config-yaml.adoc
CONFLICT (content): Merge conflict in modules/installation-bare-metal-config-yaml.adoc
Auto-merging modules/installation-adding-registry-pull-secret.adoc
Auto-merging modules/installation-about-restricted-network.adoc
Auto-merging installing/installing_vsphere/installing-restricted-networks-vsphere.adoc
Auto-merging installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
Auto-merging installing/installing_aws/installing-restricted-networks-aws.adoc
error: Failed to merge in the changes.
Patch failed at 0001 *: Rename bastion -> mirror host

Details

In response to this:

/cherrypick enterprise-4.2

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.

@openshift-cherrypick-robot

@kalexand-rh: #19435 failed to apply on top of branch "enterprise-4.3":

Using index info to reconstruct a base tree...
M	installing/installing_aws/installing-restricted-networks-aws.adoc
M	installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
M	installing/installing_vsphere/installing-restricted-networks-vsphere.adoc
M	modules/installation-about-restricted-network.adoc
M	modules/installation-adding-registry-pull-secret.adoc
M	modules/installation-bare-metal-config-yaml.adoc
M	modules/installation-generate-aws-user-infra-install-config.adoc
M	modules/installation-mirror-repository.adoc
M	modules/installation-obtaining-installer.adoc
M	modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
M	modules/installation-vsphere-config-yaml.adoc
M	modules/olm-building-operator-catalog-image.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/olm-building-operator-catalog-image.adoc
Auto-merging modules/installation-vsphere-config-yaml.adoc
Auto-merging modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
CONFLICT (content): Merge conflict in modules/installation-user-infra-generate-k8s-manifest-ignition.adoc
Auto-merging modules/installation-obtaining-installer.adoc
CONFLICT (content): Merge conflict in modules/installation-obtaining-installer.adoc
Auto-merging modules/installation-mirror-repository.adoc
Auto-merging modules/installation-generate-aws-user-infra-install-config.adoc
Auto-merging modules/installation-bare-metal-config-yaml.adoc
CONFLICT (content): Merge conflict in modules/installation-bare-metal-config-yaml.adoc
Auto-merging modules/installation-adding-registry-pull-secret.adoc
Auto-merging modules/installation-about-restricted-network.adoc
Auto-merging installing/installing_vsphere/installing-restricted-networks-vsphere.adoc
Auto-merging installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
Auto-merging installing/installing_aws/installing-restricted-networks-aws.adoc
error: Failed to merge in the changes.
Patch failed at 0001 *: Rename bastion -> mirror host

Details

In response to this:

/cherrypick enterprise-4.3

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.

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.4

@openshift-cherrypick-robot

@kalexand-rh: #19435 failed to apply on top of branch "enterprise-4.4":

Using index info to reconstruct a base tree...
M	modules/installation-about-restricted-network.adoc
M	modules/installation-adding-registry-pull-secret.adoc
M	modules/installation-bare-metal-config-yaml.adoc
M	modules/installation-creating-mirror-registry.adoc
M	modules/installation-generate-aws-user-infra-install-config.adoc
M	modules/installation-mirror-repository.adoc
M	modules/installation-obtaining-installer.adoc
M	modules/installation-vsphere-config-yaml.adoc
M	modules/olm-building-operator-catalog-image.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/olm-building-operator-catalog-image.adoc
Auto-merging modules/installation-vsphere-config-yaml.adoc
Auto-merging modules/installation-obtaining-installer.adoc
CONFLICT (content): Merge conflict in modules/installation-obtaining-installer.adoc
Auto-merging modules/installation-mirror-repository.adoc
Auto-merging modules/installation-generate-aws-user-infra-install-config.adoc
Auto-merging modules/installation-creating-mirror-registry.adoc
Auto-merging modules/installation-bare-metal-config-yaml.adoc
CONFLICT (content): Merge conflict in modules/installation-bare-metal-config-yaml.adoc
Auto-merging modules/installation-adding-registry-pull-secret.adoc
Auto-merging modules/installation-about-restricted-network.adoc
error: Failed to merge in the changes.
Patch failed at 0001 *: Rename bastion -> mirror host

Details

In response to this:

/cherrypick enterprise-4.4

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.

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.5

@openshift-cherrypick-robot

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

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.

@wking wking deleted the rename-bastion-to-mirror-host branch June 23, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants