[OSDOCS-5240] Installer support to automatically create the MachineSets when installing in existing VPC on AWS w/Local Zones #57427

maxwelldb · 2023-03-20T16:01:25Z

Version(s): 4.13

Link to docs preview: https://57427--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_aws/installing-aws-localzone.html

QE review:

QE has approved this change.

ocpdocs-previewbot · 2023-03-20T16:47:37Z

🤖 Updated build preview is available at:
https://57427--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/15883

mtulio · 2023-03-20T17:19:13Z

@maxwelldb :

@mtulio Is there anything in https://docs.openshift.com/container-platform/4.12/installing/installing_aws/installing-aws-vpc.html that would be incompatible with your procedure? Primarily, I'm thinking of requirements, "About using a custom VPC," the log in sections, and the general install-config content.

This work (phase-1) complements the previous documentation created on [1][2] (aka phase-0). I think [1] (more specific [3]) could be adapted to this PR, thus we can keep a single page for Local Zone subject, thoughts (+ @kalexand-rh @dfitzmau )?

[1] https://docs.openshift.com/container-platform/4.12/installing/installing_aws/installing-aws-localzone.html
[2] #54535
[3]
Creating the installation configuration file: https://docs.openshift.com/container-platform/4.12/installing/installing_aws/installing-aws-localzone.html#installation-generate-aws-user-infra-install-config_installing-aws-localzone
Creating the Kubernetes manifest files: https://docs.openshift.com/container-platform/4.12/installing/installing_aws/installing-aws-localzone.html#installation-localzone-generate-k8s-manifestinstalling-aws-localzone

maxwelldb · 2023-03-20T17:35:01Z

@mtulio I'm also happy to integrate whatever is new for SPLAT-363 into https://docs.openshift.com/container-platform/4.12/installing/installing_aws/installing-aws-localzone.html. That assumes that the user story for the new content is simpatico with the old content, which I gather is the case?

At the moment, this PR is about getting what I grabbed from openshift/installer#6371 into something that roughly resembles our mod docs format and then thinking through what is essential to the story and what is not. It's flexible. 👍

mtulio · 2023-03-20T17:41:04Z

That assumes that the user story for the new content is simpatico with the old content, which I gather is the case?

Yes, currently phase-1 (this PR) will automate the sections I mentioned above, adding manifests manually.

At the moment, this PR is about getting what I grabbed from openshift/installer#6371 into something that roughly resembles our mod docs format and then thinking through what is essential to the story and what is not. It's flexible.

That's perfect! Thanks!

installing/installing_aws/installing-aws-vpc-local-zones.adoc

maxwelldb · 2023-03-21T16:11:14Z

modules/install-creating-aws-local-zones-subnet.adoc

Probably redundant. Keeping for now. Not currently incorporated into installation guide.

modules/install-creating-install-config-aws-local-zones.adoc

maxwelldb · 2023-03-21T16:15:57Z

modules/install-creating-aws-local-zones-vpc-stack.adoc

Probably redundant. Keeping for now. Not currently incorporated into installation guide.

maxwelldb · 2023-03-21T17:42:00Z

@mtulio https://57427--docspreview.netlify.app/openshift-enterprise/latest/installing/installing_aws/installing-aws-localzone.html incorporates some of the edge pool content from openshift/installer#6371 and replaces the CloudFormation templates with the latest from the installer repo. What do you think so far? Feel free to make comments or suggestions against the source files directly.

The 4.12 guide uses JSON files to handle a number of parameters for configuration and deployment. Is there any reason not to keep to that pattern here, at least as far as CloudFormation templates go?

Assuming that's fine, it seems like what remains to be exported is just ZONE_GROUP_NAME. Does that seem right?

mtulio

Great! Initial review of the structure. Thanks

mtulio · 2023-03-21T19:09:48Z

modules/machines-edge-machine-pool.adoc

+* `zone_type=local-zone`
+* `zone_group=<LOCAL_ZONE_GROUP>`


Those labels should be updated to:

machine.openshift.io/zone-group=<$ZONE_GROUP_NAME>

machine.openshift.io/zone-type=local-zone

Could you please make sure those labels are set correctly on the entire Local Zone book? It's defined on the Enhancement Proposal and installer implementation for phase-1, but it seems to passed from the phase-0 docs review.

Beyond what this PR touches, I'm not seeing these labels anywhere.

modules/installation-aws-local-zones-ref-deployment.adoc

modules/install-creating-install-config-aws-local-zones.adoc

modules/machines-edge-machine-pool.adoc

mtulio · 2023-03-21T19:26:00Z

modules/nw-aws-local-zones-user-workload-ingress.adoc

@@ -0,0 +1,9 @@
+:content-type: CONCEPT


same comment for file modules/installation-aws-local-zones-ref-deployment.adoc

Marking this for removal.

mtulio · 2023-03-21T19:30:06Z

The 4.12 guide uses JSON files to handle a number of parameters for configuration and deployment. Is there any reason not to keep to that pattern here, at least as far as CloudFormation templates go?

Particularly I prefer the inline method which could save some steps. I think it is an improvement based on the last version. Open for thoughts.

maxwelldb · 2023-03-21T20:06:39Z

Particularly I prefer the inline method which could save some steps. I think it is an improvement based on the last version. Open for thoughts.

My preference is to use content that's already been through our review process when possible, though I'm not against your method if there's time. Could we agree to make that a low priority switch within this PR?

maxwelldb · 2023-03-21T20:10:36Z

modules/nw-aws-local-zones-user-workload-ingress.adoc

Not sure where to put this yet, or even if it should be included.

I think it could be important to note if the user add the gp3 as volume type to be used on edge pool, the machineset manifest will be forced to use it, if gp3 is not supported on the location (discovered by the subnet ID for Local Zones) the machine will fail to create.

is it possible to add a comment after the 'title' of the example? (below)

.Example edge pool with a custom EBS type

@mtulio Which example are you referring to? Just want to be sure that I'm tagging the right thing.

@mtulio Which example are you referring to? Just want to be sure that I'm tagging the right thing.

https://github.com/openshift/openshift-docs/pull/57427/files#diff-e8e7d7d1e0bcd0f4d0d8c83c41c85d326762ebc6d9a96fcae8b52f055e79a926R74-R101

I can insert text before or after the sample and its title, sure.

mtulio · 2023-03-21T20:10:50Z

Particularly I prefer the inline method which could save some steps. I think it is an improvement based on the last version. Open for thoughts.

My preference is to use content that's already been through our review process when possible, though I'm not against your method if there's time. Could we agree to make that a low priority switch within this PR?

For sure, I am reviewing the other references for AWS CLI in our docs, it also uses --parameters file:// when using cloudformation, so LGTM to keep it.

Example:

modules/install-creating-install-config-aws-local-zones.adoc

Co-authored-by: Yunfei Jiang <[email protected]>

maxwelldb · 2023-05-09T15:03:45Z

@yunjiang29 What do you think of my comment here: https://github.com/openshift/openshift-docs/pull/57427/files/73048b401fc2219248d960e153ea71f5b131495b#r1187570706

Generate install-config.yaml -> read about what you'll edit in the install-config.yaml -> edit install-config.yaml isn't an unusual pattern in the docs, and I don't think there's duplication in the current state of the PR, but I'm happy to change things. This is not my usual territory. :)

yunjiang29 · 2023-05-10T06:34:02Z

/lgtm

maxwelldb · 2023-05-10T12:32:15Z

Thank you!

lpettyjo

Otherwise, LGTM. Nice work!

modules/edge-machine-pools-aws-local-zones.adoc

modules/machine-edge-pool-review-nodes.adoc

modules/machines-edge-machine-pool.adoc

modules/edge-machine-pools-aws-local-zones.adoc

openshift-ci · 2023-05-11T19:21:38Z

New changes are detected. LGTM label has been removed.

modules/edge-machine-pools-aws-local-zones.adoc

modules/install-creating-install-config-aws-local-zones.adoc

modules/machine-edge-pool-review-nodes.adoc

modules/machines-edge-machine-pool.adoc

maxwelldb · 2023-05-11T20:01:05Z

/cherry-pick enterprise-4.13

openshift-cherrypick-robot · 2023-05-11T20:02:47Z

@maxwelldb: new pull request created: #59950

Details

In response to this:

/cherry-pick enterprise-4.13

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.

…s on the structure (openshift#4) * OSDOCS-5240 openshift#57427: review rendered page with important fixes on the structure * Apply suggestions from code review Co-authored-by: Max Bridges <[email protected]> * Apply suggestions from code review Co-authored-by: Max Bridges <[email protected]> --------- Co-authored-by: Max Bridges <[email protected]>

maxwelldb self-assigned this Mar 20, 2023

openshift-ci bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 20, 2023

maxwelldb mentioned this pull request Mar 20, 2023

OSDOCS-5240: doc/fix-aws-localzones: replace jq to aws --query openshift/installer#6993

Merged

maxwelldb added the branch/enterprise-4.13 label Mar 20, 2023

maxwelldb added this to the Planned for 4.13 GA milestone Mar 20, 2023

maxwelldb changed the title ~~[OSDOCS-5240] Add AWS Local Zones + VPC installation guide~~ [OSDOCS-5240] Installer support to automatically create the MachineSets when installing in existing VPC on AWS w/Local Zones Mar 20, 2023

maxwelldb force-pushed the aws-local-zones-existing-vpc-osdocs5240 branch from 1baddd9 to f8ee526 Compare March 21, 2023 15:15

maxwelldb commented Mar 21, 2023

View reviewed changes

installing/installing_aws/installing-aws-vpc-local-zones.adoc Outdated Show resolved Hide resolved

maxwelldb force-pushed the aws-local-zones-existing-vpc-osdocs5240 branch from 0f5493c to c86374c Compare March 21, 2023 16:00

openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 21, 2023

maxwelldb commented Mar 21, 2023

View reviewed changes

modules/install-creating-install-config-aws-local-zones.adoc Show resolved Hide resolved

maxwelldb commented Mar 21, 2023

View reviewed changes

openshift-ci bot 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 Mar 21, 2023

maxwelldb force-pushed the aws-local-zones-existing-vpc-osdocs5240 branch from 527f0ac to b3ced87 Compare March 21, 2023 17:10

maxwelldb requested a review from mtulio March 21, 2023 17:42

mtulio reviewed Mar 21, 2023

View reviewed changes

maxwelldb commented Mar 21, 2023

View reviewed changes

maxwelldb commented Mar 23, 2023

View reviewed changes

modules/install-creating-install-config-aws-local-zones.adoc Show resolved Hide resolved

maxwelldb requested a review from mtulio March 23, 2023 14:09

maxwelldb mentioned this pull request May 9, 2023

[enterprise-4.13] ReleaseNote for AWS Local Zones - Phase 1 IPI edge pool #59728

Merged

1 task

Update param key and val block

ecfe528

Co-authored-by: Yunfei Jiang <[email protected]>

maxwelldb requested a review from yunjiang29 May 9, 2023 15:04

openshift-ci bot assigned yunjiang29 May 10, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 10, 2023

maxwelldb added the peer-review-needed Signifies that the peer review team needs to review this PR label May 10, 2023

lpettyjo added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels May 11, 2023

lpettyjo self-requested a review May 11, 2023 12:54

lpettyjo reviewed May 11, 2023

View reviewed changes

lpettyjo added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels May 11, 2023