Merge pull request #68297 from tchaikov/wip-feedback-without-tracker

mgr/feedback: fix flaky test_issue_tracker_create_with_invalid_key

Reviewed-by: Nizamudeen A <nia@redhat.com>
Reviewed-by: Afreen Misbah <afreen@ibm.com>

qa/tasks/mgr/dashboard/test_feedback.py

@@ -7,6 +7,14 @@ class FeedbackTest(MgrModuleTestCase):
     def setUpClass(cls):
         super().setUpClass()
         cls._ceph_cmd(['mgr', 'module', 'enable', 'feedback'], wait=3)
+        # Point the feedback module at an unreachable host so the test
+        # does not depend on tracker.ceph.com being available. Any
+        # create_issue call will fail fast with a ConnectionError
+        # (a RequestException subclass) which the dashboard controller
+        # is expected to surface as HTTP 400.
+        cls._ceph_cmd(['config', 'set', 'mgr',
+                       'mgr/feedback/tracker_url',
+                       'invalid.example.invalid'])
         cls._get(
             '/api/mgr/module',
             retries=5,

src/pybind/mgr/dashboard/controllers/feedback.py

@@ -38,14 +38,11 @@ class FeedbackController(RESTController):
             response = mgr.remote('feedback', 'validate_and_create_issue',
                                   project, tracker, subject, description, api_key)
         except RuntimeError as error:
-            if "Invalid issue tracker API key" in str(error):
-                raise DashboardException(msg='Error in creating tracker issue: Invalid API key',
-                                         component='feedback')
-            if "KeyError" in str(error):
-                raise DashboardException(msg=f'Error in creating tracker issue: {error}',
-                                         component='feedback')
-            raise DashboardException(msg=f'{error}',
-                                     http_status_code=500,
+            # Any failure from the tracker API (invalid key, network error,
+            # upstream 5xx, etc.) is a client/upstream error, not an internal
+            # server error. Surface it as 400 via DashboardException's default
+            # status code.
+            raise DashboardException(msg=f'Error in creating tracker issue: {error}',
                                      component='feedback')
 
         return response

src/pybind/mgr/feedback/module.py

@@ -9,7 +9,7 @@ from requests.exceptions import RequestException
 
 from .cli import FeedbackCLICommand
 
-from mgr_module import HandleCommandResult, MgrModule
+from mgr_module import HandleCommandResult, MgrModule, Option
 import errno
 
 from .service import CephTrackerClient
@@ -19,6 +19,15 @@ from .model import Feedback
 class FeedbackModule(MgrModule):
     CLICommand = FeedbackCLICommand
 
+    MODULE_OPTIONS = [
+        Option(
+            name='tracker_url',
+            type='str',
+            default='tracker.ceph.com',
+            desc='Hostname of the Ceph issue tracker (Redmine) instance',
+            runtime=True),
+    ]
+
     # there are CLI commands we implement
     @FeedbackCLICommand.Read('feedback set api-key')
     def _cmd_feedback_set_api_key(self, key: str) -> HandleCommandResult:
@@ -60,7 +69,7 @@ class FeedbackModule(MgrModule):
         """
         Fetch issue list
         """
-        tracker_client = CephTrackerClient()
+        tracker_client = CephTrackerClient(self.get_module_option('tracker_url'))
         try:
             response = tracker_client.list_issues()
         except Exception:
@@ -83,7 +92,7 @@ class FeedbackModule(MgrModule):
                 return HandleCommandResult(stderr='Issue tracker key is not set. Set key with `ceph set issue_key <your_key>`')
         except Exception as error:
             return HandleCommandResult(stderr=f'Error in retreiving issue tracker API key: {error}')
-        tracker_client = CephTrackerClient()
+        tracker_client = CephTrackerClient(self.get_module_option('tracker_url'))
         try:
             response = tracker_client.create_issue(feedback, current_api_key)
         except RequestException as error:
@@ -121,13 +130,13 @@ class FeedbackModule(MgrModule):
         return 'Successfully deleted API key'
 
     def get_issues(self):
-        tracker_client = CephTrackerClient()
+        tracker_client = CephTrackerClient(self.get_module_option('tracker_url'))
         return tracker_client.list_issues()
 
     def validate_and_create_issue(self, project: str, tracker: str, subject: str, description: str, api_key=None):
         feedback = Feedback(Feedback.Project[project].value,
                                 Feedback.TrackerType[tracker].value, subject, description)
-        tracker_client = CephTrackerClient()
+        tracker_client = CephTrackerClient(self.get_module_option('tracker_url'))
         stored_api_key = self.get_store('api_key')
         try:
             if api_key:

src/pybind/mgr/feedback/service.py

@@ -6,12 +6,14 @@ from requests.exceptions import RequestException
 
 from .model import Feedback
 
-class config:
-    url = 'tracker.ceph.com'
-    port = 443
+DEFAULT_TRACKER_URL = 'tracker.ceph.com'
+
 
 class CephTrackerClient():
 
+    def __init__(self, tracker_url: str = DEFAULT_TRACKER_URL):
+        self.tracker_url = tracker_url
+
     def list_issues(self):
         '''
         Fetch an issue from the Ceph Issue tracker
@@ -20,7 +22,7 @@ class CephTrackerClient():
             'Content-Type': 'application/json',
         }
         response = requests.get(
-            f'https://{config.url}/issues.json', headers=headers)
+            f'https://{self.tracker_url}/issues.json', headers=headers)
         if not response.ok:
             if response.status_code == 404:
                 raise FileNotFoundError
@@ -40,7 +42,7 @@ class CephTrackerClient():
             raise Exception("Ceph Tracker API Key not set")
         data = json.dumps(feedback.as_dict())
         response = requests.post(
-            f'https://{config.url}/projects/{feedback.project_id}/issues.json',
+            f'https://{self.tracker_url}/projects/{feedback.project_id}/issues.json',
             headers=headers, data=data)
         if not response.ok:
             if response.status_code == 401: