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

docs: migrate docs to GitHub repository #403

Open
wants to merge 65 commits into
base: dev
Choose a base branch
from

Conversation

sergeyberezansky
Copy link
Collaborator

Opening a PR for easy commenting on the current docs

sergeyberezansky and others added 30 commits November 1, 2024 12:04
### TL;DR

Added multi-architecture support for AMD64 and ARM64 platforms in Docker builds and updated GitHub Actions workflows.

### What changed?

- Added QEMU setup in GitHub Actions workflows for multi-architecture support
- Included `platforms: linux/amd64,linux/arm64` in Docker build steps
- Updated README to mention support for both AMD64 and ARM64 platforms
- Made minor formatting improvements in workflow files

### How to test?

1. Build the Docker image for both AMD64 and ARM64 platforms
2. Verify that the image can be pulled and run on both architectures
3. Deploy the Helm chart on a Kubernetes cluster with mixed AMD64 and ARM64 nodes
4. Ensure that the CSI driver functions correctly on both architectures

### Why make this change?

This change expands the compatibility of the CSI WekaFS Plugin to support ARM64 architecture, in addition to the existing AMD64 support. This allows for greater flexibility in deployment across different hardware platforms and cloud environments, potentially improving performance and cost-efficiency for users with ARM-based infrastructure.
…S and support same fs name on SCMC (#383)

### TL;DR
Refactored mount handling to improve container name management and mount reference counting.

### What changed?
- Added container name caching to ApiClient
- Introduced `EnsureLocalContainer` method to handle container name resolution
- Simplified mount reference counting using a flat map structure
- Added utility function to extract container names from mount points
- Fixed duplicate reference count increment in NFS mounting
- Added comprehensive test coverage for container name extraction
- Improved logging for mount operations and debugging

### How to test?
1. Test mounting filesystems with different container configurations
2. Verify container name extraction from mount points using the new test cases
3. Check mount reference counting behavior with multiple mount/unmount operations
4. Validate logging output for mount operations
5. Test compatibility with both legacy and new mounting scenarios

### Why make this change?
- Improves reliability of container name handling in multi-cluster environments
- Simplifies mount reference counting logic to prevent memory leaks
- Enhances debugging capabilities through better logging
- Reduces code duplication and potential race conditions
- Makes the codebase more maintainable and testable
Added link to the doc and reworded the intro
@graphite-app graphite-app bot requested a review from tigrawap November 28, 2024 14:31
Copy link

graphite-app bot commented Nov 28, 2024

Graphite Automations

"Request reviewers once CI passes" took an action on this PR • (11/28/24)

1 reviewer was added to this PR based on Sergey Berezansky's automation.

Copy link
Collaborator Author

@sergeyberezansky sergeyberezansky left a comment

Choose a reason for hiding this comment

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

@AriAttias I created a PR on behalf of you.
Please just fix the relevant places and continue pushing to same branch

@@ -29,9 +29,6 @@ https://github.com/weka/csi-wekafs
- [SELinux Support & Installation Notes](selinux/README.md)
- [Using Weka CSI Plugin with NFS transport](docs/NFS.md)

## Additional Documentation
- [Official Weka CSI Plugin documentation](https://docs.weka.io/appendices/weka-csi-plugin)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same should be done on README.md.gotmpl

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've removed the reference to the official doc.


## Usage
- [Deploy an Example application](https://github.com/weka/csi-wekafs/blob/master/docs/usage.md)
- [SELinux Support & Installation Notes](https://github.com/weka/csi-wekafs/blob/master/selinux/README.md)

## Additional Documentation
- [Official Weka CSI Plugin documentation](https://docs.weka.io/appendices/weka-csi-plugin)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same should be done on README.md.gotmpl

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've already removed the reference to the official doc from the README.md.gotml.
Why does the comment appear twice?

docs/NFS.md Outdated
### Limitations and constraints

**Warning**:
As of version 2.5.0 and until further notice, publishing snapshot-backed volumes via NFS transport is not recommended. This is an open issue currently under investigation.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Make it a quote with prepending with >
Same way as it is done in other places

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added > for notes and warnings

docs/NFS.md Outdated

#### Mount options

The WEKA CSI Plugin automatically sets mount options for NFS transport. When custom mount options are specified in the storage class, the WEKA CSI Plugin translates them into NFS alternatives. Any unknown or unsupported mount options are ignored.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please add a comment that directly setting NFS mount options is not supported.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added

docs/NFS.md Outdated
2. **NFS failback check**:
* If the WEKA client is not set up, the plugin checks if NFS failback is enabled or if NFS use is forced.
* If NFS failback is enabled, the plugin uses NFS transport for volume provisioning and publishing.
* If NFS failback is disabled, the plugin does not start and logs an error message. See the section to enable NFS failback.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

misses link to the installation section

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added a link to the installation with NFS section.

@@ -29,7 +31,7 @@ Optional parameters:
--------------------
--debug Execute with debug level logging
--csi-volumes-dir Assume CSI volumes are stored in different directory on the filesystem. Default is "csi-volumes"
--endpoint-address API_ADDRESS:PORT of a Weka backend server, should be used for stateless clients.
--endpoint-address API_ADDRESS:PORT of a WEKA backend server for stateless clients.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should be specified if host is not connected to the cluster

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added: Specify the port if the host is not connected to the cluster.


Kubernetes does not allow modifying the StorageClass parameters; hence every volume created with the legacy-model storage class never reports its credentials.

WEKA CSI Plugin **0.7.0** provides a unique configuration mode in which legacy volumes can be bound to a single secret, referring to a single WEKA cluster API connection parameters. In this configuration mode, every request to serve, such as create, delete, and expand, a legacy Persistent Volume (or Persistent Volume Claim) that originates from a Legacy Storage Class (without reference to an API secret) communicates to that cluster.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From version 0.7.0 we provide a "fallback" mode that allows to bind the "secretless" legacy volumes to a specific sevret name.
Will be removed since CSI 3.0 hence discouraged

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added to the note:
This fallback mode of version 0.7.0 is discouraged as it will be deprecated and removed starting with CSI 3.0.

WEKA CSI Plugin **0.7.0** provides a unique configuration mode in which legacy volumes can be bound to a single secret, referring to a single WEKA cluster API connection parameters. In this configuration mode, every request to serve, such as create, delete, and expand, a legacy Persistent Volume (or Persistent Volume Claim) that originates from a Legacy Storage Class (without reference to an API secret) communicates to that cluster.

**Note**:
Volumes provisioned by the CSI Plugin of version **0.7.0** in the API-based communication model, but on older versions of the WEKA cluster (below version **3.13.0**), are still in legacy mode. However, because the storage class already contains the secret reference, specifying the `legacyVolumeSecretName` parameter is unnecessary, and you can safely skip to the **Migrate legacy volumes** procedure below.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this something I wrote in origin?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can find the original text in version 3.14 (before I came to WEKA).
https://docs.weka.io/3.14/appendix/weka-csi-plugin#upgrading-legacy-persistent-volumes-for-capacity-enforcement

@@ -1,142 +1,159 @@
# CSI WekaFS SELinux Support
# Add SELinux support
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This section refers only to SELinux enabled deployments different from RedHat OpenShift Container Platform (OCP)
For OCP, all necessary configurations are performed automatically

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added the above to the overview


The reason behind this is a lack of permissions for containers to access objects stored on Weka cluster.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

have you removed this intentionally? please re-add

Copy link
Collaborator

Choose a reason for hiding this comment

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

Re-added this line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This slide should also be changed with NFS support
Add another K8s worker node named "Branch / Remote K8s worker node" or whatever distinguishes it from the rest. Then this node does not have a Weka Client and instead uses Kernel NFS driver and connects to NFS gateway on some clusters

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or maybe instead add another slide that provides 2 nodes only connected to same / different clusters, one is NFS, another is wekafs

Removed the reference to the official doc.
Added > to warning and note
Added > before a warning
Added a link to installation with NFS
Added: Specify the port if the host is not connected to the cluster.
Added to the note:
This fallback mode of version **0.7.0** is discouraged as it will be deprecated and removed starting with CSI 3.0.
Updated the intro with:
- This section applies to ...
- The purpose of this section ...
Reworded the note about volumes provisioned in 0.7.0.
Replacing the image: k8s-multiple-clusters.png
Replaced the overview for Kubernetes Storage Integration with WEKA: WekaFS and NFS Transport Modes.
Now, we need to update the procedure steps accordingly.
edited the link to the image
Removed the note about unique fs names across multiple clusters because it is irrelevant to the architecture.
Updated the section: Kubernetes Storage Integration with WEKA: WekaFS and NFS Transport Modes
We have added a prerequisite: 
The WEKA CSI container must run as the root user to enable the management of WEKA filesystem mounts at the Worker Node level.
New title: Deployment and upgrade workflows.
So we can add this as a reference to the main README.md
I added a reference to the deployment page in the docs.
Revised the line under building the binaries
Revised the opening line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-semantic-pull-request skip-tests Allow skipping sanity tests for nested PRs and docs only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants