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

Initial support for podman + docker-compose #15986

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

eb4x
Copy link

@eb4x eb4x commented Nov 10, 2021

Some minor cleanups and support for podman/docker-compose combination,

https://www.redhat.com/sysadmin/podman-docker-compose

eb4x added 4 commits November 10, 2021 13:12
Semantic versioning, they're called major, minor and patch.

Signed-off-by: Erik Berg <[email protected]>
Alter the regex to check for what the binary/program reports itself
to be. If it's podman and version 3 or above, there are compatability
layers introduced to support docker-compose with podman.

https://www.redhat.com/sysadmin/podman-docker-compose
Signed-off-by: Erik Berg <[email protected]>
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #15986 (a605ad3) into main (6fe2a0c) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #15986      +/-   ##
==========================================
+ Coverage   66.81%   66.83%   +0.02%     
==========================================
  Files         934      934              
  Lines       77761    77761              
  Branches     2296     2296              
==========================================
+ Hits        51955    51971      +16     
+ Misses      21791    21779      -12     
+ Partials     4015     4011       -4     
Flag Coverage Δ
unittests 66.83% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...es/vulnerability/vulnerability-config.component.ts 55.55% <0.00%> (+4.76%) ⬆️
src/common/utils/passports.go 89.74% <0.00%> (+5.12%) ⬆️
src/jobservice/runner/redis.go 67.82% <0.00%> (+5.21%) ⬆️
...ortal/src/app/shared/pipes/harbor-datetime.pipe.ts 75.00% <0.00%> (+25.00%) ⬆️

@ChristianCiach
Copy link

Can this be reviewed, please?

The only reason this currently does not work is because Harbor currently does a hardcoded check for the docker-version 17.06.0.

This version of Docker is ancient. Another way to make it work with podman is to just remove this version check.

@m4r1k
Copy link

m4r1k commented Apr 19, 2022

Hey everybody,

Apologies for not working on this anymore. Last August, I moved to a different role where my involvement with Harbor has essentially ended. Hopefully, @bpereto or @wy65701436 will be able to make it happen :-)

Best of luck!

@leonidas-o
Copy link

Hello what's the status here?
I don't think it is simply solved by refactoring common.sh, install.sh or the prepare file. I tried to refactor the files a bit to see how far could I come with podman. Tried with Harbor 2.5.0 and some aliases, so something like:

  1. set echo "alias docker=podman" >> .bashrc
  2. set echo "alias docker-compose=podman-compose" >> .bashrc
  3. reload . ~/.bashrc
  4. comment out hard coded docker and docker-compose version check in install.sh
  5. add shopt -s expand_aliases + both above mentioned alias lines to install.sh and prepare

It failed almost at the end because it was using --log-driver=syslog in the docker compose file.
But when you look into podman run docs, it says:

--log-driver=”driver”

Logging driver for the container. Currently available options are k8s-file, journald, none and passthrough, with json-file aliased to k8s-file for scripting compatibility. (Default journald)

So syslog is not one of podmans options. This would have to be configurable via e.g. the harbor.yml file.

@eb4x
Copy link
Author

eb4x commented May 24, 2022

Hello what's the status here? I don't think it is simply solved by refactoring common.sh, install.sh or the prepare file. I tried to refactor the files a bit to see how far could I come with podman.

Yeah there's more to it than just this. These changes just allow you to get further than dying at docker < 17.06. As Christian pointed out, this version is "ancient", and quite possibly the whole check should just be removed as you did.

I think these individual commits are no-brainers, and have no idea what the holdup is in merging them. Maybe "Initial support" is a bit strong wording, and "allow for" would be better, as I didn't actually get it all the way working. I've since switched to quay.

If any of the commits seem illogical, I can trim them out and rebase. (Incase they prefer part1/part2 over major/minor)

@leonidas-o
Copy link

Hello what's the status here? I don't think it is simply solved by refactoring common.sh, install.sh or the prepare file. I tried to refactor the files a bit to see how far could I come with podman.

Yeah there's more to it than just this. These changes just allow you to get further than dying at docker < 17.06. As Christian pointed out, this version is "ancient", and quite possibly the whole check should just be removed as you did.

I think these individual commits are no-brainers, and have no idea what the holdup is in merging them. Maybe "Initial support" is a bit strong wording, and "allow for" would be better, as I didn't actually get it all the way working. I've since switched to quay.

If any of the commits seem illogical, I can trim them out and rebase. (Incase they prefer part1/part2 over major/minor)

maybe it isn't merged because it's not fully working that way, as said the log-driver=syslog issue has to be solved as well before it can be used with podman. Quickly went over the generated docker-compose.yml file, I think that's all.
The question is, who can make the hardcoded log-driver=syslog setting somehow configurable in harbor?

@ChristianCiach
Copy link

ChristianCiach commented May 24, 2022

I think the issue with the syslog logging driver is slightly related to the issue #16594 . If Harbor would just log to stdout/stderr like a proper container should, this issue would also be solved implicitly.

"Logging is delegated to a separate container when docker-compose achieves the exact same thing (I tore this out with no concequence)"

@github-actions
Copy link

github-actions bot commented Jul 5, 2022

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Jul 5, 2022
@leonidas-o
Copy link

maybe it isn't merged because it's not fully working that way, as said the log-driver=syslog issue has to be solved as well before it can be used with podman. Quickly went over the generated docker-compose.yml file, I think that's all. The question is, who can make the hardcoded log-driver=syslog setting somehow configurable in harbor?

Still very relevant, at least for me. I still hope someone is able to make the log-driver configurable.

@Vad1mo Vad1mo removed the Stale label Jul 5, 2022
@Vad1mo
Copy link
Member

Vad1mo commented Jul 5, 2022

adding flexibility here is a good thing, let's try to get that in. @leonidas-o did you try the PRs, does it work for you?

@leonidas-o
Copy link

Hello what's the status here? I don't think it is simply solved by refactoring common.sh, install.sh or the prepare file. I tried to refactor the files a bit to see how far could I come with podman. Tried with Harbor 2.5.0 and some aliases, so something like:

  1. set echo "alias docker=podman" >> .bashrc
  2. set echo "alias docker-compose=podman-compose" >> .bashrc
  3. reload . ~/.bashrc
  4. comment out hard coded docker and docker-compose version check in install.sh
  5. add shopt -s expand_aliases + both above mentioned alias lines to install.sh and prepare

It failed almost at the end because it was using --log-driver=syslog in the docker compose file. But when you look into podman run docs, it says:

--log-driver=”driver”

Logging driver for the container. Currently available options are k8s-file, journald, none and passthrough, with json-file aliased to k8s-file for scripting compatibility. (Default journald)

So syslog is not one of podmans options. This would have to be configurable via e.g. the harbor.yml file.

@Vad1mo unfortunately no, back then, I have done it like described here, so really just removed the hard coded docker checks and realised that the hard coded log-driver is blocking me. I agree, adding flexibility is definitely a good thing. Do you have a setup to test the PRs? Otherwise I will have to spin up a VM for that.

@wy65701436 wy65701436 added the help wanted The issues that is valid but needs help from community label Aug 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 3, 2022

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Oct 3, 2022
@Vad1mo Vad1mo removed the Stale label Oct 4, 2022
@wy65701436
Copy link
Contributor

hi, thanks for the contribution. Since currently podman+docker-compose is not in the support scope, and we need time to evaluate it, like what's the impact and how do we test it.

@leonidas-o
Copy link

Hello what's the status here? I don't think it is simply solved by refactoring common.sh, install.sh or the prepare file. I tried to refactor the files a bit to see how far could I come with podman. Tried with Harbor 2.5.0 and some aliases, so something like:

  1. set echo "alias docker=podman" >> .bashrc
  2. set echo "alias docker-compose=podman-compose" >> .bashrc
  3. reload . ~/.bashrc
  4. comment out hard coded docker and docker-compose version check in install.sh
  5. add shopt -s expand_aliases + both above mentioned alias lines to install.sh and prepare

It failed almost at the end because it was using --log-driver=syslog in the docker compose file. But when you look into podman run docs, it says:

--log-driver=”driver”

Logging driver for the container. Currently available options are k8s-file, journald, none and passthrough, with json-file aliased to k8s-file for scripting compatibility. (Default journald)

So syslog is not one of podmans options. This would have to be configurable via e.g. the harbor.yml file.

I wonder if it would be easier if this would be treated as a feature request for, let's say:

  1. making the --log-driver=syslog somehow configurable.
  2. clean up/ remove all hard coded checks for docker

Obviously you guys don't have the resource to support podman and I think this isn't necessary. Maybe laying the focus on smaller items like, optimisation and clean up. With this two changes, you would open up the door for the community to come up with a podman solution. Then it can still be: "Podman is not officially supported" but everyone who is interested, like me, can try to make it run, document the approach and maybe upload it somewhere in the harbor docs, clearly saying, no not officially supported but using this approach should make it run. And who knows, maybe one day, one find some time to look into it and make it officially supported, if not, simply keep it unofficial. I think this is definitely better than completely locking out podman by using hard coded log-driver and docker checks.
What do you guys think, does it make sense what I'm saying?

@wy65701436 wy65701436 added the kind/requirement New feature or idea on top of harbor label Nov 17, 2022
@wy65701436 wy65701436 requested a review from qnetter November 17, 2022 09:20
@wy65701436 wy65701436 self-assigned this Nov 17, 2022
@wy65701436
Copy link
Contributor

@leonidas-o It makes sense to expand the support list for the harbor. To me, it's more like a feature request.

If we agree that this is a feature request or a new feature, I would like to follow this procedure.

This PR is not enough to introduce podman support as we don't even have any tests, this at least includes podman setup, pull/push (single imge, imge index), retag, etc.

cc @qnetter

leonidas-o added a commit to leonidas-o/community that referenced this pull request Nov 18, 2022
According to: goharbor/harbor#15986 (comment) I'm submitting this document to give the start for a possible harbor podman support.

Signed-off-by: Leo <[email protected]>
@leonidas-o
Copy link

@leonidas-o It makes sense to expand the support list for the harbor. To me, it's more like a feature request.

If we agree that this is a feature request or a new feature, I would like to follow this procedure.

This PR is not enough to introduce podman support as we don't even have any tests, this at least includes podman setup, pull/push (single imge, imge index), retag, etc.

cc @qnetter

@wy65701436 created a PR, feel free to update it if something isn't right: goharbor/community#208

@Vad1mo Vad1mo requested review from Vad1mo and removed request for qnetter August 10, 2023 15:45
@Vad1mo Vad1mo added the never-stale Do not stale label Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted The issues that is valid but needs help from community kind/requirement New feature or idea on top of harbor never-stale Do not stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants