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.

Sign up for GitHub

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

fix docker build problem and added arm64 support #67

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from
+44 −15
Open

fix docker build problem and added arm64 support #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2614429
fix docker build problem and added arm64 support
AmirulAndalib Dec 26, 2022
a71366b
minor typo fixes
AmirulAndalib Dec 30, 2022
8db0019
minor fixes
AmirulAndalib Dec 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions .github/workflows/beta.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,42 @@ on:
push:
branches:
- 'beta'
workflow_dispatch:

env:
IMAGE: toddrob/searcharr
jobs:
docker:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
toddrob99 marked this conversation as resolved.
Show resolved Hide resolved
steps:
-
name: Set up QEMU
uses: docker/setup-qemu-action@v2
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
-
uses: actions/checkout@v2
-
name: Login to DockerHub
uses: docker/login-action@v2
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
-
- if: startsWith(github.ref, 'refs/heads/master')
run: echo "TAG=latest" >> $GITHUB_ENV
- if: startsWith(github.ref, 'refs/tags')
run: echo "TAG=$(git describe --tags)" >> $GITHUB_ENV
toddrob99 marked this conversation as resolved.
Show resolved Hide resolved
- name: upgrade apt packages..
run: sudo apt-get update -y && sudo apt-get upgrade -y
toddrob99 marked this conversation as resolved.
Show resolved Hide resolved
# https://github.com/docker/setup-qemu-action
- name: Set up QEMU
uses: docker/setup-qemu-action@v2
# https://github.com/docker/setup-buildx-action
- name: Set up Docker Buildx
id: buildx
uses: docker/setup-buildx-action@v2

-
name: Build and push
uses: docker/build-push-action@v3
with:
platforms: linux/amd64,linux/arm64,linux/arm/v7
context: ./
file: ./Dockerfile
push: true
tags: toddrob/searcharr:beta
platforms: linux/amd64,linux/arm64,linux/arm/v7,linux/arm64/v8
tags: toddrob/searcharr:latest
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the beta.yml workflow file, so the docker tag needs to be toddrob/searcharr:beta.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to still be incorrect

22 changes: 18 additions & 4 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
FROM python:3.11-slim-buster
FROM python:slim
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I like to pin the python version, so it won't automatically start using v12 without me testing it. I'm not too concerned about slim vs. slim-buster, but what is your reasoning for slim over slim-buster? It appeared slim-buster supported more architectures when I was looking before.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slim uses debian bullseye and buster uses debian buster

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think slim should be used over slim-buster? And please update to pin python 3.11.


LABEL org.opencontainers.image.source="https://github.com/toddrob99/searcharr"
LABEL org.opencontainers.image.description="Docker for SEARCHARR"
LABEL Name=Searcharr Version="v1.2"

ARG TARGETPLATFORM BUILDPLATFORM
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of these? I don't see the args being used anywhere.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARG TARGETPLATFORM BUILDPLATFORM is a must for multi architecture builds

and using labels are not necessary but for tagging name and dockerhub repository to this github repository it is used...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the ARGs what was stopping the arm64/v8 arch build from working? Why do the other archs work without these ARGs?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove -beta from the Dockerfile version and increment to v1.3


LABEL Name=Searcharr Version=1.2

WORKDIR /app

RUN chmod -R 777 /app && \
chmod -R +x /app && \
chmod -R 705 /app
Comment on lines +12 to +14
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gives full permissions to the /app directory in case of any Permissions Denied errors

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you encounter any Permission Denied errors? I have not, and I have not received any reports of such issues. This seems unnecessary.


ADD . /app

RUN python3 -m pip install --upgrade pip
RUN python3 -m pip install -r requirements.txt
RUN apt-get update -y && \
apt-get upgrade -y
toddrob99 marked this conversation as resolved.
Show resolved Hide resolved

RUN python3 -m pip install --upgrade pip && \
python3 -m pip install -r requirements.txt && \
apt-get autoremove -y

CMD ["python3", "searcharr.py"]
2 changes: 1 addition & 1 deletion docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: '3.0'
services:
searcharr:
container_name: searcharr
image: toddrob/searcharr:latest
build: .
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why build instead of pulling the image? That will require that users clone the repo instead of just creating a docker-compose.yml file.

volumes:
- ./data:/app/data
- ./logs:/app/logs
Expand Down