Script Runner addon

[gadelkareem]

Script Runner makes it easy to run a bash script on all your k8s nodes with one command.

hetzner-kube cluster addon install --name demo script-runner /path/to/script.sh 

[xetys]

wow, that's I cool. I'll review asap

[mavimo]

Yeah! I suggest to allow to specify the server type, actually we have 3 main "type" of server, etcd, master and worker.. I think most of time we need to run specific script only on a specific type of server, so maybe have a flag to define what kind of server should be affected from script execution should be a big improvement.

[gadelkareem]

@mavimo we can use the power of bash and keep the addon simple :)

#!/bin/bash

if [[ $(hostname -s) = *master* ]]; then
    exit 0
fi

[mavimo]

@gadelkareem it's a possibility, so the addon is simple but bash script should be "complex" ;)

I think is also a bit fragile because we can't guarantee that the name of server match the master / worker / etcd name, we generate new server with the specified name but a user can add the server to the pool with a different name or rename it.. so IMHO we should move server type definition on addon instead of into the script that we deploy

[gadelkareem]

@mavimo I see that the names are not constant, especially if it is an external server being added. Maybe we should have a file on the server to hint its type?

I think the bash script will probably be much more complex than the hostname check example above. And since the user already knows his own cluster then it should be trivial to manage the script based on the hostname or any other properties. Because if we start adding a new argument for each use case then it will not be as simple as now.

For example, with the current use case of choosing the instance group to run on, we will need to think if the user also wants to run the script on 2 groups (master and worker) or 3 of them. And what if there is a new group created then we need to modify this addon. That's why I always prefer to keep it simple and if we have feature request then it could be added to it.

[mavimo]

@gadelkareem WDYT to send as args to script invocation some information about current server / cluster topology?

[gadelkareem]

@mavimo good idea! which info do you think would be nice to pass to the script?

[mavimo]

@gadelkareem NB: I'm not sure but what we should pass, I'll try to write some proposal for discussion 😄

We should start with something like:

• Server type (master, worker, etcd) or a combination of them (some server should have more than one role at time) in order to allow script to have specific config (eg port exposed by firewall? ;) )
• Master IP: because some time script should need to dialog with the K8s API for specific operation
• Cluster type (without HA, with HA level 3, with HA level 4, ..) or List of server configs in order to make easy to add action that dialogue with other servers in pool

Maybe point 2 and 3 should be mixed if we build a JSON that contains all cluster information as second argument like:

 /tmp/script-to-run.sh master {"masters":[{"name":"master-1","ip":"111.222.0.1"}]...}

where script should process the second information if needed (eg: echo $2 | jq .masters[].ip)

[gadelkareem]

Ok so now the script will have the node group as first param then the cluster info which is the same as the JSON from the config file.

[gadelkareem]

I also allowed the display of output for each node since it will be different.

[xetys]

The LGTM already, but I would love to see some docs for the users on how to use that. With some examples, so users will know how to deal with the params. Maybe a distinct page like the HA-guide and wireguard info page?

[mavimo]

Please re-sort import definition, typically:

• stdlib packages (alphabetic)
• EMPTY LINE
• other packages (alphabetic)

[gadelkareem]

hmm.. did you mean remove empty line?

[mavimo]

I mean that we should change it to:

import (
	"encoding/json"
	"fmt"
	"io/ioutil"
	"log"
	"strings"
	"time"

	"github.com/xetys/hetzner-kube/pkg/clustermanager"
)

[mavimo]

I'm not sure to understand why we are going to use replacer here, this is needed since there are some info in JSON encoded that can generate invalid string argument? Is possibile to use some different approach in order to do not manipulate result value, like:

echo $(cat <<EOF
<JSON_MARSHALLED_VALUE>
EOF)

I'm not sure is doable, but will allow us to do not manipulate the value and be "safe" on changes we do in config..

[gadelkareem]

I don't think the run command will allow multi line. If it works for you, please let me know.

[mavimo]

@gadelkareem do you have time to write some doc? If you need some help please ping me :)

[gadelkareem]

@mavimo Since you are asking. Do you have an example config.json that i can use?

[gadelkareem]

added some docs!

[mavimo]

Markdown suggest to use:

Suggested change

	1) Node group, for example (master, worker, etcd)
	1. Node group, allowed values are `master`, `worker` and `etcd`

[mavimo]

Markdown suggest to use

Suggested change

	2) Cluster info JSON config, which is similar to the hetzner-kube config found in `~/.hetzner-kube/config.json`
	1. Cluster info JSON config, which is similar to the `hetzner-kube` configuration that you can found in `~/.hetzner-kube/config.json`

[mavimo]

Bash script should start with:

#!/bin/bash

this should be omitted (than specify on the doc that should not be included) or is missing here (than add to the sample)?

[mavimo]

Please add a space between title and text and between text and code sample

[mavimo]

@gadelkareem I know I should sound like a "nuisance" (sorry), added some comments to the doc :)

[gadelkareem]

cool done ;)