fix(apptainer): ensure apptainer jobs run as privileged

[gilbsgilbs]

Hi,

Honestly, I'm not really sure about this one, I may just be missing the obvious. Feel free to close that PR if that's the case.

I wasn't able to reach the branch where the container is set as privileged when running in Apptainer because job.is_containerized would always evaluate to false, regardless of the snakemake flags I use. I believe checking the deployment method is sufficient to detect for container jobs, and avoid the explicit use of --kubernetes-privileged true flag.

---

fix(apptainer): ensure apptainer jobs run as privileged

Apptainer jobs require privileged access to function correctly, for
instance to mount volumes and manage cgroups.

The previous implementation incorrectly relied on `job.is_containerized`
to enable privileged mode for Apptainer jobs. I suspect this flag is
intended for use with the `--containerize` option and does not reliably
indicate that a job is running in a container.

This commit enables privileged mode whenever the Apptainer deployment
method is used.

Summary by CodeRabbit

Bug Fixes

• Corrected the container privilege mode logic in the Kubernetes executor plugin for Apptainer deployments, ensuring consistent application of privileged settings based on explicit privilege flags or deployment method selection, providing more predictable container execution behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

A conditional block setting container privileged mode for APPTAINER containerized jobs was removed and consolidated. Privileged mode is now centrally determined by checking if either an explicit privileged flag is set or APPTAINER is among the deployment methods.

Changes

Cohort / File(s)	Change Summary
Privileged mode logic consolidation snakemake_executor_plugin_kubernetes/__init__.py	Removed separate APPTAINER-specific conditional branch that granted privileged mode for containerized jobs. Consolidated logic to single final condition granting privileged status when explicit privileged flag is set or APPTAINER is in deployment methods.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify that the consolidated conditional logic is semantically equivalent to the original separate branches
• Confirm edge cases are handled correctly (privileged flag only, APPTAINER only, both, neither)
• Check that no unintended behavior changes occur for non-APPTAINER or non-containerized deployments

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: ensuring Apptainer jobs run with privileged mode, which aligns with the core purpose of the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

snakemake_executor_plugin_kubernetes/__init__.py (1)

84-87: Update documentation to reflect Apptainer auto-enable behavior.

Consider updating the help text for the privileged setting to clarify that privileged mode is automatically enabled when using the Apptainer deployment method, even if this flag is set to False.

Apply this diff to improve the documentation:

     privileged: Optional[bool] = field(
         default=False,
-        metadata={"help": "Create privileged containers for jobs."},
+        metadata={"help": "Create privileged containers for jobs. Note: privileged mode is automatically enabled when using Apptainer deployment method."},
     )

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5f720e and 133a595.

📒 Files selected for processing (1)

• snakemake_executor_plugin_kubernetes/__init__.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

• snakemake_executor_plugin_kubernetes/__init__.py

🧠 Learnings (1)

📚 Learning: 2024-08-14T14:18:38.175Z

Learnt from: johanneskoester
Repo: snakemake/snakemake-executor-plugin-kubernetes PR: 22
File: snakemake_executor_plugin_kubernetes/__init__.py:32-39
Timestamp: 2024-08-14T14:18:38.175Z
Learning: Do not suggest type annotation of the `self` argument of methods in the Snakemake executor plugin for Kubernetes.

Applied to files:

• snakemake_executor_plugin_kubernetes/__init__.py

🔇 Additional comments (1)

snakemake_executor_plugin_kubernetes/__init__.py (1)

350-353: The code usage pattern confirms deployment_method is a collection type.

Based on verification:

• deployment_method is accessed from self.workflow.deployment_settings (Snakemake core package)
• The code uses the in operator at line 352: DeploymentMethod.APPTAINER in self.workflow.deployment_settings.deployment_method, which only works with collection types (list, set, tuple, or custom types with __contains__)
• If deployment_method was a single enum value, the code would fail with a TypeError at runtime
• The pattern is consistent with supporting multiple deployment methods

The consolidated logic is correct and properly checks whether Apptainer is among the configured deployment methods. However, since deployment_method is from the external Snakemake package (not verifiable within this repository), manually confirm that your version of the Snakemake package documents deployment_method as a collection type and that this code works correctly in your test environment before merging.