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

Use app folder for main JDownloader files #102

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

egvimo
Copy link
Contributor

@egvimo egvimo commented Nov 28, 2021

This PR moves the main app to a separate folder (app). This way this directory can be mounted as a volume which persists the data after an update.

THIS IS A BREAKING CHANGE! Because of the extra directory layer the mounts has to be changed.

I've created an image and tested this on my Kubernetes cluster. A small example for a pod:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: jdownloader
  labels:
    app: jdownloader
spec:
  replicas: 1
  selector:
    matchLabels:
      app: jdownloader
  template:
    metadata:
      labels:
        app: jdownloader
    spec:
      containers:
        - name: jdownloader
          image: jaymoulin/jdownloader
          env:
            - name: MYJD_USER
              value: "[email protected]"
            - name: MYJD_PASSWORD
              valueFrom:
                secretKeyRef:
                  name: my_jd_secret
                  key: password
          volumeMounts:
            - mountPath: /opt/JDownloader/app
              name: exec
            - mountPath: /opt/JDownloader/app/cfg
              name: cfg
            - mountPath: /opt/JDownloader/Downloads
              name: downloads
      volumes:
        - name: exec
          emptyDir: {}
        - name: cfg
          hostPath:
            path: /path/to/jd/cfg
            type: Directory
        - name: downloads
          hostPath:
            path: /path/to/downloads
            type: Directory

Fixes: #94, #91, #82 and maybe other related.

@egvimo egvimo force-pushed the feature/k8s-support branch from 4d9ab54 to 4720ee1 Compare November 28, 2021 17:08
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
debian.Dockerfile Outdated Show resolved Hide resolved
@jaymoulin
Copy link
Owner

related #94

@jaymoulin jaymoulin self-assigned this Nov 28, 2021
@jaymoulin jaymoulin added this to the 2.0.0 milestone Nov 28, 2021
@egvimo
Copy link
Contributor Author

egvimo commented Nov 28, 2021

If you accept this PR, I can update the readme file, too.

I also can change the paths and names if you think others are better suited. They were just my preference.

daemon.sh Outdated Show resolved Hide resolved
daemon.sh Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
debian.Dockerfile Outdated Show resolved Hide resolved
@jaymoulin
Copy link
Owner

@egvimo thank you for your contribution.

This is an effective implementation of #94 (comment) (which is still waiting for vote BTW) but I think it could have been easier.

Yes please change the README.md as everything must be related in the same commit. IMHO implementing only the exec subfolder + cp in config.sh would ease review, bug avoiding, documentation maintaining and understanding for migrating users. (even if I checked, I could have missed some behaviours).

@egvimo egvimo force-pushed the feature/k8s-support branch 2 times, most recently from cd3f8b0 to 90fa005 Compare November 28, 2021 19:00
@egvimo
Copy link
Contributor Author

egvimo commented Nov 28, 2021

@jaymoulin You're right. I've reverted all my preferences and optimizations. Now only the new exec folder is included. Everything else can be done separately.

daemon.sh Outdated Show resolved Hide resolved
@egvimo
Copy link
Contributor Author

egvimo commented Nov 28, 2021

After some thoughts I think app would be a better choice for the folder. What do you think?

@egvimo egvimo force-pushed the feature/k8s-support branch from 90fa005 to bf6a60d Compare December 1, 2021 18:44
@egvimo egvimo changed the title Use exec folder for main JDownloader files Use app folder for main JDownloader files Dec 1, 2021
@egvimo
Copy link
Contributor Author

egvimo commented Dec 1, 2021

I've changed the folder to app and updated everything accordingly. And I updated the K8s example with everything necessary to run the app.

@jaymoulin jaymoulin merged commit 4f55aff into jaymoulin:master Dec 2, 2021
@egvimo egvimo deleted the feature/k8s-support branch December 3, 2021 05:55
Repository owner locked as resolved and limited conversation to collaborators Dec 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker Swarm/Portainer Boot Loop
2 participants