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

OpsGenie state returns with incorrect type for changes #55562

Open
colin-stubbs opened this issue Dec 9, 2019 · 2 comments
Open

OpsGenie state returns with incorrect type for changes #55562

colin-stubbs opened this issue Dec 9, 2019 · 2 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Regression The issue is a bug that breaks functionality known to work in previous releases. severity-high 2nd top severity, seen by most users, causes major problems
Milestone

Comments

@colin-stubbs
Copy link

Description of Issue

'changes' should be a dictionary, e.g.


      ID: monitoring-http-IPTPRDORC101-81-alert-opsgenie-close
Function: opsgenie.close_alert
    Name: aurizon-ndc-b1-monitor-http-IPTPRDORC101-81-alert-opsgenie
  Result: False
 Comment: An exception occurred in this state: 'Changes' should be a dictionary.
 Changes:   

The state returns False even though it has succeeded in talking to the OpsGenie API. Hence dependent states can't reliably use True/False return from the opsgenie states.

Simple diff that fixes this is,

--- opsgenie.py.orig 2019-12-09 15:50:35.283153678 +1000
+++ opsgenie.py 2019-12-09 15:47:55.242250951 +1000
@@ -75,7 +75,7 @@
ret = {
'result': '',
'name': '',

  •    'changes': '',
    
  •    'changes': {},
       'comment': ''
    
    }

Setup

2019.2.2

Steps to Reproduce Issue

Try to use any OpsGenie state, e.g. create_alert or close_alert

Versions Report

$ salt --versions-report
Salt Version:
Salt: 2019.2.2

Dependency Versions:
cffi: 1.6.0
cherrypy: Not Installed
dateutil: 1.5
docker-py: 1.10.6
gitdb: 0.6.4
gitpython: 1.0.1
ioflo: 1.3.8
Jinja2: 2.7.2
libgit2: 0.26.3
libnacl: 1.6.1
M2Crypto: 0.31.0
Mako: 0.8.1
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: 1.2.5
pycparser: 2.14
pycrypto: 2.6.1
pycryptodome: 3.7.3
pygit2: 0.26.4
Python: 2.7.5 (default, Aug 7 2019, 00:51:29)
python-gnupg: 0.4.3
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: 0.9.0
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4

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

@waynew waynew added this to Needs triage in [Test] Triage Dec 9, 2019
@waynew waynew added Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE P4 Priority 4 labels Dec 9, 2019
@waynew waynew moved this from Needs triage to Low priority in [Test] Triage Dec 9, 2019
@waynew waynew added the fixed-pls-verify fix is linked, bug author to confirm fix label Dec 9, 2019
@sagetherage sagetherage added this to the Approved milestone Dec 17, 2019
@sagetherage sagetherage added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 26, 2020
@sagetherage
Copy link
Contributor

@colin-stubbs we will hold a test clinic Apr 2nd at the Open Hour or I can get you with an engineer from the Open Core team to discuss writing tests for the PR, LMK.

@sagetherage sagetherage added this to To do in [TEST] Wayne's Work via automation May 29, 2020
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label May 29, 2020
@sagetherage sagetherage added this to Considering in Magnesium May 29, 2020
@sagetherage sagetherage removed the P4 Priority 4 label Jun 3, 2020
@sagetherage sagetherage moved this from Considering to Commit in Magnesium Jun 23, 2020
@sagetherage sagetherage moved this from Commit to In progress in Magnesium Jul 7, 2020
@sagetherage sagetherage modified the milestones: Approved, Magnesium Jul 14, 2020
waynew pushed a commit to waynew/salt that referenced this issue Sep 11, 2020
Ensures changes is returned as a dictionary rather than an empty string.
@sagetherage sagetherage removed the Magnesium Mg release after Na prior to Al label Sep 18, 2020
@sagetherage sagetherage removed this from In progress in Magnesium Sep 18, 2020
@sagetherage sagetherage modified the milestones: Magnesium, Approved Sep 18, 2020
@sagetherage
Copy link
Contributor

de-scoping for the Magnesium release since there is still no test case written in the linked PR

@sagetherage sagetherage added severity-high 2nd top severity, seen by most users, causes major problems Regression The issue is a bug that breaks functionality known to work in previous releases. and removed fixed-pls-verify fix is linked, bug author to confirm fix labels Jun 16, 2021
@sagetherage sagetherage removed this from the Approved milestone Jun 16, 2021
@sagetherage sagetherage added this to the Silicon milestone Jun 16, 2021
@sagetherage sagetherage added the Silicon v3004.0 Release code name label Jun 16, 2021
@sagetherage sagetherage removed the Silicon v3004.0 Release code name label Aug 12, 2021
@sagetherage sagetherage modified the milestones: Silicon, Approved Aug 12, 2021
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 Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Regression The issue is a bug that breaks functionality known to work in previous releases. severity-high 2nd top severity, seen by most users, causes major problems
Projects
No open projects
[Test] Triage
  
Low priority
Development

Successfully merging a pull request may close this issue.

3 participants