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

Add option to return an exit-code when occ status signals an update is needed #35873

Merged
merged 3 commits into from
Jan 4, 2023

Conversation

leegarrett
Copy link

Signed-off-by: Lee Garrett [email protected]

Summary

This is an improved version of my earlier PR #35830. This is now a parameter to status, so it allows for adding more return codes later, which can then for example be consumed in systemd units, or monitoring checks.

It adds a parameter that can be used in scripts. ./occ status -e returns 0 when operating normally, and 1 if there is maintenance mode, 2 if ./occ upgrade is required.

TODO

  • Test coverage?

Checklist

@szaimen szaimen added this to the Nextcloud 26 milestone Dec 23, 2022
@szaimen szaimen requested review from juliusknorr, nickvergessen, a team, ArtificialOwl and come-nc and removed request for a team December 23, 2022 02:09
@szaimen szaimen added the 3. to review Waiting for reviews label Dec 23, 2022
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise you would need to add a tab on each line

core/Command/Status.php Outdated Show resolved Hide resolved
core/Command/Status.php Show resolved Hide resolved
core/Command/Status.php Outdated Show resolved Hide resolved
@come-nc
Copy link
Contributor

come-nc commented Jan 3, 2023

@leegarrett Code style checker still complains, this time about empty lines and spacing: https://github.com/nextcloud/server/actions/runs/3824719618/jobs/6507559027

Also for the comments I’d prefer if you avoid using #.

@come-nc
Copy link
Contributor

come-nc commented Jan 3, 2023

Also last commit is missing signoff and DCO complains: https://github.com/nextcloud/server/pull/35873/checks?check_run_id=10400685089

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

@leegarrett Code style checker still complains, this time about empty lines and spacing: https://github.com/nextcloud/server/actions/runs/3824719618/jobs/6507559027

Also for the comments I’d prefer if you avoid using #.

I've hopefully fixed the issue now. Since the 2nd commit wasn't signed off, I've now just squashed it with the first, removed the extra whitespace, and removed the comments. They didn't add much information, anyway.

core/Command/Status.php Outdated Show resolved Hide resolved
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my question on output, but otherwise looks good, codesniffer and DCO are both happy. Failure in drone is unrelated.

@nickvergessen nickvergessen added 4. to release Ready to be released and/or waiting for tests to finish enhancement feature: occ and removed 3. to review Waiting for reviews labels Jan 3, 2023
@leegarrett
Copy link
Author

Not sure if resolved conversations get notified, so here's a copy/paste of it:

Yes, you don't want output e.g. in ExecCondition for a system service, as this will also be logged in the unit every time and pollutes the logs unnecessarily. We can however add it back conditionally with -v if you see any use of this output.

@nickvergessen
Copy link
Member

Not sure if resolved conversations get notified, so here's a copy/paste of it:

They do,

Yes, you don't want output e.g. in ExecCondition for a system service, as this will also be logged in the unit every time and pollutes the logs unnecessarily. We can however add it back conditionally with -v if you see any use of this output.

That makes sense. Will make it so.

@nickvergessen nickvergessen merged commit c8160a6 into nextcloud:master Jan 4, 2023
@welcome
Copy link

welcome bot commented Jan 4, 2023

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@leegarrett
Copy link
Author

Hello, can we also get this backported to v25?

@nickvergessen
Copy link
Member

usually we only backport bugfixes, but it's arguable here. I will ask some people

@nickvergessen nickvergessen changed the title Implement occ status command via return codes v2 (Fixes: #35704) Add option to return an exit-code when occ status signals an update is needed Jan 18, 2023
@nickvergessen
Copy link
Member

/backport to stable25

@solracsf
Copy link
Member

No documentation on this?

@nickvergessen nickvergessen added the pending documentation This pull request needs an associated documentation update label Jan 19, 2023
@nickvergessen
Copy link
Member

@leegarrett
Copy link
Author

FYI, the doc update PR is at nextcloud/documentation#9583

@DaphneMuller
Copy link

hello @leegarrett
Thank you so much for your work on this pull request! This ticket seems to still have the tag 'pending documentation'.
Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@nickvergessen
Copy link
Member

It's linked above nextcloud/documentation#9583

@nickvergessen nickvergessen removed the pending documentation This pull request needs an associated documentation update label Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: occ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement occ status command via return codes
7 participants