Skip to content

Heketi on OpenShift#5364

Merged
gaurav-nelson merged 1 commit intoopenshift:masterfrom
gaurav-nelson:heketiOnOCP
Nov 3, 2017
Merged

Heketi on OpenShift#5364
gaurav-nelson merged 1 commit intoopenshift:masterfrom
gaurav-nelson:heketiOnOCP

Conversation

@gaurav-nelson
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Didn't see /usr/share/heketi/toplogy-sample.json, may because I just have heketi-client RPM package installed. Should hint customer here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @wkshi I have updated the docs to include more details around this one by adding following note:

Depending upon your method of installation this file may not exist. If its missing, manually create the *_toplogy-sample.json_* file, as shown in the following example.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me~

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

# heketi-cli topology load --json=topology.json
Error: Unable to open config file

The topology.json does existing, may some steps missing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is a extra "," here. Remove it then "heketi-cli topology load --json=toplogy-sample.json" could works.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed the extra "," from line 179, in the latest commit and also fixed it in the gluster dynamic example.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me~

@gaurav-nelson
Copy link
Copy Markdown
Contributor Author

@openshift/team-documentation PTAL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assumes you have familiarity ??

@gaurav-nelson
Copy link
Copy Markdown
Contributor Author

[rev_history]
|xref:../install_config/storage_examples/containerized_heketi_with_dedicated_gluster.adoc#install-config-storage-examples-containerized-heketi-dedicated-gluster[Containerized Heketi for managing dedicated GlusterFS storage]
|Added an example with instructions to install Heketi on OpenShift and use to manage external GlusterFS storage
%

@gaurav-nelson
Copy link
Copy Markdown
Contributor Author

Thanks @mburke5678 I have added rev history as well. Are you done with peer review?

Copy link
Copy Markdown
Contributor

@mburke5678 mburke5678 Nov 1, 2017

Choose a reason for hiding this comment

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

Does EmptyDir need mark up? i looked quickly at the docs and there doesn't seem to be consistent usage. Some EmptyDir, some EmptyDir and some with no mark up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Kubernetes uses emptyDir maybe we should use the same markup. I'll check this in our docs and submit a new PR with the changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mburke5678 addressed this in #6084

Copy link
Copy Markdown
Contributor

@mburke5678 mburke5678 Nov 1, 2017

Choose a reason for hiding this comment

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

s/its/ it is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Run the heketi-cli command to load the topology of your environment (??)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I will just go with Run the following command to load the topology of your environment:

Copy link
Copy Markdown
Contributor

@mburke5678 mburke5678 Nov 1, 2017

Choose a reason for hiding this comment

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

Where the last storageclass is talking about a storage class object, should this be two words?
Or, "...skip the following storage class creation steps..."
Or StorageClass like in the "Dynamic Provisioning and Creating Storage Classes" chapter?
There does not seem to be a consistent way that we are using this term.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we should use StorageClass when we talk about specific object and use storage class in general.

@mburke5678
Copy link
Copy Markdown
Contributor

@gaurav-nelson A few more nits and an open question on how we present the term storage class. LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants