这是indexloc提供的服务，不要输入任何密码
Skip to content

Add missing post-delete scripts to checks, caddy-vhosts and haproxy-vhosts #7455

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

Jump to bottom
Merged
merged 3 commits into from
Jan 13, 2025
Merged

Add missing post-delete scripts to checks, caddy-vhosts and haproxy-vhosts #7455

merged 3 commits into from
Jan 13, 2025

Conversation

@turicas
Copy link
Contributor

@turicas turicas commented Jan 13, 2025

This PR fixes #7443. Since this was a simple fix, I preferred not to create an issue/PR for each plugin.

Note: I'm not used to the Dokku test suite and couldn't run it properly here, so I just copied the post-delete from another plugin and changed the names accordingly. If you feel like a test is needed, could you please send me more context on how to do it?
The tests could be something like this:

# For checks

APP_NAME="test-app-checks-bug"
dokku apps:create "$APP_NAME"

dokku checks:report "$APP_NAME" | grep 'Checks wait to retire:'
# TODO: assert no value defined

dokku checks:set "$APP_NAME" wait-to-retire 120
dokku checks:report "$APP_NAME" | grep 'Checks wait to retire:'
# TODO: assert output has '120'

echo "$APP_NAME" | dokku apps:destroy "$APP_NAME"
dokku apps:create "$APP_NAME"
dokku checks:report "$APP_NAME" | grep 'Checks wait to retire:'
# TODO: assert no value defined
# For caddy-vhosts

APP_NAME="test-app-bug-plugin-caddy-data"
dokku apps:create "$APP_NAME"
dokku caddy:report "$APP_NAME" | grep 'Caddy tls internal'
# TODO: check output 'false' (default value)

dokku caddy:set "$APP_NAME" tls-internal true
dokku caddy:report "$APP_NAME" | grep 'Caddy tls internal'
# TODO: check output 'true'

echo "$APP_NAME" | dokku apps:destroy "$APP_NAME"
dokku apps:create "$APP_NAME"
dokku caddy:report "$APP_NAME" | grep 'Caddy tls internal' 
# TODO: check output 'false'

For haproxy I could not reproduce it since it seems that it ignores the app settings - the only settings that matter are --global, which is weird because the haproxy:report <app-name> shows "Haproxy image:" for each app (and it doesn't have the prefix "Global" as other plugins like domains). To clarify:

APP_NAME="test-app-bug-plugin-haproxy-data"
dokku apps:create "$APP_NAME"
dokku haproxy:report "$APP_NAME" | grep 'Haproxy image:'
# Gives: byjg/easy-haproxy:4.4.0

# New image for app: report does not show it
dokku haproxy:set "$APP_NAME" image new-value-for-app
dokku haproxy:report "$APP_NAME" | grep 'Haproxy image:'
# Result: byjg/easy-haproxy:4.4.0

# New image for global: report does show it
dokku haproxy:set --global image new-value-for-global
dokku haproxy:report "$APP_NAME" | grep 'Haproxy image:'
# Result: new-value-for-global

@josegonzalez
Copy link
Member

josegonzalez commented Jan 13, 2025

Yeah the report output always includes global values for apps, so thats fine.

@josegonzalez josegonzalez merged commit 6419acc into dokku:master Jan 13, 2025
github-actions bot pushed a commit that referenced this pull request Jan 13, 2025
Release 0.35.15
4245f13 
# History

## 0.35.15

Install/update via the bootstrap script:

```shell
wget -NP . https://dokku.com/install/v0.35.15/bootstrap.sh
sudo DOKKU_TAG=v0.35.15 bash bootstrap.sh
```

### Bug Fixes

- #7455: @turicas Add missing post-delete scripts to checks, caddy-vhosts and haproxy-vhosts

### Dependencies

- #7458: @dependabot[bot] chore(deps): bump traefik from 3.3.0 to 3.3.1 in /plugins/traefik-vhosts
- #7457: @dependabot[bot] chore(deps): bump google.golang.org/grpc from 1.69.2 to 1.69.4 in /tests/apps/gogrpc
@turicas turicas deleted the fix/missing-post-delete branch January 17, 2025 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

type: bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

All app's plugin properties must be deleted after the app is destroyed

2 participants

@turicas @josegonzalez