Skip to content
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

Implement occ status command via return codes #35704

Closed
leegarrett opened this issue Dec 9, 2022 · 8 comments · Fixed by #35873
Closed

Implement occ status command via return codes #35704

leegarrett opened this issue Dec 9, 2022 · 8 comments · Fixed by #35873
Labels

Comments

@leegarrett
Copy link

leegarrett commented Dec 9, 2022

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

I propose the --check-if-disabled flag to occ maintenance:mode. It doesn't write any output, it just returns 0 if maintenance mode is off, and 1 if it's on.

This allows to programmatically check for maintenance mode, which makes automation easier. It allows us to use it as an conditional for the cron job (e.g. ExecCondition in the systemd unit), so the cron job will be skipped when maintenance mode is running. Currently it generates an admin email every 5 minutes during maintenance mode.

@leegarrett leegarrett added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Dec 9, 2022
@kesselb
Copy link
Contributor

kesselb commented Dec 9, 2022

Hey, sounds good to me.

https://github.com/nextcloud/server/blob/master/core/Command/Maintenance/Mode.php is the file to modify.

Would you mind sending a pull request? I guess everything you need is already in the file ;)

@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Dec 9, 2022
@solracsf
Copy link
Member

if [ "$(occ config:system:get maintenance)" == 'false' ]; then
    //fire
fi

@luddinho
Copy link

Or the same with this occ command combined with grep:

if [ "$(occ maintenance:mode | grep -o 'enabled\|disabled')" == "disabled" ]; then
    //fire
fi

@szaimen szaimen added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Dec 19, 2022
@leegarrett
Copy link
Author

Hey, sounds good to me.

https://github.com/nextcloud/server/blob/master/core/Command/Maintenance/Mode.php is the file to modify.

Would you mind sending a pull request? I guess everything you need is already in the file ;)

Thanks, and done in PR #35830. I took a look at Core/Command/Maintenance/ModeTest.php but didn't understand the code well enough to add test coverage, too.

@MichaIng
Copy link
Member

The two above mentioned minimally more complex methods to programmatically check for maintenance mode already now do not make this redundant?

@leegarrett
Copy link
Author

The two above mentioned minimally more complex methods to programmatically check for maintenance mode already now do not make this redundant?

I'm aware that you can do this over a shell call. However:

  • It's not always possible nor desirable to spawn a shell
  • Having to spawn a shell with e.g. sh -c makes the test hard to read with double enclosure quoting, due to the need to escape inside quotes and newlines, and easy for the test to silently fail
  • Depending on textual output is flawed as it's not guaranteed to be stable
  • Those examples are not covered by tests, which this ideally should have.

@leegarrett leegarrett changed the title Implement occ maintenance:mode --check-if-disabled Implement occ status command via return codes Dec 22, 2022
leegarrett pushed a commit to leegarrett/server that referenced this issue Dec 22, 2022
Running `./occ status -e` will produce any output. However, it will:

exit 0 during normal operation,
exit 1 when in maintenance mode,
exit 2 when `./occ upgrade` is needed.

Signed-off-by: Lee Garrett <[email protected]>
@leegarrett
Copy link
Author

I've done a second PR which I think is overall cleaner and better thought out. It's a parameter to ./occ status instead of maintenance:mode, and doesn't solely decide on maintenance mode. It's also possible to expand it in the future to return other (non-zero) exit codes for other critical issues that prevent normal operation.

@leegarrett
Copy link
Author

The new PR also avoids the negation that the old one was plagued with.

leegarrett pushed a commit to leegarrett/server that referenced this issue Dec 25, 2022
Running `./occ status -e` will produce any output. However, it will:

exit 0 during normal operation,
exit 1 when in maintenance mode,
exit 2 when `./occ upgrade` is needed.

Signed-off-by: Lee Garrett <[email protected]>
leegarrett pushed a commit to leegarrett/server that referenced this issue Jan 3, 2023
Running `./occ status -e` will produce any output. However, it will:

exit 0 during normal operation,
exit 1 when in maintenance mode,
exit 2 when `./occ upgrade` is needed.

Signed-off-by: Lee Garrett <[email protected]>
nickvergessen added a commit that referenced this issue Jan 4, 2023
Implement occ status command via return codes v2 (Fixes: #35704)
backportbot-nextcloud bot pushed a commit that referenced this issue Jan 18, 2023
Running `./occ status -e` will produce any output. However, it will:

exit 0 during normal operation,
exit 1 when in maintenance mode,
exit 2 when `./occ upgrade` is needed.

Signed-off-by: Lee Garrett <[email protected]>
akhil1508 pushed a commit to e-foundation/server that referenced this issue Sep 28, 2023
Running `./occ status -e` will produce any output. However, it will:

exit 0 during normal operation,
exit 1 when in maintenance mode,
exit 2 when `./occ upgrade` is needed.

Signed-off-by: Lee Garrett <[email protected]>
Signed-off-by: Akhil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants