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

fix(cStor, deployments): add OpenEBS base directory in deployments #1583

Merged
merged 4 commits into from
Jan 9, 2020

Conversation

mittachaitu
Copy link

@mittachaitu mittachaitu commented Jan 9, 2020

Signed-off-by: mittachaitu [email protected]

What this PR does / why we need it:
This PR mounts the /var/openebs of the container into OPENEBS_IO_BASE_DIR/<cstor_target/pool>/<resource_name> on the host path.
These changes are made to persist core files on the host path.

How Users Can Configure
Users can specify the value of OPENEBS_IO_BASE_DIR in Maya deployment.
While creating cStor pool/volume corresponding component will read the ENV value
and use base_dir for mounting. If ENV doesn't exist base_dir will mount /var/openebs.

Below are PR's to support dumping core in /var/openebs/core inside a container:
cStor: mayadata-io/cstor#284
istgt: openebs-archive/istgt#298

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:
These changes are for SPC type of provisioning subsequent PRs will be raised for CSPC.

Checklist:

  • Fixes #
  • Labelled this PR & related issue with documentation tag
  • PR messages has document related information
  • Labelled this PR & related issue with breaking-changes tag
  • PR messages has breaking changes related information
  • Labelled this PR & related issue with requires-upgrade tag
  • PR messages has upgrade related information
  • Commit has unit tests
  • Commit has integration tests

Copy link
Contributor

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

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

changes are good

@mittachaitu mittachaitu added the pr/upgrade-changes-pending This PR requires upgrade changes to be automated label Jan 9, 2020
@@ -579,6 +586,8 @@ spec:
mountPath: /var/run
- name: conf
mountPath: /usr/local/etc/istgt
- name: storagepath
mountPath: {{ .Config.PersistentStoragePath.value }}

Choose a reason for hiding this comment

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

why PersistentStoragePath variable is needed? it has to be "/var/openebs" all the time, other wise istgt image needs to be regenerated with the new mount path? isn't it?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, If that variable changed then istgt image also needs to be regenerated. So one shouldn't touch that PersistentStoragePath variable(Made comments above that variable).
We are using in different places so made variable to make it use.

Copy link
Author

Choose a reason for hiding this comment

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

I will hard code it

Copy link
Contributor

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@pawanpraka1 pawanpraka1 left a comment

Choose a reason for hiding this comment

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

looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/upgrade-changes-pending This PR requires upgrade changes to be automated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants