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

First batch of changes for PR #10

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

Conversation

ezekieldas
Copy link

Hello! I wanted to present these changes before this becomes another project I never finish. Your work has inspired me to get all my *arrs and *arr pipeline automated. This is something I've been meaning to do for well over a year.

There are some significant changes here but they should still (generally) accomodate your original codebase/configuration. I'll summarize:

  • docker compose template is more jinja. I abstracted a lot of stuff here to be dependent on vars (eg, group_vars, host_vars, etc.)
  • tasks/main.yml has been given cleaner loops. This too reading more from vars.
  • And the vars I forgot to add in this commit, but a sample below. I'll include with next along with any other modifications you might suggest.
  • Testing: I'll do something in the next few days that supports your original configuration.

(afterthought: I could merge these vars into defaults/main.yml, with the elements you originially had in place (ie, transmission, flaresolver, sonarr-hd, etc.))

These are the vars I built on:

##
## This includes variables used with https://github.com/buildarr/ansible-playbooks
##

# docker_host_ip_ports: "0.0.0.0"

arrstack_env_dir: /usr/local/docker/arr
arrstack_data_dir_create: false

##
## network_mode: will take precendence over ports:
##
services:
  - service_name: sonarr
    image: lscr.io/linuxserver/sonarr
    image_tag: version-3.0.10.1567
    container_name: sonarr
    # network_mode: bridge
    port_healthcheck: 8989
    ports:
      - 8989:8989/tcp
    volumes:
      - ./sonarr_config:/config
#    depends_on:
#      - foobar
  - service_name: radarr
    image: lscr.io/linuxserver/radarr
    image_tag: latest
    container_name: radarr
    # network_mode: service:foobar
    port_healthcheck: 7878
    ports:
      - 7878:7878/tcp
    volumes:
      - ./radarr_config:/config
#    depends_on:
#      - foobar
  - service_name: prowlarr
    image: lscr.io/linuxserver/prowlarr
    image_tag: latest
    container_name: prowlarr
    # network_mode: service:foobar
    port_healthcheck: 9696
    ports:
      - 9696:9696/tcp
    volumes:
      - ./prowlarr_config:/config
#    depends_on:
#      - foobar

@Callum027
Copy link
Member

Callum027 commented Apr 24, 2024

Hi there @ezekieldas, thank you for your contribution. This is a really good bit of work.

I've actually been using an Ansible collection I wrote for my own use to deploy my media stack with Buildarr. I've just uploaded it to GitHub as the following repository:

https://github.com/buildarr/ansible-collection-mediastack

It does a bit more than these playbooks, such as setting up FlareSolverr for Prowlarr, Jellyfin and Jellyseerr.

If you don't mind, would you consider proposing your changes against the collection?

It needs some work to update it and add things like Molecule testing, but it'd be great if we could collaborate on making this something that can be released on Ansible Galaxy.

@ezekieldas
Copy link
Author

Hello! Yes, I'd be happy to collaborate on any/all things buildarr related. At an earlier exchange we had with a buildarr issue, you proposed having a chat on Discord. I'll try to get that setup at some point today so we can look at the details.

I want to offer another PR for this codebase just to get some ideas across. No expectations of merging the PR as it looks like you may abandon this repo (?) but I think it might help illustrate how I'm thinking of the automation pipeline in general.

Again, let's find a greater bandwidth space (Discord) to discuss. I'll see about getting that setup today.

Cheers!

@ezekieldas
Copy link
Author

A couple of afterthoughts:

  • I'll message you here once I have the Discord setup
  • We have some TZ differences. I think you're reading from NZ. I'm in US West coast.

image: "{{ arrstack_sonarr_container_uri }}:{{ arrstack_sonarr_container_tag }}"
hostname: sonarr-4k
restart: always
{% include 'gluetun.j2' ignore missing %}
Copy link
Member

Choose a reason for hiding this comment

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

Could you please include a way to disable including this using the Ansible inventory?

For example, I host my *Arrs on a NAS in my home, and don't need to setup VPNs to access it (and even then, I actually have a separate setup for all that).

@Callum027
Copy link
Member

Callum027 commented Apr 26, 2024

Apart from the gluetun comment I made (which I'm happy to treat as an optional example for people who need it), it seems reasonable.

I'm not going to put too much work into reviewing this as it's mainly an example of what can be done, more than anything. If we're going to do the collection thing, I'd like to put the work into making it nice and proper over there.

@ezekieldas
Copy link
Author

I have another PR queued up which potentially replaces this one. I've been using this as a means to test a full, first-time provision of a typical arrstack (ie, prowlarr, sonarr, radarr, flaresolverr + buildarr). There are specifics to my environment and network design which introduce complications in achieving functionality without manual intervention but nonetheless have proven to make this role + a buildarr layout more modular.

And I believe modularity is key here. It's difficult but necessary to try to accomodate as any arrangements as possible. I'll unpack more of that later. So speaking of modularity...

The unconditional include on the gluetun file was a mistake made in haste. However, I think it'd be good to include an example of how a full service configuration can be brought in without definining it as a dictionary. More options for modularity. And extending it in such a way that functionality is the designer's responsibility.

In using and refinining this specific repo I've been forced to 1) review my ansible-fu 2) understand buildarr extensively. I'm hoping this will offer a bit for the collection.

@ezekieldas
Copy link
Author

@Callum027 Overdue but I finally got my Discord working. My username is same as here. Please add me and/or send your Discord name.

@Callum027
Copy link
Member

@Callum027 Overdue but I finally got my Discord working. My username is same as here. Please add me and/or send your Discord name.

Thanks. You can join the Discord server I created for Buildarr using the following invite link.

https://discord.gg/9vkHv2PRuH

Copy link
Member

@Callum027 Callum027 left a comment

Choose a reason for hiding this comment

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

The comments I made here are simply addressing some things that would probably be transferred when we start working on the collection. I have more to say on how things should be designed, but let's discuss those when we start seriously looking at working on the collection.

For these example playbooks, I think this'll do for now.

protocol: http
url_base: null
api_key: "{{ arrstack_instance_apikeys['sonarr'] if arrstack_instance_apikeys is defined and 'sonarr' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
version: 3.0.10.1567
Copy link
Member

Choose a reason for hiding this comment

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

You generally don't want to explicitly set the version in the configuration, unless you're using a config file to generate a Docker Compose configuration using the buildarr compose command.

Comment on lines +40 to +56
strig00_bot:
notification_triggers:
on_grab: false
on_import: true
on_upgrade: true
on_rename: false
on_series_delete: false
on_episode_file_delete: false
on_episode_file_delete_for_upgrade: true
on_health_issue: false
include_health_warnings: false
on_application_update: true
tags: []
type: telegram
bot_token: {{ vault_telegram_bot_token }}
chat_id: {{ vault_telegram_chat_id }}
send_silently: false
Copy link
Member

Choose a reason for hiding this comment

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

Can you make the Telegram bot optional?

Comment on lines +72 to +78
{% if arrstack_analytics_disable is defined and arrstack_analytics_disable %}
analytics:
send_anonymous_usage_data: false
{% else %}
analytics:
send_anonymous_usage_data: true
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% if arrstack_analytics_disable is defined and arrstack_analytics_disable %}
analytics:
send_anonymous_usage_data: false
{% else %}
analytics:
send_anonymous_usage_data: true
{% endif %}
analytics:
send_anonymous_usage_data: {{ not (arrstack_analytics_disable | default(False)) | string | lower }}

Comment on lines +27 to +33
{% if arrstack_analytics_disable is defined and arrstack_analytics_disable %}
analytics:
send_anonymous_usage_data: false
{% else %}
analytics:
send_anonymous_usage_data: true
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% if arrstack_analytics_disable is defined and arrstack_analytics_disable %}
analytics:
send_anonymous_usage_data: false
{% else %}
analytics:
send_anonymous_usage_data: true
{% endif %}
analytics:
send_anonymous_usage_data: {{ not (arrstack_analytics_disable | default(False)) | string | lower }}

@@ -357,11 +358,12 @@ sonarr:

# Sonarr instance for grabbing anime shows.
sonarr-anime:
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] }}"
hostname: {{ hostname_sonarr | default('localhost') }}
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] if arrstack_instance_apikeys is defined and 'sonarr-anime' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
Copy link
Member

Choose a reason for hiding this comment

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

Putting in an incorrect value when something is not found is an anti-pattern I believe.

Instead, let Ansible raise an error.

Suggested change
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] if arrstack_instance_apikeys is defined and 'sonarr-anime' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] }}"

# Sonarr instance for grabbing anime shows.
sonarr-anime:
hostname: {{ hostname_sonarr | default('localhost') }}
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] if arrstack_instance_apikeys is defined and 'sonarr-anime' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
Copy link
Member

Choose a reason for hiding this comment

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

Putting in an incorrect value when something is not found is an anti-pattern I believe.

Instead, let Ansible raise an error.

Suggested change
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] if arrstack_instance_apikeys is defined and 'sonarr-anime' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
api_key: "{{ arrstack_instance_apikeys['sonarr-anime'] }}"

# Sonarr instance for grabbing 4K TV shows.
sonarr-4k:
hostname: {{ hostname_sonarr | default('localhost') }}
api_key: "{{ arrstack_instance_apikeys['sonarr-4k'] if arrstack_instance_apikeys is defined and 'sonarr-4k' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
Copy link
Member

Choose a reason for hiding this comment

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

Putting in an incorrect value when something is not found is an anti-pattern I believe.

Instead, let Ansible raise an error.

Suggested change
api_key: "{{ arrstack_instance_apikeys['sonarr-4k'] if arrstack_instance_apikeys is defined and 'sonarr-4k' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
api_key: "{{ arrstack_instance_apikeys['sonarr-4k'] }}"

# Sonarr instance for grabbing HD/SD TV shows.
sonarr-hd:
hostname: {{ hostname_sonarr | default('localhost') }}
api_key: "{{ arrstack_instance_apikeys['sonarr-hd'] if arrstack_instance_apikeys is defined and 'sonarr-hd' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
Copy link
Member

Choose a reason for hiding this comment

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

Putting in an incorrect value when something is not found is an anti-pattern I believe.

Instead, let Ansible raise an error.

Suggested change
api_key: "{{ arrstack_instance_apikeys['sonarr-hd'] if arrstack_instance_apikeys is defined and 'sonarr-hd' in arrstack_instance_apikeys else 'ansible_api_key_not_found' }}"
api_key: "{{ arrstack_instance_apikeys['sonarr-hd'] }}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants