http.wait_for_successful_query state with wait_for and test=true blocks and waits

[rossengeorgiev]

Description of Issue/Question

The state waits for the wait_for amount of time even when test=True

Setup

test.sls

http://127.0.0.1:8000/:
  http.wait_for_successful_query:
    - wait_for: 120
    - request_interval: 5
    - status: 200

Steps to Reproduce Issue

salt-call -l debug --local state.sls test test=true

Versions Report

Salt Version:
           Salt: 2017.7.7

Dependency Versions:
           cffi: 1.6.0
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: 0.26.3
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.4
         Python: 2.7.5 (default, Jul 13 2018, 13:06:57)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.5.1804 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-862.11.6.el7.x86_64
         system: Linux
        version: CentOS Linux 7.5.1804 Core

[dwoz]

@rossengeorgiev Thanks for reporting this. It looks like this behavior was by design. The state will make the web request when test=True.

@techhat can you confirm this behavior is intended?

[rossengeorgiev]

I don't think it makes the request, hence why its waiting the full wait_for. I haven't looked at the code.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[sagetherage]

Waiting for @techhat to comment

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[techhat]

I cannot confirm whether this behavior was intended; I didn't write it.

[sagetherage]

closing as expected behavior since this ticket is over a year old @rossengeorgiev please let us know if you still have questions.

[rossengeorgiev]

@sagetherage I don't mean to be rude, but no one so far appears to have gone and read the code. I have not at any point mentioned requests, its all about waiting. When this state is ran with test=true, no request is ever made because of this code:

salt/salt/utils/http.py

Lines 285 to 287 in 0b46284

	if test is True:
	if test_url is None:
	return {}

That empty result {} eventually hits:

salt/salt/states/http.py

Lines 153 to 158 in 0b46284

	try:
	ret = query(name, **kwargs)
	if ret['result']:
	return ret
	except Exception as exc:
	caught_exception = exc

Since there is no result key, we get a KeyError exception, which eventually leads into:

salt/salt/states/http.py

Lines 160 to 169 in 0b46284

	if time.time() > starttime + wait_for:
	if not ret and caught_exception:
	# workaround pylint bug https://www.logilab.org/ticket/3207
	raise caught_exception # pylint: disable=E0702
	return ret
	else:
	# Space requests out by delaying for an interval
	if 'request_interval' in kwargs:
	log.debug("delaying query for {0} seconds.".format(kwargs['request_interval']))
	time.sleep(kwargs['request_interval'])

That code doesn't test for test=true, and will loop and loop until wait_for has passed

This is clearly a bug, and not expected behavior. Does this affects 2018 and 2019? Yes.

[sagetherage]

Don't worry about being rude, I certainly didn't take rudely! And thank you for the extra details. I will get this ticket into a planning session (coming up soon) and get it discussed.

[sagetherage]

The Bug label was added this week to the stalebot config and will exclude this ticket from the stalebot labeling it as stale.

[julioz]

I would like to update this thread (since the latest movement was in 2020) to mention it is still happening in Salt 3005.