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.

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

[BUG] file.managed unnecessarily fetches remote sources to cache #66342

Closed
lkubb opened this issue Apr 10, 2024 · 2 comments
Closed

[BUG] file.managed unnecessarily fetches remote sources to cache #66342

lkubb opened this issue Apr 10, 2024 · 2 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@lkubb
Copy link
Contributor

lkubb commented Apr 10, 2024

Description
Using file.managed with a remote source (e.g. https://) usually requires specifying a source_hash in some way. I would expect it to be used to check if an existing file's contents need to change, which is not the case. Instead, Salt uses this source hash to check if a file downloaded to cache is corrupt only, calculates its source hash and compares it to the existing file.

This behavior wastes a lot of resources (CPU cycles, disk space, bandwidth) both locally and on the remote and is completely unnecessary if we already had what we wanted before starting all this work. We're not trying to verify remote sources or manage the cache after all.

Related: #64373

Setup
irrelevant

Steps to Reproduce the behavior

$ echo "Am I not good enough for you?" > testfile
$ sha256sum testfile
b099df4a11f400546110027ffd2a5af88dd541ad353c9a0ebdc95f28ee8430cb  testfile
$ salt-call state.single file.managed $(pwd)/testfile source=https://127.0.0.1:1337/nonexistent source_hash=b099df4a11f400546110027ffd2a5af88dd541ad353c9a0ebdc95f28ee8430cb
[ERROR   ] Failed to cache https://127.0.0.1:1337/nonexistent: Error: [Errno 111] Connection refused reading /nonexistent
local:
----------
          ID: /tmp/testfile
    Function: file.managed
      Result: False
     Comment: Failed to cache https://127.0.0.1:1337/nonexistent: Error: [Errno 111] Connection refused reading /nonexistent
     Started: 17:42:21.671160
    Duration: 14.082 ms
     Changes:

Expected behavior

local:
----------
          ID: /tmp/testfile
    Function: file.managed
      Result: True
     Comment: File /tmp/testfile is in the correct state
     Started: 17:45:10.117584
    Duration: 7.519 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:   7.519 ms

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3006.7

Python Version:
        Python: 3.10.13 (main, Feb 19 2024, 03:31:20) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.11
     gitpython: 3.1.42
        Jinja2: 3.1.3
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.15.1
         smmap: 5.0.1
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: rocky 9.3 Blue Onyx
        locale: utf-8
       machine: x86_64
       release: 5.14.0-362.13.1.el9_3.x86_64
        system: Linux
       version: Rocky Linux 9.3 Blue Onyx

Additional context
PR incoming

@lkubb lkubb added Bug broken, incorrect, or confusing behavior needs-triage labels Apr 10, 2024
@network-shark
Copy link
Contributor

I do have a state which bootstraps freebsd and was wondering why it takes so long on each run. I thought it was my bad and gave up. thank you for bringing this up !

@lkubb
Copy link
Contributor Author

lkubb commented May 14, 2024

Closed by #66343 (backport for 3006: #66513)

@lkubb lkubb closed this as completed May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

No branches or pull requests

3 participants