Skip to content
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

[OSDOCS-4880]: [feature] ALB on OSD/ROSA #62273

Merged
merged 1 commit into from
Sep 29, 2023
Merged

[OSDOCS-4880]: [feature] ALB on OSD/ROSA #62273

merged 1 commit into from
Sep 29, 2023

Conversation

mletalie
Copy link
Contributor

@mletalie mletalie commented Jul 11, 2023 •
edited
Loading

Version(s):

4.13+
Issue:

https://issues.redhat.com/browse/OSDOCS-4880
Link to docs preview:

https://62273--docspreview.netlify.app/openshift-rosa/latest/networking/aws-load-balancer-operator
QE review:

  • QE has approved this change.

Additional information:

@mletalie mletalie marked this pull request as draft July 11, 2023 20:15
@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 11, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Jul 11, 2023 •
edited
Loading

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

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

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2023
@openshift-ci openshift-ci bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 17, 2023
@mletalie mletalie force-pushed the OSDOCS-4880 branch 2 times, most recently from 499ac95 to 91b6911 Compare August 3, 2023 12:25
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 3, 2023
@mletalie
Copy link
Contributor Author

mletalie commented Aug 11, 2023 •
edited
Loading

Hello @bchandra-ocp,
Please review the opening title/paragraph, prerequisites and note as shown in the image below. Please let me know if any additional information needs to be added, or altered in any way. At this point not focusing as much on grammar/style as I am the factual accuracy of the content. Given the size/volume of the info in this one ticket, reviews will need to be done in stages. Please add comments/suggestions in the "Files changed" section of this PR here: https://github.com/openshift/openshift-docs/pull/62273/files.

Thanks.
https://62273--docspreview.netlify.app/openshift-rosa/latest/networking/aws-load-balancer

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 14, 2023
@mletalie mletalie force-pushed the OSDOCS-4880 branch 2 times, most recently from 1749959 to 8c9ef4f Compare August 15, 2023 20:51
@bchandra-ocp
Copy link

The AWS Load Balancer (ALB) Operator is a Red-Hat managed Operator

Red-Hat supported Operator?
because, we don't manage it. we only support it for bugs etc.

@mletalie
Copy link
Contributor Author

Need review for partial docs as highlighted in the attached image and can be found in the preview link. Please add comments in the 'Files Changed' section of this PR: https://github.com/openshift/openshift-docs/pull/62273/files

Preview link: https://62273--docspreview.netlify.app/openshift-rosa/latest/networking/aws-load-balancer

**Right now I am trying to get feedback to whether the information provided is factually correct/the reader will be able to follow the steps to accomplish the goal. Formatting/style will be addressed ASAP. Please ignore the section immediately following "For more information on adding tags to AWS resources (including VPCs and subnets), see Tag your Amazon EC2 resources." That is just a placeholder for me at this point. **

image
image

bchandra-ocp
bchandra-ocp suggested changes Aug 17, 2023
View reviewed changes
networking/aws-load-balancer.adoc Outdated Show resolved Hide resolved
modules/aws-load-balancer-operator-install.adoc Outdated

[NOTE]
====
When installing for use in an AWS Local Zone, the Local Zone is enabled for the account and the ALB Operator is available within the Local Zone/region.
Copy link

Choose a reason for hiding this comment

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

It is not "the ALB Operator is available within the ...".
It is "the Application Load Balancer (ALB) is an available feature within the Local Zone"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not "the ALB Operator is available within the ...". It is "the Application Load Balancer (ALB) is an available feature within the Local Zone"

Ack. Changed.

modules/aws-load-balancer-operator-install.adoc Outdated

.Procedure

. Specify the cluster infrastructure ID and the cluster OpenID Connect (OIDC) DNS using the following commands:
Copy link

Choose a reason for hiding this comment

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

Specify is an odd word for this. Here, customer is identifying these properties. Perhaps, we use 'Identify'?

Copy link
Contributor Author

@mletalie mletalie Aug 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Specify is an odd word for this. Here, customer is identifying these properties. Perhaps, we use 'Identify'?

Ack. Changed. (sidenote: I was using "Specify" based on an existing doc in our doc set, but I think "Identify" is better. Will see how it holds up going through review process).

modules/aws-load-balancer-operator-install.adoc Outdated Show resolved Hide resolved
modules/aws-load-balancer-operator-install.adoc Outdated Show resolved Hide resolved
modules/aws-load-balancer-operator-install.adoc Outdated Show resolved Hide resolved
@mletalie
Copy link
Contributor Author

mletalie commented Aug 21, 2023 •
edited
Loading

Hello @bchandra-ocp,
Thank you for the review you have done so far. I made changes based on your feedback. I have added steps, and wanted to see if you could take a look at that new info (as seen in accompanying screenshot). I am not sure about several of the steps, but tried to recreate them as best as I could based on the info in the ALBO Features spreadsheet. As mentioned above, I will most likely separate these steps in different modules per your suggestion, but want to make sure the steps overall are correct first. Thanks.
image
CC: @AndrewJones-RH

lihongan
lihongan reviewed Aug 23, 2023
View reviewed changes
modules/aws-load-balancer-operator-install.adoc Outdated
----
<1> Replace <aws-load-balancer-operator-role-arn>' with the AWS IAM role created in step 2a.
+
. Create the Red Hat-supported AWS Load Balancer (ALB) Operator:
Copy link

Choose a reason for hiding this comment

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

BTW, user can install ALBO via Console>OperatorHub as well. see https://docs.openshift.com/container-platform/4.13/networking/aws_load_balancer_operator/install-aws-load-balancer-operator.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lihongan,
Hmmmm....okay, I may have been confused on that issue... I thought for STS clusters we could not use the install option in the Operator Hub? Maybe I misinterpreted. @bchandra-ocp, Can you confirm that we can (or cannot) use the OperatorHub in this case (to install the ALBO on an STS cluster)?
image

Copy link

Choose a reason for hiding this comment

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

Just the prerequisite is different for non-STS and STS cluster. Fon non-STS cluster user should follow the prerequisite in above page and create credentialrequest, then the secret will be provisioned automatically.
For STS cluster user need to prepare the secret manually, and it is already mentioned here before this step so user can continue the installation from OperoatorHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just the prerequisite is different for non-STS and STS cluster. Fon non-STS cluster user should follow the prerequisite in above page and create credentialrequest, then the secret will be provisioned automatically. For STS cluster user need to prepare the secret manually, and it is already mentioned here before this step so user can continue the installation from OperoatorHub.

I am still confused about how the OperatorHub can be used for installing the ALBO based on this prerequisite:
image
If a user has "An existing Red Hat OpenShift Service on AWS cluster with BYO-VPC installed in STS mode", can they or can they not use the OperatorHub to install the ALBO. If they cannot, then should the OperatorHub be mentioned in this doc? I wouldn't think so. Please see this comment (which leads me to believe the OperatorHub does not apply to this doc). #62273 (comment).

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
{product-title} (ROSA)
endif::openshift-rosa[]
cluster with bring-your-own-VPC (BYO-VPC) configuration across multiple availability zones (AZ) installed in STS mode.
* You have access to the cluster as a user with the `dedicated-admin` role.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a non-ROSA step to have cluster-admin access?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was told by SRE that we should not include cluster-admin access in the doc.

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
ifdef::openshift-rosa[]
ROSA
endif::openshift-rosa[]
cluster as a user with the `dedicated-admin` role and create a new project using the following command:
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-ROSA step to use cluster-admin access?

Copy link
Contributor Author

@mletalie mletalie Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah...I see why you are asking this (I think). Originally, this feature was for ROSA and OSD....was told late in the game that it does not work for OSD...only for ROSA. IF that will change at some point, not sure.

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
$ OPERATOR_ROLE_ARN=$(aws iam get-role --role-name albo-operator --output json | jq -r '.Role.Arn')
$ echo $OPERATOR_ROLE_ARN
----
For more information on creating AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create.html[Creating IAM roles].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For more information on creating AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create.html[Creating IAM roles].
+
For more information on creating AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create.html[Creating IAM roles].

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
$ curl -o albo-operator-permission-policy.json https://raw.githubusercontent.com/alebedev87/aws-load-balancer-operator/aws-cli-commands-for-sts/hack/operator-permission-policy.json
aws iam put-role-policy --role-name albo-operator --policy-name perms-policy-albo-operator --policy-document file://albo-operator-permission-policy.json
----
For more information on adding AWS IAM permissions to AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_manage-attach-detach.html[Adding and removing IAM identity permissions].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For more information on adding AWS IAM permissions to AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_manage-attach-detach.html[Adding and removing IAM identity permissions].
+
For more information on adding AWS IAM permissions to AWS IAM roles, see link:https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_manage-attach-detach.html[Adding and removing IAM identity permissions].

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc Outdated
EOF
----
+

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc Outdated
----
+

For more information regarding formatting credentials files, see link:https://access.redhat.com/documentation/en-us/openshift_container_platform/4.13/html/authentication_and_authorization/managing-cloud-provider-credentials#cco-mode-sts[Using manual mode with Amazon Web Services Security Token Service].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For more information regarding formatting credentials files, see link:https://access.redhat.com/documentation/en-us/openshift_container_platform/4.13/html/authentication_and_authorization/managing-cloud-provider-credentials#cco-mode-sts[Using manual mode with Amazon Web Services Security Token Service].
For more information about formatting credentials files, see link:https://access.redhat.com/documentation/en-us/openshift_container_platform/4.13/html/authentication_and_authorization/managing-cloud-provider-credentials#cco-mode-sts[Using manual mode with Amazon Web Services Security Token Service].

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
For more information regarding formatting credentials files, see link:https://access.redhat.com/documentation/en-us/openshift_container_platform/4.13/html/authentication_and_authorization/managing-cloud-provider-credentials#cco-mode-sts[Using manual mode with Amazon Web Services Security Token Service].


.. Create the operator's credentials secret with the generated AWS credentials:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.. Create the operator's credentials secret with the generated AWS credentials:
+
.. Create the operator's credentials secret with the generated AWS credentials:

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc Outdated
Comment on lines 178 to 179


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

mburke5678
mburke5678 reviewed Sep 29, 2023
View reviewed changes
modules/aws-installing-an-aws-load-balancer-operator.adoc
----
$ oc -n aws-load-balancer-operator create secret generic aws-load-balancer-controller-cluster --from-file=credentials=albo-controller-aws-credentials.cfg
----
+
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+

@mburke5678 mburke5678 removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Sep 29, 2023
@mletalie mletalie force-pushed the OSDOCS-4880 branch 2 times, most recently from dcdba60 to 7b7c7d1 Compare September 29, 2023 14:25
@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Sep 29, 2023
@mletalie
ALB on OSD/ROSA
aa745af 
	modified:   modules/aws-load-balancer-operator-install.adoc
@jldohmann jldohmann added merge-review-in-progress Signifies that the merge review team is reviewing this PR branch/enterprise-4.13 branch/enterprise-4.14 labels Sep 29, 2023
@jldohmann jldohmann added this to the Continuous Release milestone Sep 29, 2023
@jldohmann jldohmann merged commit 7f004b3 into openshift:main Sep 29, 2023
@jldohmann
Copy link
Contributor

/cherrypick enterprise-4.14

@jldohmann
Copy link
Contributor

/cherrypick enterprise-4.13

@jldohmann jldohmann removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Sep 29, 2023
@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Sep 29, 2023
Merged
@openshift-cherrypick-robot
Copy link

@jldohmann: new pull request created: #65534

In response to this:

/cherrypick enterprise-4.14

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 openshift-cherrypick-robot mentioned this pull request Sep 29, 2023
Merged
@openshift-cherrypick-robot
Copy link

@jldohmann: new pull request created: #65535

In response to this:

/cherrypick 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.

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

@bchandra-ocp bchandra-ocp bchandra-ocp requested changes

@jneczypor jneczypor jneczypor left review comments

@bburt-rh bburt-rh bburt-rh left review comments

@mburke5678 mburke5678 mburke5678 left review comments

@lihongan lihongan lihongan left review comments

@alebedev87 alebedev87 alebedev87 left review comments

@jldohmann jldohmann jldohmann approved these changes

Assignees
No one assigned
Labels
branch/enterprise-4.13 branch/enterprise-4.14 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
Continuous Release
Development

Successfully merging this pull request may close these issues.