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

feat(csidriver): declare seLinuxMount support #598

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions deploy/helm/charts/templates/csidriver.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ spec:
attachRequired: false
podInfoOnMount: false
storageCapacity: {{ .Values.feature.storageCapacity }}
seLinuxMount: true
Copy link
Member

Choose a reason for hiding this comment

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

Maybe set this via helm variable?

Copy link
Member

Choose a reason for hiding this comment

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

moreover if this is supported in Kubernetes version ≥ 1.27 then why are we setting it to true as default

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to be accepted from 1.25.x, so I suggest this:

Suggested change
seLinuxMount: true
{{- if semverCompare ">= 1.25.x" .Capabilities.KubeVersion.Version }}
seLinuxMount: true
{{- end }}

Copy link
Author

Choose a reason for hiding this comment

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

thank you

1 change: 1 addition & 0 deletions deploy/zfs-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2787,3 +2787,4 @@ spec:
attachRequired: false
podInfoOnMount: false
storageCapacity: true
seLinuxMount: true
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
seLinuxMount: true
{{- if semverCompare ">= 1.25.x" .Capabilities.KubeVersion.Version }}
seLinuxMount: true
{{- end }}

84 changes: 84 additions & 0 deletions design/enable-selinux-mount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
---
title: SELinux Mount Support for LocalPV-ZFS
authors:
- "@cjorge-graphops"
creation-date: 2024-11-01
last-updated: 2024-11-01
---
Comment on lines +1 to +7
Copy link
Member

Choose a reason for hiding this comment

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

@tiagolobocastro Should this be here?

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean vs the root openebs? Seems we already have pv-migration here in this repo as well.
So basically, going forward do we want to keep designs per repo or on umbrella repo? CC @avishnu

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just keep it here for now.
Let's discuss on the next maintainers call on how to organize these better.


# SELinux Mount Support for LocalPV-ZFS

## Table of Contents

* [Summary](#summary)
* [Problem](#problem)
* [Proposal](#proposal)
* [Implementation Plan](#implementation-plan)
* [Test Plan](#test-plan)
* [GA Criteria](#ga-criteria)

## Summary

This design proposes adding SELinux mount support to the ZFS LocalPV CSI driver by enabling the `seLinuxMount` feature in the CSIDriver specification. This will allow Kubernetes to perform efficient SELinux relabeling for volumes containing many small files. Technical details on this and motivation are available in the [KEP](https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/1710-selinux-relabeling).

## Problem

When PVCs contain millions of small files, SELinux relabeling takes an enormous amount of time. This is because the CRI (i.e CRI-O) currently has to recursively relabel all files individually. With the SELinux mount support feature introduced in Kubernetes 1.27, this process can be optimized by passing SELinux context through mount options, but requires CSI driver support.

## Proposal

Enable SELinux mount support by:

1. Setting `seLinuxMount: true` in the CSIDriver specification
2. Location: `deploy/helm/charts/templates/csidriver.yaml`

This is valid because:
- ZFS volumes are independently mounted from the Linux kernel's perspective
- ZFS supports SELinux context mount options

No additional changes are required in the CSI driver implementation as Kubernetes will:
- Only pass `-o context=<SELinux label>` when all conditions are met
- Verify SELinux OS support before attempting to use the feature
- Handle the feature gates and pod requirements

No need for further upgrade-path considerations as the CSIDriver can be updated in place

## Implementation Plan

1. Add `seLinuxMount: true` to the CSIDriver spec:
```yaml
apiVersion: storage.k8s.io/v1
kind: CSIDriver
metadata:
name: zfs.csi.openebs.io
spec:
seLinuxMount: true
# ... existing fields remain unchanged
```

2. Document requirements in README:
- Feature gates needed: `ReadWriteOncePod` and `SELinuxMountReadWriteOncePod`
- Kubernetes version ≥ 1.27 for beta support
- Pod must specify SELinux level
- PV must be RWOP.

Note: I'm unsure this is necessary or makes sense to document, thoughts?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think it's good to document the requirements.


## Test Plan

1. Verify basic functionality:
- Create PVC with millions of small files
- Verify mount succeeds with SELinux context
- Confirm relabeling performance improvement

Note: I explored validating that indeed Kubernetes acts as the KEP design describes by:
- Test with feature gates disabled
- Test with PODs that don't set SELinux level or PVCs that aren't `ReadWriteOncePod`

had no easily available non-SELinux system to test with. But these aren't really the concern of zfs-localpv as the responsibility lies elsewhere.

## GA Criteria

1. All test cases passing consistently
2. No reported issues with SELinux mount support
3. Documentation updated and complete
Loading