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.

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

"caddy add-package", "caddy remove-package" could be idempotent #6548

Open
gedw99 opened this issue Aug 29, 2024 · 8 comments · May be fixed by #6577
Open

"caddy add-package", "caddy remove-package" could be idempotent #6548

gedw99 opened this issue Aug 29, 2024 · 8 comments · May be fixed by #6577
Labels
bug 🐞 Something isn't working good first issue 🐤 Good for newcomers

Comments

@gedw99
Copy link

gedw99 commented Aug 29, 2024 •
edited
Loading

For idempotency, it would be useful if caddy add-package and caddy remove-package could be idempotent.

caddy remove-package, will return "Error: package is not added" if I do it twice.

caddy add-package, will return "Error: package is already added", if I do it twice.

This idempotency is a nice thing in any architecture, when managing a fleet and building a fleet, because you can just store the package list somewhere and then do add-package without worrying if it was done in the past. Treat them like Cows, not delicate kittens :)

what is the best way to handle this ?

```caddy add-package github.com/x/y```, will return "Warn: package github.com/x/y is already added",
@gedw99 gedw99 mentioned this issue Aug 29, 2024
Closed
@mholt mholt changed the title caddy add-package and caddy add-package could be idempotent, so that its easy to upgrade and downgrade modules in a large fleet without race conditions causing issues. caddy add-package and caddy remove-package could be idempotent, so that its easy to upgrade and downgrade modules in a large fleet without race conditions causing issues. Aug 29, 2024
@mholt
Copy link
Member

mholt commented Aug 29, 2024

I guess that makes sense. Want to submit a PR?

@mholt mholt added the bug 🐞 Something isn't working label Aug 29, 2024
@mholt mholt added this to the v2.8.5 milestone Aug 29, 2024
@mholt mholt added the good first issue 🐤 Good for newcomers label Aug 29, 2024
@gedw99
Copy link
Author

gedw99 commented Aug 29, 2024 •
edited
Loading

Hey @mholt

Can you also comment on #6549, since it's related.

Might have some bearing on this PR. I would like to get this as tight as possible since it is SBOM related.

@gedw99 gedw99 changed the title caddy add-package and caddy remove-package could be idempotent, so that its easy to upgrade and downgrade modules in a large fleet without race conditions causing issues. "caddy add-package", "caddy remove-package" could be idempotent Aug 29, 2024
@mohammed90 mohammed90 mentioned this issue Aug 29, 2024
Closed
@mholt
Copy link
Member

mholt commented Sep 2, 2024

I think we should just stick to idempotency for now. As mentioned in other issues, build automation with these commands isn't really intended (there are better tools for the job like xcaddy).

@gedw99
Copy link
Author

gedw99 commented Sep 2, 2024

Got it . Thanks @mholt for steering me .

@armadi1809 armadi1809 linked a pull request Sep 18, 2024 that will close this issue
Open
@loren-osborn
Copy link

loren-osborn commented Oct 9, 2024 •
edited
Loading

I wanted to confirm my understanding of this issue before I look deeper into it:
My understanding of idempotency is identical behavior, no matter how many times of the operation is performed.
My understanding of the use case, however, is that this is primarily for fleet deployment automation. I believe automated tools for fleet deployment primarily depend on exit codes for error detection, while human users primarily rely on error messages.

To clarify the desired behavior I suspect it would still be useful to display human readable error, messages to human users in a non-idempotent fashion (indicating that no change was made), while the exit code should behave idempotently. (Indicating no error deleting something already missing, or adding something already present.)

Is this the desired behavior?

@francislavoie
Copy link
Member

Correct @loren-osborn, see #6577 (comment)

@loren-osborn
Copy link

Looks like I should spend my time on other tasks if you already have a PR for this issue.

@gedw99
Copy link
Author

gedw99 commented Oct 9, 2024

agree that error codes are a key element automation.

@mholt mholt modified the milestones: v2.9.0-beta.3, v2.9.0-beta.4 Nov 1, 2024
@mholt mholt removed this from the v2.9.0-beta.4 milestone Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐞 Something isn't working good first issue 🐤 Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make add/remove package commands idempotent armadi1809/caddy
4 participants
@mholt @francislavoie @loren-osborn @gedw99