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

Update documentation for Operator #219

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Update documentation for Operator #219

merged 1 commit into from
Aug 31, 2020

Conversation

nitisht
Copy link
Contributor

@nitisht nitisht commented Jul 29, 2020

Depends on #188

@nitisht nitisht marked this pull request as ready for review July 29, 2020 15:10
Copy link
Contributor

@ravindk89 ravindk89 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 areas I think we can tighten up. Eventually I need to migrate this content, but until then this looks like a good start.

Main point is making sure we're using examples consistently. SO if we have a quickstart, then examples for modifying/expanding should assume that quickstart as the 'base'.

README.md Outdated Show resolved Hide resolved

## Getting Started
## Steps to Run

Copy link
Contributor

Choose a reason for hiding this comment

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

Should state explicitly up here what this command does:

The following command creates a distributed MinIO deployment with 4 MinIO ``servers``. Each MinIO server consists of four ``volumes`` with a ``capacity`` of 1Ti each. 

You can follow up noting something like

To modify the number of servers, volumes, or capacity per volume, change the values for ``servers``, ``volumes``, and
``capacity`` respectively. 

- MinIO Erasure Coding relies on at least 4 drives across all MinIO instances in the distributed system. If the value of 
  ``servers X volumes`` is less than ``4``, MinIO cannot enable erasure coding on the deployment.

- The MinIO operator cannot expand standalone deployments (``servers 1``) to distributed deployments.

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot to ask this during the last engineering meeting - I remember vaguely we had some docs noting that we cannot expand from standalone to distributed. Maybe that's in my imagination though - I don't see it anymore (perhaps its no longer a concern). If so can scratch the last bullet.

kubectl minio operator create
kubectl create ns tenant1-ns
kubectl create secret generic minio-secret --from-literal=accesskey=minio --from-literal=secretkey=minio123 --namespace tenant1-ns
kubectl minio tenant create --name tenant1 --secret minio-secret --servers 4 --volumes 4 --capacity 1Ti --namespace tenant1-ns
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Should add connection information.

"To connect to the created cluster, add the cluster to your mc configuration and perform operations on it:

mc alias set tenant1-ns https://<service hostname>:9000  minio minio123

mc admin info

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 assuming we auto-create the service for them - if we can provide guidance on what the resulting URL will be, we should include that to make this easier.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave this as a nit, but I'd still recommend we provide guidance on how to connect (if at all possible).

docs/adding-zones.md Outdated Show resolved Hide resolved
docs/adding-zones.md Outdated Show resolved Hide resolved
ravindk89
ravindk89 previously approved these changes Aug 3, 2020
Copy link
Contributor

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

I'm going to consider my feedback requests as Nits, since the goal of this document is to get the user to a working deployment. And the instructions as-is should accomplish that. My suggestions are mostly polish.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/adding-zones.md Outdated Show resolved Hide resolved
docs/adding-zones.md Outdated Show resolved Hide resolved
docs/console.md Outdated Show resolved Hide resolved
kustomization.yaml Outdated Show resolved Hide resolved
docs/adding-zones.md Outdated Show resolved Hide resolved
docs/adding-zones.md Outdated Show resolved Hide resolved
ravindk89
ravindk89 previously approved these changes Aug 31, 2020
Copy link
Contributor

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

LGTM mod one lingering optional nit

@nitisht
Copy link
Contributor Author

nitisht commented Aug 31, 2020

Nit: "with new capacity of 32Ti" might imply that the total tenant capacity is 32Ti, rather than adding 32Ti to the existing tenant. "with additional capacity of 32Ti" should be a bit more explicit.

done

@kannappanr kannappanr merged commit 97cc990 into minio:master Aug 31, 2020
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.

None yet

5 participants