Decouple rootless value from image name generation. #532

Closed
opened 2023-10-11 19:44:56 +00:00 by dgershman · 7 comments

Currently you are required to have a tag that has the suffix -rootless with the helper for gitea.image. By making this change you could use another image (say the Iron Bank one, that doesn't append -rootless to the tag.

Currently you are required to have a tag that has the suffix `-rootless` with the helper for `gitea.image`. By making this change you could use another image (say the Iron Bank one, that doesn't append `-rootless` to the tag.
Member

You are right. Using a different image is currently not easily possible, if possible.

From a design perspective we then should move image.rootless out of image and append the -rootless suffix only for gitea/gitea images. To make it fully flexible, there should probably be an image.fullOverride or so to prevent possible false-positive gitea/gitea detections.

Feel free to open a PR for it.

You are right. Using a different image is currently not easily possible, if possible. From a design perspective we then should move `image.rootless` out of `image` and append the `-rootless` suffix only for `gitea/gitea` images. To make it fully flexible, there should probably be an `image.fullOverride` or so to prevent possible false-positive `gitea/gitea` detections. Feel free to open a PR for it.
Author

PR opened #534

PR opened #534
Contributor

saw this hasn't been touched in a few weeks so I made a new PR, #550, incorporating the changes @dgershman made with unit tests. Happy to have the unit tests merged into his PR and close mine, just wasn't sure what the right protocol was to continue the work he's already done.

Note: to get unit tests to pass locally i had to update the template paths (e.g templates/gitea/deployment.yaml --> ../../templates/gitea/deployment.yaml) unless I removed the templates field from the test suite and added the needed template path to each test. I didn't commit those changes cause I assumed the setup should work correctly in CI.

saw this hasn't been touched in a few weeks so I made a new PR, #550, incorporating the changes @dgershman made with unit tests. Happy to have the unit tests merged into his PR and close mine, just wasn't sure what the right protocol was to continue the work he's already done. Note: to get unit tests to pass locally i had to update the template paths (e.g `templates/gitea/deployment.yaml` --> `../../templates/gitea/deployment.yaml`) unless I removed the `templates` field from the test suite and added the needed template path to each test. I didn't commit those changes cause I assumed the setup should work correctly in CI.
Member

saw this hasn't been touched in a few weeks so I made a new PR, #550, incorporating the changes @dgershman made with unit tests. Happy to have the unit tests merged into his PR and close mine, just wasn't sure what the right protocol was to continue the work he's already done.

Thanks for continuing this and adding tests.

Note: to get unit tests to pass locally i had to update the template paths (e.g templates/gitea/deployment.yaml --> ../../templates/gitea/deployment.yaml) unless I removed the templates field from the test suite and added the needed template path to each test. I didn't commit those changes cause I assumed the setup should work correctly in CI.

This seems to be an issue with the latest unittest plugin version. I experienced the same locally when I tried upgrading it. But didn't had enough time at that moment to dive deeper into it.

> saw this hasn't been touched in a few weeks so I made a new PR, #550, incorporating the changes @dgershman made with unit tests. Happy to have the unit tests merged into his PR and close mine, just wasn't sure what the right protocol was to continue the work he's already done. Thanks for continuing this and adding tests. > Note: to get unit tests to pass locally i had to update the template paths (e.g `templates/gitea/deployment.yaml` --> `../../templates/gitea/deployment.yaml`) unless I removed the `templates` field from the test suite and added the needed template path to each test. I didn't commit those changes cause I assumed the setup should work correctly in CI. This seems to be an issue with the latest unittest plugin version. I experienced the same locally when I tried upgrading it. But didn't had enough time at that moment to dive deeper into it.
Contributor

Interesting. I was wondering if it was a new version issue, but similarly didn't have time to look too closely at it. Thanks for taking a look at the PR.

Interesting. I was wondering if it was a new version issue, but similarly didn't have time to look too closely at it. Thanks for taking a look at the PR.
Contributor

Since @dgershman hasn't commented on this effort in quite a while is it possible to move forward with the PR I opened? Are there any other changes y'all would like to see?

Since @dgershman hasn't commented on this effort in quite a while is it possible to move forward with the PR I opened? Are there any other changes y'all would like to see?
Author

I'd be in favor of your PR over mine.

I'd be in favor of your PR over mine.
pat-s closed this issue 2023-11-14 21:42:27 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/helm-chart#532
No description provided.