Add `healthcheck` subcommand.

reitermarkus commented 4 years ago

Description

Implements and closes https://github.com/openfaas-incubator/of-watchdog/issues/86.

Motivation and Context

Explained in https://github.com/openfaas-incubator/of-watchdog/issues/86.

[x] I have raised an issue to propose this change (required)

How Has This Been Tested?

This was tested using a static website template:

FROM alpine as tempdir

RUN mkdir -p /tempdir

FROM scratch

COPY --from=openfaas/of-watchdog:latest-dev-x86_64 /fwatchdog /fwatchdog
COPY --from=tempdir /tempdir /tmp

WORKDIR /home/app

COPY public public

HEALTHCHECK --interval=2s CMD ["/fwatchdog", "healthcheck"]

ENV mode="static"

CMD ["/fwatchdog"]

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I've read the CONTRIBUTION guide
[x] I have signed-off my commits with git commit -s
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

derek[bot] commented 4 years ago

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

alexellis commented 4 years ago

Please fill out the whole issue template, it's required for all contributors to follow the process outlined by the community. Thank you for your interest.

derek[bot] commented 4 years ago

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

alexellis commented 4 years ago

The commit message subject looks good 👍 and thanks for signing off.

Could you fill out the body of the commit with some notes for the commit log? A year or so ago I found this blog post useful for learning how to create useful messages: https://chris.beams.io/posts/git-commit/

reitermarkus commented 4 years ago

Added some more detail to the commit messages.

alexellis commented 4 years ago

https://github.com/openfaas-incubator/of-watchdog/pull/87/files#discussion_r347115625

Thank you for your opinion. Please could you change this as per the request? I believe that it's the only remaining item :+1:

reitermarkus commented 4 years ago

Thank you for your opinion. Please could you change this as per the request?

I'm not sure exactly what happened here. I argued that a subcommand is more appropriate than a flag in this case and you dismiss my opinion while thanking me and telling me how to idiomatically parse a command line flag.

can you raise a similar PR for the classic watchdog?

If someone wants new features shouldn't they already be using of-watchdog anyways?

alexellis commented 4 years ago

I realise I didn't give you a very detailed explanation of why this is needed for the classic watchdog. I hope you can trust my opinion as the maintainer and lead of the project? This will need to be applied to the classic watchdog for backwards compatibility with the classic templates and for existing users.

alexellis commented 4 years ago

As for switch flag.Arg(0), I asked you to add a unit test and to change to what we use everywhere else in the project for consistency.

One of the things you declared in the PR template was:

[x] My code follows the code style of this project.

This change does not follow the style of the project, so I'm trying to help you (I didn't expect an argument)

alexellis commented 4 years ago

Here's what I was looking for, which also documents the behaviour of the flag and shows up on of-watchdog --help (your example does not)

    var runHealthcheck bool
    flag.BoolVar(&runHealthcheck, "run-healthcheck", false, "Check for the a lock-file, when using an exec healthcheck. Exit 0 for present, non-zero when not found.")
    flag.Parse()

    if runHealthcheck {
        if lockFilePresent() {
            os.Exit(0)
        }

        fmt.Fprintf(os.Stderr, "unable to find lock file\n")
        os.Exit(1)
    }

Example:

 fprocess=date ./of-watchdog -help
Usage of ./of-watchdog:
  -run-healthcheck
        Check for the a lock-file, when using an exec healthcheck. Exit 0 for present, non-zero when not found.

openfaas / of-watchdog