hook fails on all Jenkins commands except quietDown and cancelQuietDown

[axinojolais]

Hello !

Perhaps I'm missing something obvious, because it's strange that no one saw that before ?

In the following block, the statement in the if cannot be true unless action is either quietDown or cancelQuietDown. So return will not be called, and the else statement from the try block will be executed, leading to a hook error.

jenkins-charm/lib/charms/layer/jenkins/api.py

Lines 214 to 222 in 734965f

	try:
	# Jenkins doesn't return an error for some actions
	if (client.jenkins_open(request) and
	(action == "quietDown" or action == "cancelQuietDown")):
	return
	except requests.exceptions.HTTPError as error:
	self._check_response(error)
	else:
	raise RuntimeError(fail_message)

I'm not sure what the intent is here, will investigate and make a PR

[axinojolais]

According to https://python-jenkins.readthedocs.io/en/latest/api.html, jenkins_open returns a str so I have no idea how the original version of _execute_action ever worked :

jenkins-charm/lib/charms/layer/jenkins/api.py

Lines 159 to 167 in 155d241

	def _execute_action(self, action, fail_message):
	client = self._make_client()
	request = requests.Request("POST", urljoin(self.url, action))
	try:
	client.jenkins_open(request)
	except requests.exceptions.HTTPError as error:
	self._check_response(error)
	else:
	raise RuntimeError(fail_message)

[alejdg]

All actions, except for those two, when executed return HTTPError. This is a "normal" behavior in Jenkins.

So when things happen as expected all actions will fall into the except part. Only when something different occurs the else will part will be executed.

[axinojolais]

Well the action safeRestart on the Jenkins I'm looking at (version 2.235.1) doesn't return HTTPError so the raise RuntimeError is reached. Every single time.

[alejdg]

@axinojolais maybe jenkins has changed the return for that action, possibly for all of them.

So this issue is definitely valid and we should check what implemented actions are failing due to that and add them to the same place as quietDown and cancelQuietDown:

jenkins-charm/lib/charms/layer/jenkins/api.py

Line 217 in 734965f

(action == "quietDown" or action == "cancelQuietDown")):

[axinojolais]

@mthaddon @darkalia I don't agree this is fixed. #93 only fixes safeRestart. What about the other commands ? Have we tested them ? Do they work ?

[ben-ballot]

@mthaddon @darkalia I don't agree this is fixed. #93 only fixes safeRestart. What about the other commands ? Have we tested them ? Do they work ?

I agree it's not fixed. It's only part of the fix to unstuck a situation.
My bad for using "Fixes: #91" in the commit message.
We should check for those commands as well:

• restart
• exit
• safeExit