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

detect Varnish instances and try to be helpful #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gquintard
Copy link
Contributor

One Karnaugh map later and I realized that #116 (comment)
could have been expressed more simply:

  • as a rule, when we issue at least one warning, we pause for 10s to give time to the user to read said warning
  • if we can ping varnish, proceed
  • if we can't ping:
    • if -n was given: warn then proceed
    • if there's no -n:
      • without any candidate, warn then proceed
      • there's only one candidate, and we can ping it: override, warn then proceed
      • there's only one candidate but we can't ping: error out
      • more than one candidate: list and error our

Read on for example outputs.

If varnishadm ping is successful with the current NAME argument, no
question asked, go for it:

Info: Working directory: /tmp/varnishgather.Z1LTcYva/varnishgather-flamp-20241106-214646
Info: Complete varnishadm command line deduced to:
Task: 1: varnishd -V
...

If pinging failed, there are a few cases:

If NAME was given, or if NAME wasn't given but there's no varnishd
detected, issue a warning, pause a bit, then proceed:

Info: Working directory: /tmp/varnishgather.BBE414gf/varnishgather-flamp-20241106-221201
#######################################################
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?
#######################################################
Info: Complete varnishadm command line deduced to:
<PAUSE 10s>
Task: 1: varnishd -V
...

if no NAME was given but there's exactly one candidate, and it's
pingable, override NAME, warn about it, pause then proceed:

Info: Working directory: /tmp/varnishgather.ZeUDcfg7/varnishgather-flamp-20241106-222151
#######################################################
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?

There's only one varnishd running and it's pingable.

Overriding -n argument to /tmp/beepboop
#######################################################
<PAUSE 10s>
Info: Complete varnishadm command line deduced to: -n /tmp/beepboop
Task: 1: varnishd -V
...

If no NAME is given, and there's only one candidate but it's not
pingable, just error out:

Info: Working directory: /tmp/varnishgather.eZSa78VU/varnishgather-flamp-20241106-224856
#######################################################
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?
#######################################################
<EXIT>

Finally, if there are no given NAME, and we have more than one
candidate, we just propose a list of candidates and error out:

Info: Working directory: /tmp/varnishgather.dkGgfqHe/varnishgather-flamp-20241106-223210
#######################################################
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?

We've also detected one or more varnishd that
could possibly be pinged with these -n arguments:
				-n /tmp/beepboop
				-n /tmp/beepboop2
#######################################################
<EXIT>

@hermunn and @zackmay, tagging you in as you were part of the original discussion.

One Karnaugh map later and I realized that #116 (comment)
could have been expressed more simply.

If `varnishadm ping` is successful with the current NAME argument, no
question asked, go for it:

```
Info: Working directory: /tmp/varnishgather.Z1LTcYva/varnishgather-flamp-20241106-214646
Info: Complete varnishadm command line deduced to:
Task: 1: varnishd -V
...
```

If pinging failed, there are a few cases:

If NAME was given, or if NAME wasn't given but there's no varnishd
detected, issue a warning, pause a bit, then proceed:

```
Info: Working directory: /tmp/varnishgather.BBE414gf/varnishgather-flamp-20241106-221201
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?
Info: Complete varnishadm command line deduced to:
<PAUSE 10s>
Task: 1: varnishd -V
...
```

if no NAME was given but there's exactly one candidate, and it's
pingable, override NAME, warn about it, pause then proceed:

```
Info: Working directory: /tmp/varnishgather.ZeUDcfg7/varnishgather-flamp-20241106-222151
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?

There's only one varnishd running and it's pingable.

Overriding -n argument to /tmp/beepboop
<PAUSE 10s>
Info: Complete varnishadm command line deduced to: -n /tmp/beepboop
Task: 1: varnishd -V
...
```

If no NAME is given, and there's only one candidate but it's not
pingable, just error out:

```
Info: Working directory: /tmp/varnishgather.eZSa78VU/varnishgather-flamp-20241106-224856
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?
<EXIT>
```

Finally, if there are no given NAME, and we have more than one
candidate, we just propose a list of candidates and error out:

```
Info: Working directory: /tmp/varnishgather.dkGgfqHe/varnishgather-flamp-20241106-223210
using: varnishadm    ping,
We couldn't ping varnishd, Is it running and is
./varnishgather running with the right permissions?

We've also detected one or more varnishd that
could possibly be pinged with these -n arguments:
				-n /tmp/beepboop
				-n /tmp/beepboop2
<EXIT>
```
@gquintard
Copy link
Contributor Author

@simonvik , @hermunn , @dridi , merging at the end of the week unless someone objects

@simonvik
Copy link
Contributor

I dont have any objections, it seems to work but is a bit unreadable :) (but so is the rest of the code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants