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.

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

NC | Upstream Docs Refactoring #8162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

romayalon
Copy link
Contributor

@romayalon romayalon commented Jun 24, 2024

Explain the changes

  • Created the following docs for NooBaa Non Containerized -
  1. Getting started
  2. Configuration
  3. NooBaa CLI
  4. Health
  5. Logging
  6. Monitoring
  7. Troubleshooting
  8. Supported S3 operations
  9. Limitations
  10. CI & Tests
  11. Upgrade
  • Updated formatting of config file customizations doc.

Issues: Fixed #xxx / Gap #xxx

  1. Delete the old non_containerized_nsfs doc.

Testing Instructions:

  • Doc added/updated
  • Tests added

Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

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

A few comments in the files:

  1. Index
  2. Getting Started
  3. Configuration

docs/NooBaaNonContainerized/GettingStarted.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/GettingStarted.md Outdated Show resolved Hide resolved

## NooBaa Non Containerized Solution

NooBaa Non Containerized is a lightweight deployment of NooBaa on Linux without depending on Kubernetes. NooBaa Non Containerized version includes NooBaa service that deploys S3 endpoints for handling S3 requests, the NooBaa CLI for managing accounts & buckets, and NooBaa Health CLI for performing system health analysis.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove the word version

NooBaa Non Containerized version includes NooBaa service that deploys S3 endpoints

I think we can call them just endpoints (instead of "S3 endpoints")

deploys S3 endpoints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the version but kept the S3, I want to make it clear for everyone the the endpoints are serving s3 requests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but in the next versions, it will be also for STS and IAM requests.

docs/NooBaaNonContainerized/GettingStarted.md Show resolved Hide resolved

NooBaa Non Containerized solution includes the following components -
1. [NooBaa Service](#enable-noobaa-service) - Deploys NooBaa S3 endpoint for handling S3 requests.
2. [NooBaa CLI](./NooBaaCLI.md) - Accounts and exported buckets management.
Copy link
Contributor

Choose a reason for hiding this comment

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

What are "exported buckets"?

docs/NooBaaNonContainerized/Configuration.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/Configuration.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/Configuration.md Show resolved Hide resolved
docs/NooBaaNonContainerized/Configuration.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/GettingStarted.md Outdated Show resolved Hide resolved
Example -
```sh
// 1. Create the storage underlying directory
// 2. Apply 777 permission to the directory (insecure)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add the "secure" step to do it?
And if not explain exactly what we did as bypass.

docs/NooBaaNonContainerized/Configuration.md Show resolved Hide resolved
* Configuration files generated under the `accounts/` or `buckets/` directories will have 600 permissions, granting read and write access exclusively to the owner of each configuration file.

Ownership
* Configuration file created by the NooBaa CLI tool will be owned by the user who ran the NooBaa CLI command.
Copy link
Contributor

Choose a reason for hiding this comment

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

yes but we need to remove this limitation

I didn't understand that, what is the plan?


`certificates/`
* <u>Type</u>: Directory.
* <u>Required</u>: False.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also remove the dot of those keys ("No.", "File."), for a cleaner look.

Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

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

ConfigFileCustomizations comments


## Introduction
A user can customize NooBaa Non Containerized by creation of config.json file under their config directory.
When a node is designated in the host_customization, Noobaa will combine the shared configuration with the node's configuration. If a configuration value is provided under the node's configuration, it will take precedence as the final configuration value applied to NooBaa service on that specific node.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure the use of "is designated" is clear (in "When a node is designated in the host_customization").

Comment on lines +8 to +10
## Introduction
A user can customize NooBaa Non Containerized by creation of config.json file under their config directory.
When a node is designated in the host_customization, Noobaa will combine the shared configuration with the node's configuration. If a configuration value is provided under the node's configuration, it will take precedence as the final configuration value applied to NooBaa service on that specific node.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should take a step and explain the 2 options that we have in the config file:

  1. shared configuration
  2. node/host configuration
    It seems that it jumps directly to the logic between them.

* <u>Key</u>: `ENDPOINT_FORKS`
* <u>Type</u>: Number
* <u>Default</u>: 0
* <u>Description</u>: Adjust the number of forks in order to increase the S3 endpoints number. This will enable NooBaa to handle a higher scale of S3 requests concurrently.
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we explain under the hood how the requests are handled between the endpoints?

Comment on lines +54 to +57
1. ENDPOINT_PORT - S3 HTTP port
2. ENDPOINT_SSL_PORT - S3 HTTPS port
3. ENDPOINT_SSL_STS_PORT - STS HTTPS port
4. EP_METRICS_SERVER_PORT - Prometheus metrics port
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a code line in each?

Suggested change
1. ENDPOINT_PORT - S3 HTTP port
2. ENDPOINT_SSL_PORT - S3 HTTPS port
3. ENDPOINT_SSL_STS_PORT - STS HTTPS port
4. EP_METRICS_SERVER_PORT - Prometheus metrics port
1. `ENDPOINT_PORT` - S3 HTTP port
2. `ENDPOINT_SSL_PORT` - S3 HTTPS port
3. `ENDPOINT_SSL_STS_PORT` - STS HTTPS port
4. `EP_METRICS_SERVER_PORT` - Prometheus metrics port

Comment on lines +54 to +57
1. ENDPOINT_PORT - S3 HTTP port
2. ENDPOINT_SSL_PORT - S3 HTTPS port
3. ENDPOINT_SSL_STS_PORT - STS HTTPS port
4. EP_METRICS_SERVER_PORT - Prometheus metrics port
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we use Prometheus in NC NSFS? You can add a link.

* <u>Key</u>: `ENDPOINT_PROCESS_TITLE`
* <u>Type</u>: String
* <u>Default</u>: 'noobaa'
* <u>Description</u>: This flag will set NooBaa process title for letting GPFS to identify the NooBaa endpoint processes. See issue #8039.
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to link the issue in GitHub.

Suggested change
* <u>Description</u>: This flag will set NooBaa process title for letting GPFS to identify the NooBaa endpoint processes. See issue #8039.
* <u>Description</u>: This flag will set NooBaa process title for letting GPFS identify the NooBaa endpoint processes. See issue [#8039](https://github.com/noobaa/noobaa-core/issues/8039).

* <u>Key</u>: `GPFS_DOWN_DELAY`
* <u>Type</u>: Number
* <u>Default</u>: 1000
* <u>Description</u> Set delay (ms) of GPFS system calls when daemon is down, to hold client replies during failover, service restart required.
Copy link
Contributor

Choose a reason for hiding this comment

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

"during failover" - maybe fail over? failure?

* <u>Key</u>: `LOG_TO_SYSLOG_ENABLED`
* <u>Type</u>: Boolean
* <u>Default</u>: true
* <u>Description</u>: This flag will enable syslog logging for the application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we refer to more information about the syslog logging?

## Config.json File Examples
The following is an example of a config.json file -

```sh
Copy link
Contributor

Choose a reason for hiding this comment

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

We can show the JSON with a JSON style ("```json").
Like in AWS CLI examples (for example in put-bucket-policy).

Copy link
Contributor

Choose a reason for hiding this comment

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

General on this file:

  • There are places where we write "service restart required" and add it in the step, and places where we just add the step, so we need to fix it. Can we add a "thumb rule" in which cases will we restart the service?
  • They're places where we add the bytes example in different styles:
    67108864 // 64MB = 64 * 1024 * 1024 vs 32MB = 33554432 bytes = 32 * 1024 * 1024 (I prefer the first one).
  • I would add to the number type the measurement explicitly and not just in the description (bytes, milliseconds, etc.).

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.

None yet

2 participants