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

Shell command invocation (bug with v0.7.0-beta.4) #14

Closed
lakemike opened this issue Jan 31, 2021 · 6 comments
Closed

Shell command invocation (bug with v0.7.0-beta.4) #14

lakemike opened this issue Jan 31, 2021 · 6 comments

Comments

@lakemike
Copy link

lakemike commented Jan 31, 2021

I noticed a bug with shell invocation on v0.7.0-beta (all versions from .1 to .4). Essentially, I am unable to get stdout to be reported to a self-hosted healthchecks instance.

apiurl="https://serveraddress/ping"; uuid="uuidstring"; /root/bin/runitor-v0.7.0-beta.4-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"

Screenshot 2021-01-31 at 22 19 54

Latest stable version 0.6.0 seems to report stdout fine:

apiurl="https://serveraddress/ping"; uuid="uuidstring"; /root/bin/runitor-v0.6.0-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"

Screenshot 2021-01-31 at 22 20 29

Linux environment: Debian buster running as LXC container on QNAP NAS (x64)
Healthchecks: Latest docker image from linuxserver/healthchecks

@bdd
Copy link
Owner

bdd commented Jan 31, 2021

First of all, do you see the stderr and stdout on the terminal when using 0.7.0-beta4?

I tried to reproduce this under Debian Buster but I cannot. Terminal output and what's sent to HC API matches.

% podman run --rm -it -e CHECK_UUID=<uid> docker.io/debian:buster
root@80c5199c710e:/# (apt -qq update && apt install -y curl) >/dev/null 2>&1
root@80c5199c710e:/# curl -sLo /usr/bin/runitor https://github.com/bdd/runitor/releases/download/v0.7.0-beta.4/runitor-v0.7.0-beta.4-linux-amd64 && \
> chmod +x /usr/bin/runitor
root@80c5199c710e:/# cat /etc/debian_version
10.7
root@80c5199c710e:/# runitor -version
runitor v0.7.0-beta.4
root@80c5199c710e:/# runitor -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
err
out

@lakemike
Copy link
Author

lakemike commented Feb 1, 2021

Console output as per below (it's the same for stable v0.6):

# apiurl="https://serveraddress/ping"; uuid="uuidstring"; /root/bin/runitor-v0.7.0-beta.4-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
err
out
# cat /etc/debian_version
10.7
# runitor -version
runitor v0.7.0-beta.4

@lakemike
Copy link
Author

lakemike commented Feb 1, 2021

I have tested some further and probably found a helpful clue. Essentially, v0.7.0-beta also works on my system when I use the default instance of healthchecks (leaving api-url away or specifying hc-ping.com):

# uuid="77b3e526-a284-498a-8ef9-014c17d2ddaa"; /root/bin/runitor-v0.7.0-beta.4-linux-amd64 -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
err
out
# uuid="77b3e526-a284-498a-8ef9-014c17d2ddaa"; /root/bin/runitor-v0.6.0-linux-amd64 -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
err
out
# apiurl="https://hc-ping.com"; uuid="77b3e526-a284-498a-8ef9-014c17d2ddaa"; /root/bin/runitor-v0.7.0-beta.4-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
err
out

Screenshot 2021-02-01 at 09 31 39

v0.7.0-beta (any of the 4 beta versions) stops working when I use api-url to refer to my own healthchecks instance (see first post).

@bdd
Copy link
Owner

bdd commented Feb 1, 2021

I'm guessing the screenshot above is from the hc-ping(.)com instance.

I was gonna ask if there was any chance you set PING_BODY_LIMIT environment variable to 0 in your instance, but you say it works fine with 0.6.0, so that's moot. In Runitor's implementation, practically -api-url is always passed. When omitted it just has a default value (https://hc-ping.com). So that single line code path is always exercised. Looking at the diff from v0.6.0 to v.0.7.0-beta.4, I can't see any changes touching that.

If you instance is available for access over HTTP (clear text), you can also run tcpdump on the client to see if the client is actually sending the body.

...or for a more isolated test, I quickly whipped up a web server which returns 200 to every request and dumps the request path and received request body to stdout. Copy paste the code below to a file, say ok.go and run it with go run ok.go. It will spawn an HTTP server listening on TCP/8080.

package main

import (
	"fmt"
	"io/ioutil"
	"net/http"
)

func main() {
	mux := http.NewServeMux()
	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(200)
		fmt.Printf("%s %s (ua: %s)\n", r.Method, r.URL.Path, r.Header.Get("user-agent"))
		b, _ := ioutil.ReadAll(r.Body)
		defer r.Body.Close()
		if len(b) > 0 {
			fmt.Printf("Body:\n%s\n", string(b))
		}
	})
	http.ListenAndServe(":8080", mux)
}

Now you can point runitor to this one, with -api-url=http://localhost:8080 parameter when running on the same host and see if it sends the request body at all. I tried, locally, it does.

@lakemike
Copy link
Author

lakemike commented Feb 1, 2021

Here is the result for the server listening on 8080:

# go run runitor.ok.go 
POST /77b3e526-a284-498a-8ef9-014c17d2ddaa/start (ua: runitor/v0.6.0 (+https://bdd.fi/x/runitor))
POST /77b3e526-a284-498a-8ef9-014c17d2ddaa (ua: runitor/v0.6.0 (+https://bdd.fi/x/runitor))
Body:
err
out

POST /77b3e526-a284-498a-8ef9-014c17d2ddaa/start (ua: runitor/v0.7.0-beta.4 (+https://bdd.fi/x/runitor))
POST /77b3e526-a284-498a-8ef9-014c17d2ddaa (ua: runitor/v0.7.0-beta.4 (+https://bdd.fi/x/runitor))
Body:
err
out

These were the runitor calls:

# apiurl="http://localhost:8080"; uuid="77b3e526-a284-498a-8ef9-014c17d2ddaa"; /root/bin/runitor-v0.6.0-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"
# apiurl="http://localhost:8080"; uuid="77b3e526-a284-498a-8ef9-014c17d2ddaa"; /root/bin/runitor-v0.7.0-beta.4-linux-amd64 -api-url=$apiurl -uuid=$uuid -- bash -c "(echo out; echo foo; echo err >&2) | grep -v foo"

@bdd
Copy link
Owner

bdd commented Feb 1, 2021

So runitor is sending the body in the request.

At this point I don't really know what's up with your healthchecks instance, not showing the body.

I'm going to close this issue for now. Feel free to re-open it if you find a case where you can reproducibly alter the runitor's behavior.

@bdd bdd closed this as completed Feb 1, 2021
bdd added a commit that referenced this issue Mar 10, 2021
Go's net/http/request.go cannot deduce the body size if what's
behind io.Reader isn't a *bytes.{Buffer,Reader} or *strings.Reader.
*internal.RingBuffer merely implements the io.Reader interface, with
no way of hinting available data length. This causes requests to use
chunked Transfer-Encoding.

Apparently Django's development server doesn't support chunked
encoding but still returns HTTP 200 while the body says HTTP 400...
To cater to Healthchecks private instances served this way, we do a
type assertion for *internal.RingBuffer, and manually set content
length. This should result a request with a Content-Length header
and no Transfer-Encoding.

Fixes #14 and #16
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

No branches or pull requests

2 participants