Decouple rootless value from image name generation. #532

New Issue

2023-10-11T19:44:56Z

dgershman commented

2023-10-11 19:44:56 +00:00

(Migrated from gitea.com)

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.

justusbunsi commented

2023-10-11 20:00:24 +00:00

(Migrated from gitea.com)

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.

dgershman commented

2023-10-12 00:32:44 +00:00

(Migrated from gitea.com)

PR opened #534

TristanHoladay commented

2023-11-02 15:45:01 +00:00

(Migrated from gitea.com)

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.

justusbunsi commented

2023-11-02 20:03:02 +00:00

(Migrated from gitea.com)

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.

TristanHoladay commented

2023-11-03 10:43:34 +00:00

(Migrated from gitea.com)

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.

TristanHoladay commented

2023-11-08 14:22:40 +00:00

(Migrated from gitea.com)

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?

dgershman commented

2023-11-08 14:49:12 +00:00

(Migrated from gitea.com)

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

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#532