Skip to content

docs: add third-party storage docs#728

Merged
Vicente-Cheng merged 1 commit intoharvester:mainfrom
Vicente-Cheng:3rd-party-docs
Apr 22, 2025
Merged

docs: add third-party storage docs#728
Vicente-Cheng merged 1 commit intoharvester:mainfrom
Vicente-Cheng:3rd-party-docs

Conversation

@Vicente-Cheng
Copy link
Contributor

@Vicente-Cheng Vicente-Cheng commented Mar 17, 2025

Add third-party storage support docs.

Related: harvester/harvester#1199

@github-actions
Copy link

github-actions bot commented Mar 17, 2025

Name Link
🔨 Latest commit 64045a2
😎 Deploy Preview https://68059f3481d35c16594a991b--harvester-preview.netlify.app

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

Initial review done. Please check my comments.

:::note
:::important

The Support Matrix below shows the capabilities of the validated CSI drivers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The Support Matrix below shows the capabilities of the validated CSI drivers.
The following table outlines the capabilities supported by the validated CSI drivers.

Comment on lines 30 to 33
For the Harvester functions to work well, the third-party CSI driver might better to have the following capabilities:
- Support expansion, for online resizing of the volume
- Support snapshot, for take snapshot of the volume/VM
- Support clone, for clone volume/VM
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For the Harvester functions to work well, the third-party CSI driver might better to have the following capabilities:
- Support expansion, for online resizing of the volume
- Support snapshot, for take snapshot of the volume/VM
- Support clone, for clone volume/VM
To enable Harvester to function well, use CSI drivers that support the following capabilities:
- Volume expansion (online resizing)
- Snapshot creation (volume and virtual machine snapshots)
- Cloning (volume and virtual machine clones)
- Usage of Read-Write-Many (RWX) volumes for [Live Migration](../vm/live-migration.md)

@Vicente-Cheng
Copy link
Contributor Author

I don't know if we're allowed to link to HEPs. The end user doc should focus on key concepts and operational procedures. Design and implementation details can be discussed only if users are required to understand them. Please check if the topics in this section must really be included.

The topic Advanced/Internal Topics I used to introduce the StorageProfile concept because this concept is very important once user wants to use their own storage solution. We need to let them know how they should configure.
Because of that, I added this topic.
Or did you think I just mentioned the StorageProfile for something topic like Advanced?

WDYT for this section? cc @ihcsim, @bk201, @WebberHuang1118, @jillian-maroket

@ihcsim
Copy link
Contributor

ihcsim commented Mar 24, 2025

@Vicente-Cheng I am not sure if we are responsible for providing storage profile examples in our documentation. IIUC, we just need users to know it will work out-of-box because Harvester integrated with CDI, right? It's not mandatory to use storage profiles in Harvester. For some well-known provisioners, CDI might even auto-create the storage profiles based on their storage classes.

If this is just for the user's FYI, would something like this make sense:

"By integrating Harvester with CDI, you can now utilize the CDI API to create custom storage profiles to simplify your data volumes definition. Storage profiles allow multiple data volumes to share the same provisioner settings.

Here is an example of a LVM storage profile: // personally, i might even omit this part

# insert example

For more information on storage profiles, refer to the CDI documentation."

Other suggestions on the same section:

  1. Omit the "Top level architecture" section. HEP sounds like a better home for it
  2. Omit the clone strategies. We are not responsible for explaining CDI concepts

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

I reviewed the parts that were skipped in the initial review. Note that I agree with Ivan's suggestions, which are reflected in my recommended wording and structural changes.

@Vicente-Cheng
Copy link
Contributor Author

I reviewed the parts that were skipped in the initial review. Note that I agree with Ivan's suggestions, which are reflected in my recommended wording and structural changes.

All updated!
Thanks for the review, please recheck.

ihcsim
ihcsim previously approved these changes Mar 31, 2025
Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

LGTM - just 2 minor suggestions.

jillian-maroket
jillian-maroket previously approved these changes Apr 1, 2025
ihcsim
ihcsim previously approved these changes Apr 2, 2025
Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Vicente-Cheng
Copy link
Contributor Author

Hi folks,
I added the two parts. Please help review them again.

  1. Added the validated CSI driver. (Even the same storage solution would use a different CSI driver, so I specify the CSI driver we validated)
  2. I added the NFS server-side configuration (squash related) to give the user a hint on how to configure it to make things go well.

@Vicente-Cheng Vicente-Cheng force-pushed the 3rd-party-docs branch 2 times, most recently from 2c07329 to 252c59e Compare April 18, 2025 02:54
@Vicente-Cheng
Copy link
Contributor Author

Vicente-Cheng commented Apr 18, 2025

Hi folks,
I also added the known issue part for [DOC] Adjusting CDI filesystemOverhead to prevent scratch volume space shortage

Also, remove the old all-in-one NFS server + CSI driver on Deploy the Combined NFS Service and CSI Driver

Please help review them. Thanks!

bk201
bk201 previously approved these changes Apr 18, 2025
ihcsim
ihcsim previously approved these changes Apr 18, 2025
Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Copy link
Contributor

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

I reviewed the new text.

@Vicente-Cheng Vicente-Cheng dismissed stale reviews from WebberHuang1118, ihcsim, and bk201 via edfefd8 April 21, 2025 01:08
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Co-authored-by: Jillian Maroket <jillian.maroket@suse.com>
Co-authored-by: Ivan Sim <ivan.sim@suse.com>
Co-authored-by: David Ko <dko@suse.com>
Co-authored-by: Volker Theile <votdev@gmx.de>
Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Vicente-Cheng Vicente-Cheng merged commit 38303eb into harvester:main Apr 22, 2025
4 checks passed
@votdev votdev mentioned this pull request May 7, 2025
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.

7 participants