fix: request disk_mb resource from k8s #1858
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
|
@johanneskoester is there a way to trigger the k8s tests on github? They seem to be skipped (I assume as I'm not a maintainer and the PR isn't approved yet?) |
|
Indeed, they only work for people with access to the repo, because of required tokens (they run on GCE). I always wanted to use something like minikube directly inside of github actions, but I did not yet find the time to implement that (maybe there is even a prebuilt action for that). However, I think that the changes are minimal enough to risk just merging them. If main branch fails afterwards, I will just contact you via this PR. For the future, I think you can also enable in your github settings that the upstream repo is allowed to commit into your PR. This way I could trigger a test run with my permissions. Regarding the volume mounts, I think you are right! A PR would be very welcome! |
As described in the body of PR snakemake#1858
|
Looks like I did break the tests, but not directly: the |
|
OK, thanks. |
### Description Fixes a bug in tests introduced in #1858 ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).
As described in the body of PR #1858, there's a typo whereby the property `container.volume_mounts` is overwritten, rather than appended to. ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. (this is allegedly tested as part of the k8s tests). * [ ] ~The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).~
🤖 I have created a release *beep* *boop* --- ## [7.14.1](v7.14.0...v7.14.1) (2022-09-23) ### Bug Fixes * allocation of local ssds in k8s tests ([#1870](#1870)) ([d0de4dc](d0de4dc)) * allow script directive to take pathlib Path ([#1869](#1869)) ([12cdc96](12cdc96)) * catch errors in remote.AUTO provider list ([#1834](#1834)) ([c613ed2](c613ed2)) * consistently use text output in conda shell commands and various little fixes for failing test cases due to conda package changes ([#1864](#1864)) ([4234fe7](4234fe7)) * declare associative arrays ([#1844](#1844)) ([90ae449](90ae449)) * fix falsely triggered reruns if input files are obtained via workflow.source_path() ([#1862](#1862)) ([2dc2e6a](2dc2e6a)) * fixed typos ([#1847](#1847)) ([a1e49b6](a1e49b6)) * k8s container volume mounts as list ([#1868](#1868)) ([5c54df3](5c54df3)) * None type error when invoking Workflow object manually ([#1731](#1731)) ([dc45ccb](dc45ccb)) * request disk_mb resource from k8s ([#1858](#1858)) ([f68f166](f68f166)) * respect shebang lines in post-deploy scripts (see deployment docs) ([#1841](#1841)) ([c26c4b6](c26c4b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
k8s supports adding ephemeral storage (e.g. node-local SSDs) to the working directory of the container. However, the size of this working directory isn't configured. This PR maps the
disk_mbresource to theephemeral-storagek8s Allocatable.As an aside, while reviewing this PR, I think I have found a small issue: during the container init code, we are setting the
container.volume.mountsproperty twice with single-entry lists, rather than once with a list of two mounts as their API seems to want. Is my reading correct, and this is a bug? If so, I'll quickly fix it.snakemake/snakemake/executors/__init__.py
Lines 1855 to 1860 in a1e49b6
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). My (@kdm9) reading of the docs implied this already happened, but I can add a more explicit statement if need be