Setting a feature to enabled: false doesn't stop the container if previously installed

[bcurran3]

Setting a previously enabled application as false should should stop the container on subsequent runs of the playbook. Currently it continues running.

[davestephens]

This is actually something I've had in my head to fix for a while, I just need to come up with a clean way to do it.

[animeai]

The simple fix is to add code similar to: name: Remove Old Sickrage Docker Container docker_container: name: sickrage state: absent keep_volumes: true Put within an if function at the top of every /tasks/*.yml so it runs if the container is set to "false". This is inelegant though and would be better addressed by a single task that runs at the start of the playbook that shuts down all containers that are set to false.

[bcurran3]

I thought that task didn't run unless it was set to enabled?

Example from nas.yml:

• import_tasks: tasks/openhab.yml when: (openhab_enabled | default(False)) tags: openhab

I'm still new to this so maybe I'm wrong...

..but I would think any cleanup would need to happen in nas.yml

[animeai]

To extrapolate on my initial remark, the tasks would need to be run every time and the rest wrapped in an "if true" statement - as I said, inelegant.

[davestephens]

Yep, that's correct, and it would probably need to be rejigged somehow to cater for stopping containers (and also why I've not tackled it yet 😁)

On Thu, 9 Jan 2020, 19:30 bcurran3, notifications@github.com wrote:

I thought that task didn't run unless it was set to enabled?

Example from nas.yml:

• import_tasks: tasks/openhab.yml when: (openhab_enabled | default(False)) tags: openhab

I'm still new to this so maybe I'm wrong...

..but I would think any cleanup would need to happen in nas.yml

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/davestephens/ansible-nas/issues/207?email_source=notifications&email_token=AAFDGMHCFDJFB2FWDX5WNQ3Q453N3A5CNFSM4KER2S3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIRPULQ#issuecomment-572717614, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFDGMHBDUEMPUCTHJVKL43Q453N3ANCNFSM4KER2S3A .

[bcurran3]

I don't understand why you couldn't just add a docker stop task_name_variable command for each task in nas.yml when the container is flagged as not enabled?

[davestephens]

Because DRY. I'd like to do it elegantly.

[bcurran3]

I'm not a DRY follower. I'm a GBB follower. Make it Good (work - and fine with incremental improvements). Make it Better. Make it Best.

What is required to do it elegantly to your satisfaction?

[bcurran3]

@davestephens Still would like to know your requirements on doing it gracefully... i.e. I can't try to move forward on this without some sort of direction/guidance to do it right.

[PurpleNinja225]

We could maybe add a post_tasks script that checked for which {{ application }}_enabled: true and flush any docker applications matching false :thinking:

[davestephens]

Yeah, it's something I've been thinking about in the background as it'd be nice to have!

Few thoughts:

• Right now applications are turned on/off at the role level, so having a stop task within the role currently isn't possible. If it wasn't, would we want to do this? Feels a bit messy.

• We could have a role at the end of execution that stops all the things that shouldn't be running. If we did this, how would that role determine things that should be switched off? Presumably by looking at all the *_enabled variables and doing something with the ones that are set to no. How would all or those variables be collected up to be acted upon?

• Something else entirely I've not considered?

On Thu, 23 Apr 2020, 01:46 bcurran3, notifications@github.com wrote:

@davestephens https://github.com/davestephens Still would like to know your requirements on doing it gracefully... i.e. I can't try to move forward on this without some sort of direction/guidance to do it right.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davestephens/ansible-nas/issues/207#issuecomment-618112355, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFDGMAMUXHJII3WWHKTM53RN6FVBANCNFSM4KER2S3A .

[bcurran3]

I've got a cleanup.yml task inserted at the end of my nas.yml that handles this. Simply the code below for each app.

- name: organizr stop if disabled but running
  ignore_errors: true
  docker_container:
    name: organizr
    state: stopped
  when: organizr_enabled == false

My ansible-nas install is not up to date (very far out of date), but I don't see why this logic couldn't be added to the end of each main.yml.

[PurpleNinja225]

Since the role is only imported if {{ application }}_enabled: true that task would never run (i think). However putting at the end of the main nas.yml still worked. I'm down for a solution like this if we can get it to loop the enabled applications because implementing as is would make nas.yml another 30% longer.

[PurpleNinja225]

quick and dirty, but this should do the trick

EDIT: and by work I mean adding this to the end of nas.yml, ooooooorrrrr we can make it its own task.yml and import it as a post task

  - name: Cleanup no longer used Applications
    docker_container:
      name: "{{ item.name }}"
      state: absent
    when: "{{ item.enabled }} == false"
    loop:
      - { name: 'pyload', enabled: 'pyload_enabled' }
      - { name: 'mylar', enabled: 'mylar_enabled' }
      - { name: 'jackett', enabled: 'jackett_enabled' }
      - { name: 'nzbget', enabled: 'nzbget_enabled' }
      - { name: 'airsonic', enabled: 'airsonic_enabled' }
      - etc

[davestephens]

Yeah, this is exactly the approach I don't want - it's yet another place to have to remember to update when new apps are added/removed, which I've taken steps to remove (traefik being a notable one).

There will be somewhere where all resultant variables are stored after the playbook is calculated that can be parsed for their state - we would need to filter that for _enabled and act upon it.

[viktor-c]

This could be closed, because at this point nas.yml was updated, each role cleans up it's own containers.

[davestephens]

I've been working on merging @anarion80 's mega-PR, should be finished tomorrow. Once that's done I'll close this 👍