-
Notifications
You must be signed in to change notification settings - Fork 83
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
Doc review #135
Doc review #135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the edits William, the text reads a lot better. It seems to me that some of the lines are out of the line character limiti (78, I think --please double-check).
The new files you added need to be discussed in a separate pull request, IMO. The file names contain spaces and that's not good. Also it's not clear to me why the directory and file names are capitalized.
Let's split the commit so that the fixes to the existing docs can go in faster, while we discuss the new files separately.
swarm-production/README.md
Outdated
persistent containers to this particular node, assign label `io.zenko.type` with | ||
value `storage` to the node: | ||
Here, we choose the host `s3-node-zenko-swarm-1.na.scality.cloud` with ID | ||
`ng8quztnef0r1x90le4d6lssj`. To ensure that Docker Swarm only schedules persistent containers to this node, assign the `io.zenko.type` label with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is not wrapped
swarm-production/README.md
Outdated
and upload anonymous stats. Zenko Orbit allows easy configuration of users, | ||
remote storage locations, replication and more, as well as instance monitoring. | ||
By default, the stack registers itself at the | ||
[Zenko Orbit](https://www.zenko.io/admin) portal and uploads anonymous stats. Zenko Orbit allows easy configuration of users, remote storage locations, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not wrapping
swarm-production/README.md
Outdated
|
||
If you would like to opt out of the remote management and monitoring, before | ||
deploying you can export this environment variable: | ||
To opt out of remote management and monitoring, export this environment variable before deployment: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is too long.
swarm-production/README.md
Outdated
the `ENDPOINT` variable configured above if applicable, or whatever the | ||
`hostname -f` command returns. | ||
> IMPORTANT: when using default port 80, it should never be specified after the | ||
> endpoint address. If using a custom port, it must be specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check wrapping settings in your editor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on these. Capitalized headings is a "book" artifact. We'll have to resolve this issue for content reuse. I can certainly un-capitalize the file names, but the headings will need to be fixed manually to one standard or the other ("up" or "down"). Either way is going to be work.
Try to do a rebase against master in the branch. It looks like you have the commits from #123 showing up in this PR as well |
Will do.
…On Fri, Jun 15, 2018 at 9:55 AM, Salim Salaues ***@***.***> wrote:
Try to do a rebase on master in the branch. It looks like you have the
commits from #123 <#123> showing up
in this PR as well
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#135 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AiJ6GHl5I_BFg56avy67H5B4PjVVd5MWks5t8-cOgaJpZM4UpzKo>
.
--
William Abernathy
Sr. Tech Writer
San Francisco
|
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
|
|
||
:: | ||
|
||
$ kubectl logs my-zenko-cloudserver-front-<id> | grep 'Instance ID' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This segment is formatted strangely. The first line is the command and the rest of the section is the example output, but by having \
appending each line, it looks like this should be one massive command that is run.
Also, we should probably clarify that the Instance ID
is the value that users are looking for and the reason why users are grabbing logs, with a quick comment that this ID will be used to register the instance on zenko.io
{%- set node = ((pv.spec.nodeAffinity.required.nodeSelectorTerms|first).matchExpressions | ||
|first)['values']|first -%} | ||
{%- set _ = released_pv.setdefault(node, []).append(pv) -%} | ||
{%- endfor -%} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least in the render mode via Github, the last line is incorrectly formatted (missing the leading tab/spaces and does not line up with the for
statement).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a persistent problem with rst. Every renderer seems to have a different take on this. I believe I've fixed this, but that depends on your renderer, I guess...
Keep an eye on this.
@@ -0,0 +1,615 @@ | |||
Introduction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an exact copy of docs/Installation/MetalK8s-CentOS8.5_install.rst
. I assume this is an accidental duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased this morning, and can't find the MetalK8s-CentOS8.5_install.rst doc anymore. I assume it's been clobbered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I see the problem. I'll clobber one of the instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This directory and file no longer exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your commits I still see a lot of commits that are already included in master
, afaik when you rebase off master those commits shouldn't show up in the PR.
3f13dd2
to
e771e91
Compare
## Testing | ||
|
||
To use the `tests` folder, update the credentials in | ||
`Zenko/tests/utils/s3SDK.js` with credentials generated in Zenko Orbit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be Zenko/tests/backbeat/s3SDK.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also theres a boatload of evironment variables that need to be set for it to run
from the bucket on which they are stored. At present, files | ||
exceeding a preset date *cannot be transitioned* to STANDARD\_IA or | ||
GLACIER storage classes. | ||
======= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything from this point seems to be duplicating the above text. Remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gitcrap. Sorry. Thought I'd got that. Fixed (again)
time has passed since the object’s creation. Zenko supports expiration of | ||
versioned or non-versioned objects, when a defined number of days has | ||
passed since those objects’ creation. This enables automatic deletion of | ||
older versions of versioned objects to reclaim storage space. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we can also expire current version of objects with separate rules, it's also supported in Orbit. (for versioned buckets, it adds a delete marker like when a user deletes the object without specifying a version ID).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Zenko from the command line or from Orbit, you can expire the
current version of an object with a separate rule. For versioned buckets, add
a delete marker on an event, such as when a user deletes an object without first
specifying a version ID.
Cloud users can apply lifecycle expiration rules (specified in Amazon’s | ||
`AWS S3 API <https://docs.aws.amazon.com/AmazonS3/latest/API/Welcome.html>`__) | ||
to buckets managed through Zenko. These rules are triggered after a defined | ||
time has passed since the object’s creation. Zenko supports expiration of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may mention we can also trigger expiration of current versions based on a date at which the rule starts applying (but no Orbit support as of now, doable with S3 API).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Zenko supports expiration of versioned or non-versioned objects, when a
defined number of days has passed since those objects’ creation. This enables
automatic deletion of older versions of versioned objects to reclaim storage
space. Zenko also supports triggering expiration of current versions on a date
from which a rule applies. This is currently feasible using the S3 API, but not
with Orbit."
Bucket lifecycle characteristics inhere to each bucket. Zenko’s lifecycle | ||
management feature enforces, but does not set these characteristics. When | ||
lifecycle expiration is enabled, the host cloud enforces buckets’ lifecycle | ||
rules. If CRR operation is enabled, Zenko replicates the expiration rules to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If CRR operation is enabled, Zenko replicates the expiration rules to all backup clouds"
This is how it is currently, but this is more of a bug that needs to be addressed, as AWS does not replicate lifecycle actions, neither do we want this behavior for Zenko. So better not to mention this as we plan to fix the current behavior soon enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
Fixed line-length issue, and review is dated, out-of-sync
tests/.secrets.env.example
Outdated
@@ -1,5 +1,3 @@ | |||
export ZENKO_STORAGE_ACCOUNT_ACCESS_KEY= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an intended change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. My suspicion is that this is an unfortunate byproduct of my Git legerdemain, but I'm not good enough at it to even know when I've fscked it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the file over from the dev/0.9 branch. I hope that's the end of this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor change 👍
I really like this, looks very nice!
docs/NewFeatures/CRR_statistics.rst
Outdated
pending, processing, and completed objects. It returns the number of | ||
errors that occurred during replication, the current throughput—in | ||
operations per second or replicating objects per second—and the number | ||
of total MB completing per second. A health check system has also been |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of MB -> bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
:: | ||
|
||
$ kubectl get pods -n kube-ops -o wide | grep es-client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely useful list of commands but I think some of these are very specific to a metal-k8s install. es-client
for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These came across with the quickstart from MetalK8s, and I moved them out of that file, but was too timid to delete them altogether. If you think it best (and insofar as these are handled already in the MetalK8s documentation) we can eliminate this file altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following through on this after discussion w/Salim. Deleting Useful_kubectl_commands.rst. This leaves the Operations directory empty, so I'll delete that as well.
docs/NewFeatures/CRR_statistics.rst
Outdated
This feature enables replication status tracking. The replication system | ||
exposes metrics through a REST API to monitor the replication of | ||
pending, processing, and completed objects. It returns the number of | ||
errors that occurred during replication, the current throughput—in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "failures" is a more accurate word choice compared with "errors".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
docs/NewFeatures/CRR_statistics.rst
Outdated
exposes metrics through a REST API to monitor the replication of | ||
pending, processing, and completed objects. It returns the number of | ||
errors that occurred during replication, the current throughput—in | ||
operations per second or replicating objects per second—and the number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "in replication operations per second" works well enough to not need the "or" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work?
"It returns the number of failures that occurred during replication, the current throughput—in replication operations or objects per second—and the total of bytes completing per second."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns the number of failures that occurred during replication, the current throughput in replication operations per second, and the total of bytes completing per second.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds perfect to me! @philipyoo What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm 👍
...and there was much rejoicing...
…On Tue, Jul 31, 2018 at 2:23 PM, Salim Salaues ***@***.***> wrote:
Merged #135 <#135>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#135 (comment)>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AiJ6GEjc8BFJzuz5tsVA9pljNcbXqTn0ks5uMMrDgaJpZM4UpzKo>
.
--
William Abernathy
Sr. Tech Writer
San Francisco
|
🍾 |
This pull request (I hope) brings my local up to date and adds RST files for new features.