Closed Ktmi closed 1 year ago
David, when you need a final review let me know, I closed some of the threads that you addressed, I can do another pass later, but since you're pushing code and still have discussion threads open here, I'll wait for you confirmation first.
Good job. It's great how it's shaping up.
David, when you need a final review let me know, I closed some of the threads that you addressed, I can do another pass later, but since you're pushing code and still have discussion threads open here, I'll wait for you confirmation first.
Good job. It's great how it's shaping up.
Also, if you could follow our Kytos-ng template PR that'd be great, facilitating for everyone. Your PRs descriptions are great, but in the template we also have information about local tests and e2e, which are very helpful for reviewers.
David, when also exploring locally here, I noticed that finished
MWs are also loaded again in the scheduler, probably won't impact at all, but do we need to load them again? From what I can tell, the only utility for a finished mw is to be listed again, which will be directly from the DB. Let me know if you had any other reason
2023-06-23 15:16:28,244 - INFO [kytos.napps.kytos/maintenance] [scheduler.py:157:_schedule] (MainThread) Scheduling "some_mw1"
2023-06-23 15:16:28,244 - INFO [kytos.napps.kytos/maintenance] [scheduler.py:157:_schedule] (MainThread) Scheduling "some_mw2"
2023-06-23 15:16:28,257 - INFO [kytos.napps.kytos/maintenance] [scheduler.py:173:_schedule] (MainThread) Scheduled "some_mw2" end at 2023-06-23 18:20:00+00:00
2023-06-23 15:16:28,257 - INFO [apscheduler.scheduler] [base.py:445:add_job] (MainThread) Adding job tentatively -- it will be properly scheduled when the scheduler starts
2023-06-23 15:16:28,257 - INFO [kytos.napps.kytos/maintenance] [scheduler.py:157:_schedule] (MainThread) Scheduling "some_mw3"
2023-06-23 15:16:28,257 - INFO [apscheduler.scheduler] [base.py:445:add_job] (MainThread) Adding job tentatively -- it will be properly scheduled when the scheduler starts
Other than that, I explored a link and and a sw under maintenance, they behaved as expected, I also simulated restarting controller when a MW hasn't been started yet, and it picked up and loaded correctly too. Once your local tests are also positive and e2e tests are passing this one can land too, also if Italo hasn't started exploratory tests yet, we can potentially merge this if you wish to take the opportunity, otherwise it'll be on 2023.1.1
, let me know what which way you prefer.
I was able to do end to end tests with both this and kytos-ng/topology#147 together, and most of the tests passed, except 3 of them which used a non-existant switch id. The switch ID issue will be fixed with kytos-ng/kytos-end-to-end-tests#245, which replaces those switch ids with valid existing ones. Doing the end to end tests with the fixed switch ids results in all maintenance tests passing.
David, when also exploring locally here, I noticed that
finished
MWs are also loaded again in the scheduler, probably won't impact at all, but do we need to load them again? From what I can tell, the only utility for a finished mw is to be listed again, which will be directly from the DB. Let me know if you had any other reason
Loading them again is completely unnecessary. I'll write a method on the DB controller just for loading the unfinished windows only.
Let's ship it @Ktmi, let me know if you'll merge it or if I should do it.
This PR makes it so that maintenance will now use service interruptions to inform other NApps of which devices are being put into maintenance. Additionally this PR marks all devices in maintenance as being in maintenance through status_reason. This PR resolves the following issues:
76
58
71
44
25
65
7
Local Test Results
Local tests were performed with kytos-ng/topology#147. With this patch, maintenance windows continued to properly dispatch link up and link down events when maintenance windows began and ended.
End-To-End Tests
Initially end to end tests had an error involving attempting to create a maintenance window with a non-existent switch. This issue is fixed by kytos-ng/kytos-end-to-end-tests#245. When running with the patched end to end tests, the following results were produced: