Write quadlet units to temporary files and rename

[simonbrauner]

Fixes: #29004

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

• Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
  commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
  the sign-off email address. See CONTRIBUTING.md
  for more information.
• Referenced issues using Fixes: #00000 in commit message (if applicable)
• Tests have been added/updated (or no tests are needed)
• Documentation has been updated (or no documentation changes are needed)
• All commits pass make validatepr (format/lint checks)
• Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

Quadlet generator does not produce corrupted units, it writes to a temporary file and renames it later instead.

[Luap99]

we already have a package which handles all this "go.podman.io/storage/pkg/ioutils"

ioutils.NewAtomicFileWriter()

I haven't checked how many new packages that will pull in for quadlet, it might trigger the bloat check (binary size) but we should try that first instead of reimplementing the logic again

[simonbrauner]

The binary is 340858 bytes bigger.

I implemented the change, did I do it idiomatically or is it better this way?

diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go
index 447957e8b6..5565f56197 100644
--- a/cmd/quadlet/main.go
+++ b/cmd/quadlet/main.go
@@ -201,18 +201,19 @@ func generateServiceFile(service *parser.UnitFile) error {
                fmt.Sprintf("Automatically generated by %s", os.Args[0]),
                "")
 
-       cw, err := ioutils.NewAtomicFileWriter(service.Path, 0o644)
+       opts := &ioutils.AtomicFileWriterOptions{ExplicitCommit: true}
+       cw, err := ioutils.NewAtomicFileWriterWithOpts(service.Path, 0o644, opts)
        if err != nil {
                return fmt.Errorf("unable to create atomic writer for service file %s: %w", service.Path, err)
        }
+       defer cw.Close()
 
        if err := service.Write(cw); err != nil {
-               cw.Close()
                return fmt.Errorf("unable to write to atomic writer for service file %s: %w", service.Path, err)
        }
 
-       if err := cw.Close(); err != nil {
-               return fmt.Errorf("unable to close atomic writer for service file %s: %w", service.Path, err)
+       if err := cw.Commit(); err != nil {
+               return fmt.Errorf("unable to commit atomic writer for service file %s: %w", service.Path, err)
        }
 
        return nil

[Luap99]

yes the explicit commit is better otherwise we still commit the file when we failed to write it fully

[Luap99]

about the binary size we get a a bunch of new packages dragged in

$ ./contrib/dependencies/dependencies.sh -c quadlet diff main pr/29011
Switched to branch 'main'
Your branch is ahead of 'origin/master' by 19563 commits.
  (use "git push" to publish your local commits)
Switched to branch 'pr/29011'
3a4,29
> context
> crypto
> crypto/cipher
> crypto/internal/boring
> crypto/internal/boring/sig
> crypto/internal/entropy
> crypto/internal/fips140
> crypto/internal/fips140/aes
> crypto/internal/fips140/aes/gcm
> crypto/internal/fips140/alias
> crypto/internal/fips140/check
> crypto/internal/fips140deps/byteorder
> crypto/internal/fips140deps/cpu
> crypto/internal/fips140deps/godebug
> crypto/internal/fips140/drbg
> crypto/internal/fips140/hmac
> crypto/internal/fips140only
> crypto/internal/fips140/sha256
> crypto/internal/fips140/sha3
> crypto/internal/fips140/sha512
> crypto/internal/fips140/subtle
> crypto/internal/impl
> crypto/internal/randutil
> crypto/internal/sysrand
> crypto/sha256
> crypto/subtle
4a31
> encoding/binary
5a33
> encoding/hex
8a37
> golang.org/x/sys/unix
14a44
> go.podman.io/storage/pkg/ioutils
15a46
> hash
59a91
> math/rand/v2
Already on 'pr/29011'

The crypto stuff is what likely bloats us the most, and seems to be only used by HashData() in ioutils and I Do not see any callers of that function in the podman code so I wonder what removing it does to the binary size. might be an easy improvement.

Otherwise we could try to split the package to only have the atomic writer in a separate one with less deps

The reason we care about the bianry size so much is because as systemd generator this thing gets pulled in the initramfs