Skip to content

Bl/adding seclistmode documentation #226

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.

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

Merged
merged 5 commits into from
Aug 7, 2018

Conversation

bdourallawzi
Copy link
Contributor

Modified documentation to the load balancer annotation doc on configuring the security list management mode.
Added a tutorial and example.
Covers documentation for the following commit: #225

Copy link
Contributor

@prydie prydie left a comment

Choose a reason for hiding this comment

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

I suggest removing the tutorial and reworking the information about the security list manager mode options into the annotations doc.

@@ -27,6 +27,7 @@ spec:
| `oci-load-balancer-subnet1` | The OCID of the first [subnet][2] of the two required subnets to attach the load balancer to. Must be in separate Availability Domains. | Value provided in config file |
| `oci-load-balancer-subnet2` | The OCID of the second [subnet][2] of the two required subnets to attach the load balancer to. Must be in separate Availability Domains. | Value provided in config file |
| `oci-load-balancer-connection-idle-timeout` | The maximum idle time, in seconds, allowed between two successive receive or two successive send operations between the client and backend servers. | `300` for TCP listeners, `60` for HTTP listeners |
| `oci-load-balancer-security-list-management-mode` | Specifies the security list mode ("All", "Frontend", "None") to configure how security lists are managed by the CCM | `"All"`
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fullstop

ports:
- name: http
port: 80
targetPort: 80
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: trailing newline

(you probably need to configure "files.insertFinalNewline": true" in vscode)

@@ -0,0 +1,93 @@
# Tutorial
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 not sure we need a tutorial specifically for the annotation, however, the possible annotation options and their effects certainly need to be documented front and centre.

Copy link
Contributor

@prydie prydie left a comment

Choose a reason for hiding this comment

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

Nearly there. The bottom of the table doesn't render nicely as HTML though unfortunately (see: here).

@@ -27,6 +27,7 @@ spec:
| `oci-load-balancer-subnet1` | The OCID of the first [subnet][2] of the two required subnets to attach the load balancer to. Must be in separate Availability Domains. | Value provided in config file |
| `oci-load-balancer-subnet2` | The OCID of the second [subnet][2] of the two required subnets to attach the load balancer to. Must be in separate Availability Domains. | Value provided in config file |
| `oci-load-balancer-connection-idle-timeout` | The maximum idle time, in seconds, allowed between two successive receive or two successive send operations between the client and backend servers. | `300` for TCP listeners, `60` for HTTP listeners |
| `oci-load-balancer-security-list-management-mode` | Specifies the [security list mode](##security-list-management-modes) (`"All"`, `"Frontend"`,`"None"`) to configure how security lists are managed by the CCM. | `"All"`
Copy link
Contributor Author

@bdourallawzi bdourallawzi Aug 3, 2018

Choose a reason for hiding this comment

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

I modified the markdown format. I am trying to link the security list management mode to the heading below but doesn't seem to work. :( Might be just GitHub.

Copy link
Member

@owainlewis owainlewis left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

@prydie prydie merged commit 0c1466e into master Aug 7, 2018
@prydie prydie deleted the bl/adding-seclistmode-documentation branch August 7, 2018 14:02
ayushverma14 pushed a commit to ayushverma14/oci-cloud-controller-manager that referenced this pull request Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants