-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
A complete maintenance planning system #1213
Conversation
server/model/maintenance.js
Outdated
title: this.title, | ||
description: this.description, | ||
start_date: this.start_date, | ||
end_date: this.end_date |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better git diff in next updates to this object
end_date: this.end_date | |
end_date: this.end_date, |
server/model/maintenance.js
Outdated
title: this.title, | ||
description: this.description, | ||
start_date: this.start_date, | ||
end_date: this.end_date |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end_date: this.end_date | |
end_date: this.end_date, |
server/model/monitor.js
Outdated
@@ -6,7 +6,7 @@ dayjs.extend(utc); | |||
dayjs.extend(timezone); | |||
const axios = require("axios"); | |||
const { Prometheus } = require("../prometheus"); | |||
const { debug, UP, DOWN, PENDING, flipStatus, TimeLogger } = require("../../src/util"); | |||
const { debug, UP, DOWN, PENDING, MAINTENANCE, flipStatus, TimeLogger} = require("../../src/util"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const { debug, UP, DOWN, PENDING, MAINTENANCE, flipStatus, TimeLogger} = require("../../src/util"); | |
const { debug, UP, DOWN, PENDING, MAINTENANCE, flipStatus, TimeLogger } = require("../../src/util"); |
server/routers/api-router.js
Outdated
@@ -5,7 +5,7 @@ const server = require("../server"); | |||
const apicache = require("../modules/apicache"); | |||
const Monitor = require("../model/monitor"); | |||
const dayjs = require("dayjs"); | |||
const { UP, flipStatus, debug } = require("../../src/util"); | |||
const { UP, MAINTENANCE, flipStatus, debug} = require("../../src/util"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const { UP, MAINTENANCE, flipStatus, debug} = require("../../src/util"); | |
const { UP, MAINTENANCE, flipStatus, debug } = require("../../src/util"); |
src/components/MonitorList.vue
Outdated
@@ -47,7 +66,7 @@ | |||
import HeartbeatBar from "../components/HeartbeatBar.vue"; | |||
import Uptime from "../components/Uptime.vue"; | |||
import Tag from "../components/Tag.vue"; | |||
import { getMonitorRelativeURL } from "../util.ts"; | |||
import {getMaintenanceRelativeURL, getMonitorRelativeURL} from "../util.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {getMaintenanceRelativeURL, getMonitorRelativeURL} from "../util.ts"; | |
import { getMaintenanceRelativeURL, getMonitorRelativeURL } from "../util.ts"; |
src/pages/EditMaintenance.vue
Outdated
Object.values(this.$root.monitorList).map(monitor => { | ||
this.affectedMonitorsOptions.push({ | ||
id: monitor.id, | ||
name: monitor.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: monitor.name | |
name: monitor.name, |
src/pages/EditMaintenance.vue
Outdated
|
||
"$route.fullPath"() { | ||
this.init(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"$route.fullPath"() { | |
this.init(); | |
} | |
"$route.fullPath"() { | |
this.init(); | |
} |
src/pages/EditMaintenance.vue
Outdated
} | ||
|
||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
}, | |
} | |
}, |
src/pages/EditMaintenance.vue
Outdated
this.processing = true; | ||
|
||
if (this.affectedMonitors.length === 0) { | ||
toast.error(this.$t("Select at least one affected monitor")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shorter i18n key
src/pages/EditMaintenance.vue
Outdated
|
||
<!-- Start Date Time --> | ||
<div class="my-3"> | ||
<label for="start_date" class="form-label">{{ $t("Start of maintenance") }}</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be good to add current timezone in label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in label, not in input...
For example:
Start of Maintenance (GMT+1)
<input ...>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. I will now commit with minor fixes (missing commas, spaces, translations) and try to make some adjustments to storing maintenance dates in the database.
Currently, the date is stored in the database in the time zone in which the date was entered by the user, not in UTC (+0) format, so it cannot be converted to the current time zone, eg when changing the time zone.
Ie. if the user enters 1:35 while creating maintenance and changes the time zone, it will still show 1:35.
server/model/monitor.js
Outdated
@@ -28,9 +29,12 @@ class Monitor extends BeanModel { | |||
* Only show necessary data to public | |||
*/ | |||
async toPublicJSON() { | |||
const maintenance = await R.getAll("SELECT mm.*, maintenance.start_date, maintenance.end_date FROM monitor_maintenance mm JOIN maintenance ON mm.maintenance_id = maintenance.id WHERE mm.monitor_id = ? AND datetime(maintenance.start_date) <= datetime('now', 'localtime') AND datetime(maintenance.end_date) >= datetime('now', 'localtime')", [this.id]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 3 this same queries in this file, 4th this same query is in api-router.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, can I ask your opinion?
The states of individual monitors are currently evaluated according to the state of the last heartbeat.
If I used this system in case of maintenance, any change in the maintenance settings (adding / removing the monitor from maintenance, changing the maintenance time) would not affect the monitor and the status page until the next heartbeat of the monitor.
By directly checking whether or not the monitor is in maintenance, it is possible to mark the monitor as in maintenance before a heartbeat occurs.
For this reason, this SQL query is also called when requesting monitor information instead of being called only twice (on the status page and at heartbeat).
In my opinion, this is more user-friendly, because the fact that the monitor is in maintenance or not can be seen immediately without waiting, even on the status page.
Do you think it will be better to leave the status display according to the last heartbeat, or do you find the my current solution better?
Thanks for the feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO your (current) solution is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback.
In this case, 2 out of 4 calls are absolutely necessary (calls from the status page - display of maintenance information on the status page and at each beat - finding out if the monitor is under maintenance) and if I am not mistaken, in the methods "toJSON()" and "toPublicJSON()" this calls are also required to let the current solution described above work.
If I'm wrong, feel free to tell me your opinion.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know . I just wanted to have this long query-code line in one place (helper function etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, understand. It shouldn't be problem I think, I think I'll be able to move it into one helper function and just call it.
Few minutes later I commited some changes, so I'll check / do this tomorrow I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. It's now in its own method.
src/components/MonitorList.vue
Outdated
@@ -47,7 +66,7 @@ | |||
import HeartbeatBar from "../components/HeartbeatBar.vue"; | |||
import Uptime from "../components/Uptime.vue"; | |||
import Tag from "../components/Tag.vue"; | |||
import { getMonitorRelativeURL } from "../util.ts"; | |||
import {getMaintenanceRelativeURL, getMonitorRelativeURL } from "../util.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {getMaintenanceRelativeURL, getMonitorRelativeURL } from "../util.ts"; | |
import { getMaintenanceRelativeURL, getMonitorRelativeURL } from "../util.ts"; |
…ch allows it to be converted between time zones
… in its own method.
@Saibamen, Everything should be OK 🙂 |
Seems like this isn't working. |
@mathiskir make sure you removed folder with previous clone. Remove whole |
It works! |
We need more people with access to the repo so we can merge this and the PR for incident timelines! We really need this |
Its such a nice update, hope he will finally merge this |
Is it possible to publish an endpoint to pause a specific monitor e.g. via REST. example: I think this would really be helpful to solve the issue regarding automated deployments with varying times. |
That sounds like a good idea. Personally, I would suggest merging this pull request and then making a pull request on it containing API endpoints for maintenance system and incident system. My point is that now this pull request is in a state where it is ready for deployment. I do not want to unnecessarily prolong the time that will be needed to implement this pull request in the main branch. |
@louislam Please? 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add JSDoc here as well. @karelkryda I know your busy currently with exams so don't worry if you can't do it right now (good luck BTW). If you like, I could lend a hand here, just let me know. I would probably merge from master first though to get all of the comments added from #1499 so we don't end up duplicating.
server/model/maintenance.js
Outdated
} | ||
|
||
/** | ||
* Return a object that ready to parse to JSON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Return a object that ready to parse to JSON | |
* Return a object that ready to parse to JSON | |
* @returns {Object} |
Yes, it's not a problem. I'm currently wondering if there will also be a need to redo the whole PR because of conflicts. If it wasn't needed, adding comments is a relatively short-term job and maybe I could do it soon. Btw. thank you for wishing me luck 😊 |
@karelkryda If I get sometime today or tomorrow, I will submit a PR to your Fork with all the merge conflicts solved. I've done this already for my own copy of uptime-kuma. |
That sounds great. If I had time on the weekend, I would try to look at it and check the code. Do you think I should implement the option to choose which page to display maintenance information on? |
@karelkryda I just submitted a PR in your fork with all the conflicts solved. Once that PR is merge it should make it easier for @louislam to review this PR. 😄 PR is here: karelkryda#3 |
Thanks, it's a lot of code 😅. I'll go through it, test it and prepare for merge. |
I'm not 100% sure if this would help but:
That way the PR will only have the changes related to this PR |
# Conflicts: # package-lock.json # server/database.js # server/model/monitor.js # server/routers/api-router.js # server/server.js # src/components/MonitorList.vue # src/components/PingChart.vue # src/icon.js # src/pages/DashboardHome.vue # src/pages/StatusPage.vue # src/router.js # src/util.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix EFLinx errors and warnings
server/model/monitor.js
Outdated
} | ||
else if (this.type === "http" || this.type === "keyword") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
else if (this.type === "http" || this.type === "keyword") { | |
} else if (this.type === "http" || this.type === "keyword") { |
server/model/monitor.js
Outdated
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
else { | |
} else { |
server/model/monitor.js
Outdated
@@ -903,6 +948,11 @@ class Monitor extends BeanModel { | |||
monitorID | |||
]); | |||
} | |||
|
|||
static async isUnderMaintenance(monitorID) { | |||
const maintenance = await R.getRow("SELECT COUNT(*) AS count FROM monitor_maintenance mm JOIN maintenance ON mm.maintenance_id = maintenance.id WHERE mm.monitor_id = ? AND datetime(maintenance.start_date) <= datetime('now') AND datetime(maintenance.end_date) >= datetime('now') LIMIT 1", [monitorID]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const maintenance = await R.getRow("SELECT COUNT(*) AS count FROM monitor_maintenance mm JOIN maintenance ON mm.maintenance_id = maintenance.id WHERE mm.monitor_id = ? AND datetime(maintenance.start_date) <= datetime('now') AND datetime(maintenance.end_date) >= datetime('now') LIMIT 1", [monitorID]); | |
const maintenance = await R.getRow("SELECT COUNT(*) AS count FROM monitor_maintenance mm JOIN maintenance ON mm.maintenance_id = maintenance.id WHERE mm.monitor_id = ? AND datetime(maintenance.start_date) <= datetime('now') AND datetime(maintenance.end_date) >= datetime('now') LIMIT 1", [ monitorID ]); |
server/routers/api-router.js
Outdated
@@ -148,6 +156,30 @@ router.get("/api/status-page/:slug", cache("5 minutes"), async (request, respons | |||
} | |||
|
|||
}); | |||
// TODO: make slug aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix TODO
src/components/MonitorList.vue
Outdated
<option value="monitor" selected>{{$t('Monitor List')}}</option> | ||
<option value="maintenance">{{$t('Maintenance List')}}</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<option value="monitor" selected>{{$t('Monitor List')}}</option> | |
<option value="maintenance">{{$t('Maintenance List')}}</option> | |
<option value="monitor" selected>{{ $t('Monitor List') }}</option> | |
<option value="maintenance">{{ $t('Maintenance List') }}</option> |
src/components/Uptime.vue
Outdated
@@ -15,6 +15,10 @@ export default { | |||
|
|||
computed: { | |||
uptime() { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/components/Uptime.vue
Outdated
@@ -26,6 +30,10 @@ export default { | |||
}, | |||
|
|||
color() { | |||
if (this.type === "maintenance" || this.monitor.maintenance) { | |||
return "maintenance" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return "maintenance" | |
return "maintenance"; |
src/components/Uptime.vue
Outdated
if (this.type === "maintenance" || this.monitor.maintenance) { | ||
return "maintenance" | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/languages/en.js
Outdated
"Pick Affected Monitors...": "Pick Affected Monitors...", | ||
"Start of maintenance": "Start of maintenance", | ||
"Expected end of maintenance": "Expected end of maintenance", | ||
Start: "Start", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated key
Start: "Start", |
src/mixins/datetime.js
Outdated
if (inputDate.getFullYear() === now.getUTCFullYear() && inputDate.getMonth() === now.getUTCMonth() && inputDate.getDay() === now.getUTCDay()) | ||
return this.datetimeFormat(value, "HH:mm"); | ||
else | ||
return this.datetimeFormat(value, "YYYY-MM-DD HH:mm"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (inputDate.getFullYear() === now.getUTCFullYear() && inputDate.getMonth() === now.getUTCMonth() && inputDate.getDay() === now.getUTCDay()) | |
return this.datetimeFormat(value, "HH:mm"); | |
else | |
return this.datetimeFormat(value, "YYYY-MM-DD HH:mm"); | |
if (inputDate.getFullYear() === now.getUTCFullYear() && inputDate.getMonth() === now.getUTCMonth() && inputDate.getDay() === now.getUTCDay()) { | |
return this.datetimeFormat(value, "HH:mm"); | |
} else { | |
return this.datetimeFormat(value, "YYYY-MM-DD HH:mm"); | |
} |
@@ -64,7 +64,7 @@ class Database { | |||
"patch-add-other-auth.sql": { parents: [ "patch-monitor-basic-auth.sql" ] }, | |||
"patch-add-radius-monitor.sql": true, | |||
"patch-monitor-add-resend-interval.sql": true, | |||
"patch-maintenance-table.sql": true, | |||
"patch-maintenance-table2.sql": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change name (delete 2
) before merging into master
# Conflicts: # server/server.js # src/components/settings/General.vue
I think it is almost done. I will have a final file changes review on GitHub and quickly release a beta for this. So everyone can test it. Based on @karelkryda's pull request, I also update followings:
|
Thank you @louislam for your cooperation, it looks very good, well done. |
Not yet using kuma, but I was waiting this feature to test. I see all the amazing work you did, and just wanted to say thanks :) Thanks a lot for this hard work! Have a wonderful day everybody :) |
It is available in 1.19.0-beta.0 now, feel free to try. I would expect it is not fully stable yet, also feel free to submit bug reports. |
I am glad that this functionality has finally been implemented and I was happy to help with it. I hope that my next PR related to the incident system (and other PRs) will soon be included in the main branch and I look forward to further excellent cooperation with @louislam. Have a nice rest of the day |
This is awesome news, a big thanks to all who made this possible! Can't wait to test this! |
This is the one I have been waiting for! I have weekly reboots on a lot of systems and until now, I'm getting flooded by alert mails. Thanks a lot to all who contributed!! |
Awesome feature, thanks a lot. |
Amazing feature! I have waited for this one sooo long :) Here a quick example:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished
Description
This pull request includes the addition of maintenance planning functionality.
Affected monitors are not tested during maintenance, but status "3" (MAINTENANCE) is returned.
No notifications are also sent during maintenance. Notifications will only be sent if the monitor status after maintenance is "DOWN" (MAINTENANCE -> DOWN).
Due to the addition of this function, the UI has also been changed (adding buttons, texts, ...).
This pull request also requires the addition of some new translations.
Related issue: #191
Type of change
Please delete any options that are not relevant.
Checklist
Screenshots (if any)