Skip to content

OSDOCS#9377: AWS Load Balancer Operator: Content Improvement#70387

Merged
mburke5678 merged 1 commit intoopenshift:mainfrom
xenolinux:ALBO-improv
Apr 2, 2024
Merged

OSDOCS#9377: AWS Load Balancer Operator: Content Improvement#70387
mburke5678 merged 1 commit intoopenshift:mainfrom
xenolinux:ALBO-improv

Conversation

@xenolinux
Copy link
Contributor

@xenolinux xenolinux commented Jan 17, 2024

@xenolinux xenolinux changed the title OSDOCS#9377: AWS Load Balancer Operator: Content Improvement [WIP] OSDOCS#9377: AWS Load Balancer Operator: Content Improvement Jan 17, 2024
@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/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 17, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Jan 17, 2024

🤖 Tue Apr 02 07:47:33 - Prow CI generated the docs preview:
https://70387--ocpdocs-pr.netlify.app

@xenolinux xenolinux changed the title [WIP] OSDOCS#9377: AWS Load Balancer Operator: Content Improvement OSDOCS#9377: AWS Load Balancer Operator: Content Improvement Mar 8, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 8, 2024
@xenolinux
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 8, 2024
@opayne1 opayne1 added peer-review-in-progress Signifies that the peer review team is reviewing this PR branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 8, 2024
@opayne1 opayne1 added this to the Continuous Release milestone Mar 8, 2024
Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

Sorry again for the late review!! I had just a couple thoughts to consider, but overall looks great! I personally would still get a QE sign off because there are changes to some of the terminology or removes some technical terminology.

<3> The Amazon Resource Name of the certificate that you attach to the load balancer.
<4> Defines the ingress class name.
<1> Specifies the Ingress name.
<2> The controller provisions the load balancer for Ingress in a public subnet to access load balancer over the internet.
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
<2> The controller provisions the load balancer for Ingress in a public subnet to access load balancer over the internet.
<2> The controller provisions the load balancer for Ingress in a public subnet to access the load balancer over the internet.

I am thinking maybe there should be a "the" before load balancer, but then you have another "the" before internet so I am not sure. Just a thought.

. Create a `service` resource:
. Create a YAML file that defines the `Service` resource:
+
.Example `service-albo.yaml` file
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
.Example `service-albo.yaml` file
.Example `service-albo.yaml` file:

Copy link
Contributor

Choose a reason for hiding this comment

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

@opayne1 @xenolinux I don't think we need a colon here, right? This is a heading.

. Deploy an ALB-backed `Ingress` resource:
. Create a YAML file that defines the `Ingress` resource:
+
.Example `ingress-albo.yaml` file
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
.Example `ingress-albo.yaml` file
.Example `ingress-albo.yaml` file:

Copy link
Contributor

Choose a reason for hiding this comment

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

@opayne1 @xenolinux I don't think we need a colon here, right? This is a heading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mburke5678 That's right. Removed the colon

<1> Defines the ingress resource.
<2> Specifies the name of the ingress resource.
<3> Specifies the name of the service resource.
<2> Specifies the ingress name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Ingress supposed to be capital? I saw some instances where you made the lowercase capital and didn't know if that applies here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused. This definition says, and its also my understanding, that when referring to an Ingress resource or the Ingress Controller, it should be capitalized and when describing traffic direction, its lowercase.

. Create an `AWSLoadBalancerController` resource file named `example-sts-iam-role.yaml` with contents such as the following example:
. Create a YAML file that defines the `AWSLoadBalancerController` object:
+
Example `sample-aws-lb-manual-creds.yaml` file
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
Example `sample-aws-lb-manual-creds.yaml` file
Example `sample-aws-lb-manual-creds.yaml` file:

Copy link
Contributor

Choose a reason for hiding this comment

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

@opayne1 @xenolinux I don't think we need a colon here, right? This is a heading.

toc::[]

You can route the traffic to different services that are part of a single domain through a single AWS Load Balancer (ALB). Each Ingress resource provides different endpoints of the domain.
You can route the traffic to different services that are part of a single domain through a single AWS Load Balancer. Each Ingress provides different endpoints of the domain.
Copy link
Contributor

Choose a reason for hiding this comment

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

I almost feel like resource should be left in here or add something else like "class" or "instance" because it specifies Ingress. Just a thought.

@opayne1 opayne1 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 Mar 12, 2024
@xenolinux xenolinux force-pushed the ALBO-improv branch 3 times, most recently from cdbd7e3 to 2755673 Compare March 13, 2024 06:52
:_mod-docs-content-type: PROCEDURE
[id="using-aws-cli-create-iam-role-alb-controller_{context}"]
= Using the AWS CLI to create an IAM role for the Controller
= Creating an AWS IAM role for the AWS Load Balancer Controller

Choose a reason for hiding this comment

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

If change to this, then we have two same subtitle of Creating an AWS IAM role for the AWS Load Balancer Controller

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the title

:_mod-docs-content-type: PROCEDURE
[id="using-ccoctl-create-iam-role-alb-controller_{context}"]
= Using ccoctl to create an IAM role for the Controller
= Creating an AWS IAM role for the AWS Load Balancer Controller

Choose a reason for hiding this comment

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

Here is another same title

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed. Modified the title

* Using the `ccoctl` binary and a predefined `CredentialsRequest` object.
* Using the AWS CLI and predefined AWS manifests.

Use the AWS CLI if your environment does not support the `ccoctl` command.

Choose a reason for hiding this comment

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

maybe we could add link to ccoctl utility https://docs.openshift.com/container-platform/4.13/authentication/managing_cloud_provider_credentials/cco-mode-sts.html#cco-ccoctl-configuring_cco-mode-sts

Because in that page we could see the note that only Linux supports ccoctl.

The ccoctl utility is a Linux binary that must run in a Linux environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@xenolinux xenolinux force-pushed the ALBO-improv branch 2 times, most recently from b38ba0c to 41fcff0 Compare March 13, 2024 09:42
@lihongan
Copy link

LGTM. thank you!

@xenolinux
Copy link
Contributor Author

/remove-label merge-review-in-progress

@openshift-ci openshift-ci bot removed the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Apr 1, 2024
@mburke5678 mburke5678 added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Apr 1, 2024
.Procedure

. You can deploy the AWS Load Balancer Operator on demand from the OperatorHub, by creating a `Subscription` object:
. You can deploy the AWS Load Balancer Operator on-demand from OperatorHub, by creating a `Subscription` object by running 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.

Per ISG, on-demand (with hyphen) is an adjective; on demand (no hyphen) is an adverb. Here, on-demand refers to deploy, and is an adverb.

Suggested change
. You can deploy the AWS Load Balancer Operator on-demand from OperatorHub, by creating a `Subscription` object by running the following command:
. You can deploy the AWS Load Balancer Operator on demand from OperatorHub, by creating a `Subscription` object by running the following command:

I'm not sure if the following makes sense, but here is the comparision:

Suggested change
. You can deploy the AWS Load Balancer Operator on-demand from OperatorHub, by creating a `Subscription` object by running the following command:
. You can deploy the on-demand AWS Load Balancer Operator from OperatorHub, by creating a `Subscription` object by running the following command:

.Procedure

. You can deploy the AWS Load Balancer Operator on demand from the OperatorHub, by creating a `Subscription` object:
. You can deploy the AWS Load Balancer Operator on demand from OperatorHub, by creating a `Subscription` object by running the following command:
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤖 [error] RedHat.TermsErrors: Use 'on-demand' rather than 'on demand'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this rule might need an update to stop false positives 🤔

= Creating an AWS IAM role by using the Cloud Credential Operator utility

You can use the `aws` command line interface to create an IAM role for the AWS Load Balancer (ALB) Operator. The created IAM role is used to interact with subnets and Virtual Private Clouds (VPCs).
You can use the AWS command line interface to create an IAM role for the AWS Load Balancer Operator. The IAM role is used to interact with subnets and Virtual Private Clouds (VPCs).
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
You can use the AWS command line interface to create an IAM role for the AWS Load Balancer Operator. The IAM role is used to interact with subnets and Virtual Private Clouds (VPCs).
You can use the AWS Command Line Interface to create an IAM role for the AWS Load Balancer Operator. The IAM role is used to interact with subnets and Virtual Private Clouds (VPCs).

.Prerequisites

* You must have access to the `aws` command line interface.
* You must have access to the AWS command line interface (`aws`).
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
* You must have access to the AWS command line interface (`aws`).
* You must have access to the AWS Command Line Interface (`aws`).

.Procedure

. You can deploy the AWS Load Balancer Operator on demand from the OperatorHub, by creating a `Subscription` object:
. You can deploy the AWS Load Balancer Operator on demand from OperatorHub, by creating a `Subscription` object by running the following command:
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤖 [error] RedHat.TermsErrors: Use 'on-demand' rather than 'on demand'.

@openshift-ci
Copy link

openshift-ci bot commented Apr 2, 2024

@xenolinux: all tests passed!

Full PR test history. Your PR dashboard.

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.

@mburke5678 mburke5678 merged commit ba46908 into openshift:main Apr 2, 2024
@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.12

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.13

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.14

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.15

@mburke5678
Copy link
Contributor

/cherrypick enterprise-4.16

@openshift-cherrypick-robot

@mburke5678: #70387 failed to apply on top of branch "enterprise-4.12":

Applying: OSDOCS#9377: AWS Load Balancer Operator: Content Improvement
Using index info to reconstruct a base tree...
M	_topic_maps/_topic_map.yml
M	modules/nw-aws-load-balancer-operator.adoc
A	modules/specifying-role-arn-albo-sts.adoc
A	modules/using-aws-cli-create-iam-role-alb-controller.adoc
A	modules/using-aws-cli-create-iam-role-alb-operator.adoc
A	modules/using-ccoctl-create-iam-role-alb-controller.adoc
A	modules/using-ccoctl-create-iam-role-alb-operator.adoc
M	networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
M	networking/aws_load_balancer_operator/understanding-aws-load-balancer-operator.adoc
Falling back to patching base and 3-way merge...
Auto-merging networking/aws_load_balancer_operator/understanding-aws-load-balancer-operator.adoc
Auto-merging networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
CONFLICT (content): Merge conflict in networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
CONFLICT (modify/delete): modules/using-ccoctl-create-iam-role-alb-operator.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-ccoctl-create-iam-role-alb-operator.adoc left in tree.
CONFLICT (modify/delete): modules/using-ccoctl-create-iam-role-alb-controller.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-ccoctl-create-iam-role-alb-controller.adoc left in tree.
CONFLICT (modify/delete): modules/using-aws-cli-create-iam-role-alb-operator.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-aws-cli-create-iam-role-alb-operator.adoc left in tree.
CONFLICT (modify/delete): modules/using-aws-cli-create-iam-role-alb-controller.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-aws-cli-create-iam-role-alb-controller.adoc left in tree.
CONFLICT (modify/delete): modules/specifying-role-arn-albo-sts.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/specifying-role-arn-albo-sts.adoc left in tree.
Auto-merging modules/nw-aws-load-balancer-operator.adoc
Auto-merging _topic_maps/_topic_map.yml
CONFLICT (content): Merge conflict in _topic_maps/_topic_map.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSDOCS#9377: AWS Load Balancer Operator: Content Improvement
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.12

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

@mburke5678: #70387 failed to apply on top of branch "enterprise-4.13":

Applying: OSDOCS#9377: AWS Load Balancer Operator: Content Improvement
Using index info to reconstruct a base tree...
M	_topic_maps/_topic_map.yml
M	modules/nw-aws-load-balancer-operator.adoc
A	modules/specifying-role-arn-albo-sts.adoc
A	modules/using-aws-cli-create-iam-role-alb-controller.adoc
A	modules/using-aws-cli-create-iam-role-alb-operator.adoc
A	modules/using-ccoctl-create-iam-role-alb-controller.adoc
A	modules/using-ccoctl-create-iam-role-alb-operator.adoc
M	networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
M	networking/aws_load_balancer_operator/understanding-aws-load-balancer-operator.adoc
Falling back to patching base and 3-way merge...
Auto-merging networking/aws_load_balancer_operator/understanding-aws-load-balancer-operator.adoc
Auto-merging networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
CONFLICT (content): Merge conflict in networking/aws_load_balancer_operator/installing-albo-sts-cluster.adoc
CONFLICT (modify/delete): modules/using-ccoctl-create-iam-role-alb-operator.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-ccoctl-create-iam-role-alb-operator.adoc left in tree.
CONFLICT (modify/delete): modules/using-ccoctl-create-iam-role-alb-controller.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-ccoctl-create-iam-role-alb-controller.adoc left in tree.
CONFLICT (modify/delete): modules/using-aws-cli-create-iam-role-alb-operator.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-aws-cli-create-iam-role-alb-operator.adoc left in tree.
CONFLICT (modify/delete): modules/using-aws-cli-create-iam-role-alb-controller.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/using-aws-cli-create-iam-role-alb-controller.adoc left in tree.
CONFLICT (modify/delete): modules/specifying-role-arn-albo-sts.adoc deleted in HEAD and modified in OSDOCS#9377: AWS Load Balancer Operator: Content Improvement. Version OSDOCS#9377: AWS Load Balancer Operator: Content Improvement of modules/specifying-role-arn-albo-sts.adoc left in tree.
Auto-merging modules/nw-aws-load-balancer-operator.adoc
Auto-merging _topic_maps/_topic_map.yml
CONFLICT (content): Merge conflict in _topic_maps/_topic_map.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSDOCS#9377: AWS Load Balancer Operator: Content Improvement
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

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.

@openshift-cherrypick-robot

@mburke5678: new pull request created: #74036

Details

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

@mburke5678: new pull request created: #74037

Details

In response to this:

/cherrypick enterprise-4.15

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

@mburke5678: new pull request created: #74038

Details

In response to this:

/cherrypick enterprise-4.16

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.

@mburke5678
Copy link
Contributor

@xenolinux Can you PTAL at the 4.12 and 4.13 cherrypicks? There are some significant differences in those branches.

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

Labels

branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 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 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

Development

Successfully merging this pull request may close these issues.

9 participants