Docker image missing but referenced bits

[jadestorm]

Hi folk! Sorry for the flurry of issues -- hopefully this is all helpful rather than annoying. =)

I am currently using the docker image: signal18/replication-manager:v3.1.7

In that image "as is", the configuration includes references to the following (from etc/local/config.toml.docker in git):

prov-db-binary-basedir= "/usr/sbin"
prov-db-client-basedir= "/usr/bin"
backup-mysqlclient-path ="/usr/bin/mysql"
backup-mysqlbinlog-path = "/usr/bin/mysqlbinlog"
haproxy-binary-path= "/usr/sbin/haproxy"
proxysql-binary-path="/usr/bin/proxysql"
backup-restic-binary-path="/usr/bin/restic"
sysbench-binary-path = "/usr/bin/sysbench"
backup-mydumper-path = "/usr/bin/mydumper"
backup-myloader-path = "/usr/bin/myloader"
backup-mysqldump-path = "/usr/bin/mysqldump"

Because the image is using mariadb-client instead of mysql, backup-mysqlbinlog-path should be /usr/bin/mariadb-binlog.

Meanwhile, mydumper and myloader are completely absent, but referenced. This causes warnings about being unable to find the version of mydumper. Now -- even commenting it out/removing it seems to cause those warnings as well, the warning bit might be expected, and I have silenced the warning myself using the ignore warnings option.

Mostly I just wanted to make sure y'all were aware of it. It's not hard to work around. =)

[caffeinated92]

Hi @jadestorm,

No worries, it's helpful for us to have you got our back.

The client path can be empty if you have installed the binary and already set in env path.

This is our function to get the path:

func (cluster *Cluster) GetMysqlclientPath() string {
	if cluster.Conf.BackupMysqlclientPath == "" {
		// Return installed mysql client on repman host instead of embedded if exists
		if out, err := exec.Command("which", "mariadb").Output(); err == nil {
			path := strings.Trim(string(out), "\r\n")
			return path
		}

		// Return installed mysql client on repman host instead of embedded if exists
		if out, err := exec.Command("which", "mysql").Output(); err == nil {
			path := strings.Trim(string(out), "\r\n")
			return path
		}
		return cluster.GetShareDir() + "/" + cluster.Conf.GoArch + "/" + cluster.Conf.GoOS + "/mysql"
	}
	return cluster.Conf.BackupMysqlclientPath
}

I will look up to the mydumper instalation, in case we forgot to include it in our dockerfile

[caffeinated92]

This PR might help with the issue for the next releases: #1208

[jadestorm]

Awesome =) I mostly noticed it because I was getting warnings about it's inability to get a version for mydumper and mysqldump. The latter was solved by switching to mariadb-dump. While the warning persists if I command out the mydumper line, I did not try setting it to an empty string. However, since you are adding it back in a coming version, I will just wait on that. =) Thanks!

[svaroqui]

I remember we included mydumper from compile because we hit a bug in mydumper at a sponsoring client , @caffeinated92 are you sure this not only a issue with the default config not pointing to the correct place

[jadestorm]

If that's the case, would it be a decent compromise to update the default docker config to have blanks for mydumper and myrestore, but maybe include it just in case someone does want to use it? (they can enable it themselves) Of course -- anyone coudl always extend their own docker image build to include them.

I would still recommend changing mysqldump to mariadb-dump since mysqldump does not exist under the currently build.

Now -- I realize generally people would be overriding the entire /etc/replication-manager path, as I am, but I do tend to poke inside docker images first to get some ideas for what it believes the defaults should be.

[svaroqui]

well i guess it was not such good idea unless the very last release , @caffeinated92 fixed an issue with empty config that a cluster named default was created because of case insensitive issue in config parsing this what putting the all multi cluster monitoring in P.L.S (translated from french argo recovery position or crazy:) if that happens a forced rm -rf /datadir/* is needed before starting with non empty config

[caffeinated92]

Sorry for the late reply,

If you want to enable the function to auto-resolve, you need to comment or empty the path of the binary client.

I did not change any of the default. I only changed the default dockerfile to be similar with .pro

The only changes was, it will also install mariadb and other needed tools (mydumper, etc) like the docker version using .pro. (Which might be the reason mydumper not found)

[jadestorm]

Hi @caffeinated92 -- that's where the problem lies then -- if I don't specify it at all I get:
Failed to get mydumper version on repman: fork/exec /usr/bin/mydumper: no such file or directory (WARN0120)

I'll try emptying the variable and see if that changes one moment.

[jadestorm]

Looks like it's because of this:

	monitorCmd.Flags().StringVar(&conf.BackupMyDumperPath, "mydumper-path", "/usr/bin/mydumper", "Deprecate Path to mydumper binary")
	monitorCmd.Flags().MarkDeprecated("mydumper-path", "Deprecated for backup-mydumper-path")

In the absense of a setting it's defaulting to /usr/bin/mydumper.

Sorry to be nitpicky! Mostly doing it to try to help with polish and such and .. well I'm being picky about warnings. =)

Innnteresting. If I set it to blank, I end up with:

Failed to get mydumper version on repman: fork/exec /usr/share/replication-manager/amd64/linux/mydumper: no such file or directory

[svaroqui]

Yup this is a relic from the time we embedded client binaries, but was removed long time ago