`dokku_clone` isn’t idempotent

AndrewKvalheim commented 1 year ago

Description of problem

dokku_clone always re-deploys the app even if the source hasn’t changed.

Steps to Reproduce

Stabilize the example from the documentation by referencing a commit:

- dokku_clone:
    app: inflector
    repository: https://github.com/cakephp/inflector.cakephp.org.git
    version: efd6065f3663cba3f641386bf6b1880bc427eff8

Run the task twice.

Actual Results

The task is imperative; the app is deployed twice.

Expected Results

The task is declarative; the app is deployed once.

Environment Information

dokku_bot.ansible_dokku v2022.10.17

This is required! Issues missing this information may be closed.

dokku report inflector output

``` -----> uname: Linux ubuntu2204.localdomain 5.15.0-69-generic #76-Ubuntu SMP Fri Mar 17 17:19:29 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux -----> memory: total used free shared buff/cache available Mem: 1975 308 117 7 1549 1474 Swap: 2047 4 2043 -----> docker version: Client: Docker Engine - Community Version: 24.0.2 API version: 1.43 Go version: go1.20.4 Git commit: cb74dfc Built: Thu May 25 21:51:00 2023 OS/Arch: linux/amd64 Context: default Server: Docker Engine - Community Engine: Version: 24.0.2 API version: 1.43 (minimum version 1.12) Go version: go1.20.4 Git commit: 659604f Built: Thu May 25 21:51:00 2023 OS/Arch: linux/amd64 Experimental: false containerd: Version: 1.6.21 GitCommit: 3dce8eb055cbb6872793272b4f20ed16117344f8 runc: Version: 1.1.7 GitCommit: v1.1.7-0-g860f061 docker-init: Version: 0.19.0 GitCommit: de40ad0 -----> docker daemon info: Client: Docker Engine - Community Version: 24.0.2 Context: default Debug Mode: true Plugins: buildx: Docker Buildx (Docker Inc.) Version: v0.10.5 Path: /usr/libexec/docker/cli-plugins/docker-buildx compose: Docker Compose (Docker Inc.) Version: v2.18.1 Path: /usr/libexec/docker/cli-plugins/docker-compose Server: Containers: 4 Running: 2 Paused: 0 Stopped: 2 Images: 7 Server Version: 24.0.2 Storage Driver: overlay2 Backing Filesystem: extfs Supports d_type: true Using metacopy: false Native Overlay Diff: true userxattr: false Logging Driver: json-file Cgroup Driver: systemd Cgroup Version: 2 Plugins: Volume: local Network: bridge host ipvlan macvlan null overlay Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog Swarm: inactive Runtimes: io.containerd.runc.v2 runc Default Runtime: runc Init Binary: docker-init containerd version: 3dce8eb055cbb6872793272b4f20ed16117344f8 runc version: v1.1.7-0-g860f061 init version: de40ad0 Security Options: apparmor seccomp Profile: builtin cgroupns Kernel Version: 5.15.0-69-generic Operating System: Ubuntu 22.04.2 LTS OSType: linux Architecture: x86_64 CPUs: 2 Total Memory: 1.929GiB Name: ubuntu2204.localdomain ID: 92bd493e-65c3-4b64-90ca-91286c10b316 Docker Root Dir: /var/lib/docker Debug Mode: false Experimental: false Insecure Registries: 127.0.0.0/8 Live Restore Enabled: false -----> git version: git version 2.34.1 -----> sigil version: 0.9.0build+bc921b7 -----> herokuish version: herokuish: v0.6.0 buildpacks: heroku-buildpack-multi v1.2.0 heroku-buildpack-ruby v254 heroku-buildpack-nodejs v213 heroku-buildpack-clojure v90 heroku-buildpack-python v232 heroku-buildpack-java v72 heroku-buildpack-gradle v39 heroku-buildpack-scala v96 heroku-buildpack-play v26 heroku-buildpack-php v234 heroku-buildpack-go v174 heroku-buildpack-nginx v23 buildpack-null v3 -----> dokku version: dokku version 0.30.6 -----> plugn version: plugn: 0.12.0build+3a27594 -----> dokku plugins: 00_dokku-standard 0.30.6 enabled dokku core standard plugin 20_events 0.30.6 enabled dokku core events logging plugin app-json 0.30.6 enabled dokku core app-json plugin apps 0.30.6 enabled dokku core apps plugin builder 0.30.6 enabled dokku core builder plugin builder-dockerfile 0.30.6 enabled dokku core builder-dockerfile plugin builder-herokuish 0.30.6 enabled dokku core builder-herokuish plugin builder-lambda 0.30.6 enabled dokku core builder-lambda plugin builder-null 0.30.6 enabled dokku core builder-null plugin builder-pack 0.30.6 enabled dokku core builder-pack plugin buildpacks 0.30.6 enabled dokku core buildpacks plugin caddy-vhosts 0.30.6 enabled dokku core caddy-vhosts plugin certs 0.30.6 enabled dokku core certificate management plugin checks 0.30.6 enabled dokku core checks plugin common 0.30.6 enabled dokku core common plugin config 0.30.6 enabled dokku core config plugin cron 0.30.6 enabled dokku core cron plugin docker-options 0.30.6 enabled dokku core docker-options plugin domains 0.30.6 enabled dokku core domains plugin enter 0.30.6 enabled dokku core enter plugin git 0.30.6 enabled dokku core git plugin haproxy-vhosts 0.30.6 enabled dokku core haproxy-vhosts plugin logs 0.30.6 enabled dokku core logs plugin network 0.30.6 enabled dokku core network plugin nginx-vhosts 0.30.6 enabled dokku core nginx-vhosts plugin plugin 0.30.6 enabled dokku core plugin plugin proxy 0.30.6 enabled dokku core proxy plugin ps 0.30.6 enabled dokku core ps plugin registry 0.30.6 enabled dokku core registry plugin repo 0.30.6 enabled dokku core repo plugin resource 0.30.6 enabled dokku core resource plugin run 0.30.6 enabled dokku core run plugin scheduler 0.30.6 enabled dokku core scheduler plugin scheduler-docker-local 0.30.6 enabled dokku core scheduler-docker-local plugin scheduler-null 0.30.6 enabled dokku core scheduler-null plugin shell 0.30.6 enabled dokku core shell plugin ssh-keys 0.30.6 enabled dokku core ssh-keys plugin storage 0.30.6 enabled dokku core storage plugin trace 0.30.6 enabled dokku core trace plugin traefik-vhosts 0.30.6 enabled dokku core traefik-vhosts plugin =====> inflector app-json information App json computed selected: app.json App json global selected: app.json App json selected: =====> inflector app information App created at: 1685404333 App deploy source: git-sync App deploy source metadata: https://github.com/cakephp/inflector.cakephp.org.git#efd6065f3663cba3f641386bf6b1880bc427eff8 App dir: /home/dokku/inflector App locked: false =====> inflector builder information Builder build dir: Builder computed build dir: Builder computed selected: Builder global build dir: Builder global selected: Builder selected: =====> inflector builder-dockerfile information Builder dockerfile computed dockerfile path: Dockerfile Builder dockerfile global dockerfile path: Dockerfile Builder dockerfile dockerfile path: =====> inflector builder-herokuish information Builder herokuish computed allowed: true Builder herokuish global allowed: true Builder herokuish allowed: =====> inflector builder-lambda information Builder lambda computed lambdayml path: lambda.yml Builder lambda global lambdayml path: lambda.yml Builder lambda lambdayml path: =====> inflector builder-pack information Builder pack computed projecttoml path: project.toml Builder pack global projecttoml path: project.toml Builder pack projecttoml path: =====> inflector buildpacks information Buildpacks computed stack: gliderlabs/herokuish:latest-20 Buildpacks global stack: Buildpacks list: Buildpacks stack: =====> inflector caddy information Caddy image: lucaslorentz/caddy-docker-proxy:2.7 Caddy letsencrypt email: Caddy letsencrypt server: https://acme-v02.api.letsencrypt.org/directory Caddy log level: ERROR Caddy polling interval: 5s Caddy tls internal: false =====> inflector ssl information Ssl dir: /home/dokku/inflector/tls Ssl enabled: false Ssl hostnames: Ssl expires at: Ssl issuer: Ssl starts at: Ssl subject: Ssl verified: =====> inflector checks information Checks disabled list: none Checks skipped list: none Checks computed wait to retire: 60 Checks global wait to retire: 60 Checks wait to retire: =====> inflector cron information Cron task count: 0 =====> inflector docker options information Docker options build: Docker options deploy: --restart=on-failure:10 Docker options run: =====> inflector domains information Domains app enabled: true Domains app vhosts: inflector.dokku.me Domains global enabled: true Domains global vhosts: dokku.me =====> inflector git information Git deploy branch: master Git global deploy branch: master Git keep git dir: false Git rev env var: GIT_REV Git sha: efd6065 Git source image: Git last updated at: 1685404434 =====> inflector haproxy information Haproxy image: byjg/easy-haproxy:4.3.0 Haproxy letsencrypt email: Haproxy letsencrypt server: https://acme-v02.api.letsencrypt.org/directory Haproxy log level: ERROR =====> inflector logs information Logs computed max size: 10m Logs global max size: 10m Logs global vector sink: Logs max size: Logs vector sink: =====> inflector network information Network attach post create: Network attach post deploy: Network bind all interfaces: false Network computed attach post create: Network computed attach post deploy: Network computed bind all interfaces: false Network computed initial network: Network computed tld: Network global attach post create: Network global attach post deploy: Network global bind all interfaces: false Network global initial network: Network global tld: Network initial network: Network static web listener: Network tld: Network web listeners: 172.17.0.3:5000 =====> inflector nginx information Nginx access log format: Nginx access log path: /var/log/nginx/inflector-access.log Nginx bind address ipv4: Nginx bind address ipv6: :: Nginx client max body size: Nginx disable custom config: false Nginx error log path: /var/log/nginx/inflector-error.log Nginx global hsts: true Nginx computed hsts: true Nginx hsts: Nginx hsts include subdomains: true Nginx hsts max age: 15724800 Nginx hsts preload: false Nginx computed nginx conf sigil path: nginx.conf.sigil Nginx global nginx conf sigil path: nginx.conf.sigil Nginx nginx conf sigil path: Nginx proxy buffer size: 4096 Nginx proxy buffering: on Nginx proxy buffers: 8 4096 Nginx proxy busy buffers size: 8192 Nginx proxy read timeout: 60s Nginx last visited at: 1685404380 Nginx x forwarded for value: $remote_addr Nginx x forwarded port value: $server_port Nginx x forwarded proto value: $scheme Nginx x forwarded ssl: =====> inflector proxy information Proxy enabled: true Proxy port map: http:80:5000 Proxy type: nginx =====> inflector ps information Deployed: true Processes: 1 Ps can scale: true Ps computed procfile path: Procfile Ps global procfile path: Procfile Ps procfile path: Ps restart policy: on-failure:10 Restore: true Running: true Status web 1: running (CID: 795461d1146) =====> inflector registry information Registry computed image repo: dokku/inflector Registry computed push on release: false Registry computed server: Registry global push on release: Registry global server: Registry image repo: Registry push on release: Registry server: Registry tag version: =====> inflector resource information =====> inflector scheduler information Scheduler computed selected: docker-local Scheduler global selected: docker-local Scheduler selected: =====> inflector scheduler-docker-local information Scheduler docker local disable chown: Scheduler docker local init process: true Scheduler docker local parallel schedule count: =====> inflector storage information Storage build mounts: Storage deploy mounts: Storage run mounts: =====> inflector traefik information Traefik api enabled: false Traefik api vhost: traefik.dokku.me Traefik basic auth password: Traefik basic auth username: Traefik dashboard enabled: false Traefik image: traefik:v2.8 Traefik letsencrypt email: Traefik letsencrypt server: https://acme-v02.api.letsencrypt.org/directory Traefik log level: ERROR Traefik priority: ```

Additional information

Given Ansible’s convention of idempotence, I expected there to be some way to express the desired state, e.g. that the app be running from a specific source, and have the playbook make changes only as necessary to reach that state.

I’ve found that I can roughly accomplish this by comparing the Git revision of the source to that of the deployed app—

- name: Synchronize source
  dokku_clone: &clone
    app: inflector
    repository: https://github.com/cakephp/inflector.cakephp.org.git
    version: efd6065f3663cba3f641386bf6b1880bc427eff8
    build: false

- name: Look up revision of source
  command: dokku git:report inflector --git-sha
  register: git_sha
  changed_when: false

- name: Look up revision of deployement
  command: dokku apps:report inflector --app-deploy-source-metadata
  register: deploy_source_metadata
  changed_when: false

- name: Deploy
  when: git_sha.stdout and "#{}".format(git_sha.stdout) not in deploy_source_metadata.stdout
  dokku_clone:
    <<: *clone
    build: true

—but this feels unnecessarily convoluted for behavior that I initially expected to find in ansible_dokku or even Dokku itself.

josegonzalez commented 1 year ago

What version of the modules are you using?

AndrewKvalheim commented 1 year ago

v2022.10.17, the latest published

josegonzalez commented 1 year ago

Looks like we try and get the commit sha but never compare it to what is being deployed: https://github.com/dokku/ansible-dokku/blob/master/library/dokku_clone.py#LL69C5-L69C12

Additionally - at least on my system - the sha is a short sha:

~# dokku git:report social-notifications --git-sha
54f6e18

Would you be willing to contribute the fix to the ansible modules?

AndrewKvalheim commented 1 year ago

Possibly, but I’m not sure how the various parts are supposed to work, e.g.:

Should dokku git:sync be able to --build-if-necessary? Is that a feature that Dokku should have anyway, or would it exist just to serve ansible_dokku? Is it acceptable for this peripheral codebase to impose constraints on Dokku’s main codebase?
Is ansible_dokku intended to faithfully adapt Dokku’s controls to Ansible playbooks, or is it a goal of this project to host higher level features?
Is it deliberate that ansible_dokku has no process management library?
Is deploy source metadata contractually stable and machine readable, or is it an unstructured annotation only intended for humans?
Is it necessary to re-deploy if the running app was deployed from an identical commit but using a method other than git:sync, such as by pushing? Is there a generic way to check for this or must it be implemented for each possible deployment method?

AndrewKvalheim commented 1 year ago

Additionally - at least on my system - the sha is a short sha:
~# dokku git:report social-notifications --git-sha
54f6e18

In my example this is addressed by searching for the string #54f6e18 anywhere in the deploy source metadata. It feels fragile.

josegonzalez commented 1 year ago

A few answers:

I don't think we can safely do a git:sync --build-if-necessary. For instance, if the remote ref the user uses changes (say, they always use master) then how should we handle that? How do we know it's a branch instead of a tag (which may also be reused)?
I think a goal of the ansible modules is to provide a porcelain on top of Dokku that makes sense for how ansible works. Dokku's commands may not be idempotent, but that doesn't stop the ansible modules from building idempotency in (by doing extra checks). Some modules in this codebase already do that.
I don't think it's deliberate that anything is missing, just that things haven't been built. It's open source, so if you make a pull request that is mergable and provides value, we can review it and get the features you're looking for released.
Anything coming out of a :report subcommand should be machine readable, though sometimes the underlying data may change in terms of what it represents. For instance, if we turn something from a string to a list of strings, then it's likely that the output will be comma-delimited. That said, the Dokku project tries hard not to change those, since that interface is used by other tools (such as ansible doku).
The --build flag for git:sync quite literally tells dokku to rebuild. That is expected behavior. If you don't want to rebuild, don't use the flag? We could add a sort of --ignore-ref-if-matches or something, which is a bit different from your --build-if-necessary, but doesn't imply that we're actually checking underlying state (just if what you specified already matches what is the current state of the repo).

The short sha being used is probably just because I didn't envision the use case of relying on that in other tooling. If you want to change it, the line upstream is here (I'd take a pull request ;) ).

josegonzalez commented 1 year ago

If you have more questions, feel free to catch me on the dokku discord/slack (I am savant on both) :)

AndrewKvalheim commented 3 months ago

Should dokku git:sync be able to --build-if-necessary?

Looks like it can now.

AndrewKvalheim commented 3 months ago

What do you think about changing the interface to work like this?

`build`	Before	After
default	`--build`	`--build-if-changes`
`false`	none	none
`true`	`--build`	`--build`

robotski commented 3 months ago

@AndrewKvalheim I've implemented this change in this branch but haven't written any new test cases, so I'm not 100% sure about correctness.

robotski commented 3 months ago

Added a simple test to Molecule that runs --build followed by --build-if-changes and asserts the app is only deployed once. Everything seems correct but I'm open to further test cases.

AndrewKvalheim commented 3 months ago

Additionally - at least on my system - the sha is a short sha:
~# dokku git:report social-notifications --git-sha
54f6e18
In my example this is addressed by searching for the string #54f6e18 anywhere in the deploy source metadata. It feels fragile.

No longer necessary as of dokku/dokku#6776

robotski commented 3 months ago

Right, but dokku_clone still rebuilds the app and registers it as changed.

AndrewKvalheim commented 3 months ago

[matching an abbreviated hash] is no longer necessary

dokku / ansible-dokku