redhat-performance / quads

:calendar: The infrastructure deployment time machine
https://quads.dev
GNU General Public License v3.0
88 stars 36 forks source link

[BUG] Extend is not setting 7,5,3,1 day expiration notifcations in the database to False (f) #551

Closed sadsfae closed 2 weeks ago

sadsfae commented 2 weeks ago

The way the code is written here: https://github.com/redhat-performance/quads/blob/latest/src/quads/cli/cli.py#L657 it should reset all the values to False but I don't see that happening looking at the database.

Let's look at a recently extended LTA assignment:

quads=# select * from assignments where cloud_id = 19;
 id | active | provisioned | validated |       description       |  owner  | ticket | qinq | wipe |    ccuser    | cloud_id | vlan_id |         cre
ated_at         
----+--------+-------------+-----------+-------------------------+---------+--------+------+------+--------------+----------+---------+------------
----------------
 14 | t      | t           | t         | *** Large Scale OCP LTA | dwilson | 1465   |    0 | f    | \x80055d942e |       19 |         | 2024-08-20 
16:30:54.583593
(1 row

quads=# select * from notifications where assignment_id = 14;
 id | fail | success | initial | pre_initial | pre | one_day | three_days | five_days | seven_days | assignment_id 
----+------+---------+---------+-------------+-----+---------+------------+-----------+------------+---------------
 14 | f    | f       | t       | t           | f   | f       | t          | t         | t          |            14
(1 row)

So notification_id is 14

We can see it won't expire until 2025-01-05:

Screenshot_2024-11-08_08-07-42

Now let's look at the database notifications table for assignment_id 14

quads=# select * from notifications where assignment_id = 14;
 id | fail | success | initial | pre_initial | pre | one_day | three_days | five_days | seven_days | assignment_id 
----+------+---------+---------+-------------+-----+---------+------------+-----------+------------+---------------
 14 | f    | f       | t       | t           | f   | f       | t          | t         | t          |            14
(1 row)

We should see one_day, three_days, five_days and seven_days notification flags being reset when it was extended to f.

We are effectively not resetting anything on extension, this means that tenants will only receive the notification they haven't ever received yet or we have no record of and once that happens they won't receive those either.

The code in the --extend method is supposed to do this but I don't see it happening in the database:

        if not check:
            data = {
                "one_day": False,
                "three_days": False,
                "five_days": False,
                "seven_days": False,
            }
            try:
                self.quads.update_notification(schedules[0].assignment.notification.id, data)
kambiz-aghaiepour commented 2 weeks ago

Looking at the code you posted which comes from https://github.com/redhat-performance/quads/blob/latest/src/quads/cli/cli.py#L658 the "data" object is being passed to each of the calls to self.quads.update_notification() a few lines further down on https://github.com/redhat-performance/quads/blob/latest/src/quads/cli/cli.py#L665 which is done inside of the for loop that iterates over the schedules (the for loop starts on https://github.com/redhat-performance/quads/blob/latest/src/quads/cli/cli.py#L635 )

So the way that reads, it seems to make multiple calls for each of the schedules. Shouldn't there be only a single call only if the extend operation is done with the --cloud flag rather than iterating over all the schedules?