Improve and actualize orchestrator-agent , extract many LVM commands into configuration file #20

Slach · 2018-03-21T13:38:25Z

Сustomize orchestrator-agent config, adding several *Command parameters
add handle signal syscall.SIGHUP with config.Reload, and terminate with SIGTERM, SIGKILL
Improve init.d script for SIGHUP and reload support
extract SeedTransferPort from const to config

Signed-off-by: Slach <bloodjazman@gmail.com>

add sourceHost parameter into /api/post-copy/ Signed-off-by: Slach <bloodjazman@gmail.com>

Signed-off-by: Slach <bloodjazman@gmail.com>

…animate_api

Signed-off-by: Slach <bloodjazman@gmail.com>

… daemon running Signed-off-by: Slach <bloodjazman@gmail.com>

and commandOutput to DEBUG log Signed-off-by: Slach <bloodjazman@gmail.com>

Signed-off-by: Slach <bloodjazman@gmail.com>

shlomi-noach

I'm a bit confused. I thought you were adapting orchestrator-agent to support Xtrabackup?
But I still see LVM terminology everywhere. Since I don't see anything xtrabackup-related, I'm asusming you have your own personal onfiguration where LVM mount command is actually some xtrabackup command. IS that the case?
If so, then I will request a rewrite of this PR ; the LVM flow is not necessarily similar to xtrabackup flow.

shlomi-noach · 2018-03-29T09:09:42Z

docker/entrypoint.sh

-    "AvailableSnapshotHostsCommand": "echo localhost\n127.0.0.1",
-    "SnapshotVolumesFilter": "-my-snapshot-",
-    "MySQLDatadirCommand": "echo '~/tmp'",
+    "SnapshotMountPoint": "${SnapshotMountPoint:-/mysql-data}",


I'd prefer using conventional shell naming, e.g. ${SNAPSHOT_MOUNT_POINT}. This applies to all of the rest of the variables.

shlomi-noach · 2018-03-29T09:10:16Z

etc/init.d/orchestrator-agent.bash

@@ -26,6 +26,11 @@ DESC="orchestrator-agent: MySQL management agent"
 PIDFILE=/var/run/$NAME.pid
 SCRIPTNAME=/etc/init.d/$NAME

+# This files can be used to inject pre-service execution
+# scripts, such as exporting variables or whatever. It's yours!
+[ -f /etc/default/orchestrator-agent ] && . /etc/default/orchestrator-agent


Very good. If we're down this rabbit hole, let's also add /etc/profile.d/orchestrator-agent to the party.

/etc/profile.d/orchestrator-agent
also added see here https://github.com/Slach/orchestrator-agent/blob/reanimate_api/etc/init.d/orchestrator-agent.bash#L33

shlomi-noach · 2018-03-29T09:10:48Z

etc/init.d/orchestrator-agent.bash

@@ -60,21 +65,39 @@ case "$1" in
    PID=$(cat $PIDFILE)
    cd $DAEMON_PATH
    if [ -f $PIDFILE ]; then
-      kill -HUP $PID
-      printf "%s\n" "Ok"
+      kill -TERM $PID


shlomi-noach · 2018-03-29T09:11:12Z

etc/init.d/orchestrator-agent.bash

      rm -f $PIDFILE
+      # Wait for orchestrator to stop otherwise restart may fail.


Change to "Wait for orchestrator-agent to stop..."

shlomi-noach · 2018-03-29T09:12:34Z

go/cmd/orchestrator-agent/main.go

-	sig := <-c
-	log.Fatalf("Got signal: %+v", sig)
+	signal.Notify(c, syscall.SIGHUP, syscall.SIGKILL, syscall.SIGTERM, syscall.SIGTERM)
+	go func() {


This doesn't block. Can you please explain the reasoning for changing from blocking to non-blocking?

it's a just backported your code from here ;)
https://github.com/github/orchestrator/blame/master/go/logic/orchestrator.go#L138
i can restore to blocking behavior

yes please.

ok. i return blocking behavior

shlomi-noach · 2018-03-29T09:13:15Z

go/cmd/orchestrator-agent/main.go

+				config.Reload()
+			case syscall.SIGTERM, syscall.SIGKILL, syscall.SIGINT:
+				log.Infof("Received %s. Shutting down orchestrator-agent", sig.String())
+				// probably should poke other go routines to stop cleanly here ...


If you poke goroutines there's nothing to promise they will complete before exiting.

and also fixed some mistypes in init.d script Signed-off-by: Slach <bloodjazman@gmail.com>

Slach · 2018-03-29T10:05:24Z

yes i just work for adoption for current LVM based commands for add oportunity use xtrabackup with LVM similar flow

ok. i rewrite PR header

shlomi-noach · 2018-03-29T10:09:11Z

Then please hold off and let's discuss. IMO all the new

* `LogicalVolumesCommand`              (string), command which list logical volumes, default implementation used lvs
+* `GetMountCommand`                    (string), command which get mount point parameters by mount point name, default implementation over cat /etc/mtab
+* `UnmountCommand`                     (string), command which unmount current mount point, default implementation just execute umount
+* `MountLVCommand`                     (string), command which mount selected snapshot, default implementation execute mount selected LVM snapshot 
+* `RemoveLVCommand`                    (string), command which remove selected snapshot, default implementation execute lvremove selected LVM snapshot
+* `MySQLTailErrorLogCommand`           (string), command which return last 20 lines from MySQL @@log_error file
+* `GetLogicalVolumeFSTypeCommand`      (string), command which return logical volume filesystem type
 * `CreateSnapshotCommand`              (string), command which creates new LVM snapshot of MySQL data

commands should be unfortunately removed. And you should add specialized Xtrabackup commands, and then design a Xtrabackup flow. Which takes this PR to a very different place.

Signed-off-by: Slach <bloodjazman@gmail.com>

Slach · 2018-03-29T11:50:37Z

hm, but i'm not broke current functionality
and in my xtrabackup flow is really similar with current lvm based implementation
you can see it here
https://bitbucket.org/production_databases/mysql_live_upgrade/src

Signed-off-by: Slach <bloodjazman@gmail.com>

shlomi-noach · 2018-04-08T10:24:52Z

go/http/api.go

@@ -363,6 +363,10 @@ func (this *HttpAPI) PostCopy(params martini.Params, r render.Render, req *http.
 	if err := this.validateToken(r, req); err != nil {
 		return
 	}
+	qs := req.URL.Query()
+	if sourceHost, exists := params["sourceHost"]; !exists || sourceHost=="" {


In golang, if the value does not exist in the map, then the returned value is "" anyway. So you can rewrite as:

if sourceHost := params["sourceHost"]; sourceHost == "" {

shlomi-noach · 2018-04-08T10:29:45Z

hm, but i'm not broke current functionality

True!

(and I'm returned from vacation, sorry for the late response)

But still I do not feel comfortable that a command named MountLVCommand would issue a, say, xtrabackup prepare.
There may be similar mapping right now, but the result code & configuration is confusing IMO. If I were a user of this product, I wouldn't feel good about configuring Xtrabackup commands under LVM based configuration.

This obviously does not reflect on your excellent job (!) in making orchestrator-agent run Xtrabackup, for which I'm grateful :)

Let's try and think what would be an elegant solution to this. Do we:

abstract away those commands?
Define some meta-flow?
Copy+paste to indicate Xtrabackup specific command & flow?

MaxFedotov · 2018-04-11T22:01:07Z

Hello @shlomi-noach . Sorry for bursting into the conversation, but seems like I’m working right now on the same problem, as it is in this PR - adding different backup methods (xtrabackup, xtrabackup-stream, mydumper, mysqldump) for orchestrator-agent in order to automate provisioning of new slave hosts.
I’ve made a small research of code in orchestrator and orchestrator agent code and I want to share and discuss my ideas about adding xtrabackup\mydumper\mysqlbackup seed capabilities to orchestrator-agent. I also created a draft with necessary API calls and overall process, which will be needed in order to implement this functionality.
Maybe it is possible to create a new issue, where we can discuss this?

Slach · 2018-04-12T04:39:22Z

@MaxFedotov great ;) if you send new PR and new issue, maybe i don't need finalize my branch, or you can share your ideas on this PR, and we will discuss together

shlomi-noach · 2018-04-12T05:49:18Z

@MaxFedotov hi! And this reminds me I got an email from you, 5 days ago, to which I haven't responded. Sorry about that -- I was on vacation and the email got lost.

Yes, please open a new issue and we can discuss.

Just FYI that I haven't been working on orchestrator-agent for ages now, and am not using it myself.

Thank you!

MaxFedotov · 2018-04-12T09:23:49Z

@shlomi-noach No problem, vacations are a good time to relax from everything :)
And it seems like issues are disabled for this repo. Can you please enable them?
@Slach Will be glad to share my ideas with you! I think I will be able to complete everything in a couple of weeks

shlomi-noach · 2018-04-12T09:59:38Z

Issues now enabled.

timhughes · 2018-09-03T14:09:06Z

My two cents: I am looking at doing something similar but using ZFS or BTRFS snapshots rather than the LVM ones. Somehow making this plug-able would be excellent

Slach added 15 commits March 15, 2018 18:33

work in progress commit

32ec11b

Signed-off-by: Slach <bloodjazman@gmail.com>

Merge branch 'add_docker_and_vagrant' into reanimate_api

ad5b6a8

extract some LVM related commands into customizable config parameters

0d08d34

add sourceHost parameter into /api/post-copy/ Signed-off-by: Slach <bloodjazman@gmail.com>

add customizable MySQLTailErrorLogCommand

56e4362

Signed-off-by: Slach <bloodjazman@gmail.com>

add backward compatibility for old orchestrator post-copy call

caa8db6

Signed-off-by: Slach <bloodjazman@gmail.com>

Merge branch 'master' of github.com:github/orchestrator-agent into re…

de935cc

…animate_api

add customizable GetLogicalVolumeFSTypeCommand

ac7cf93

Signed-off-by: Slach <bloodjazman@gmail.com>

add include /etc/ files for iject DAEMONOPTS or other settings before…

0d74d97

… daemon running Signed-off-by: Slach <bloodjazman@gmail.com>

add config reload by SIGHUP

3055395

and commandOutput to DEBUG log Signed-off-by: Slach <bloodjazman@gmail.com>

fix MySQLTailErrorLogCommand

5d1b823

Signed-off-by: Slach <bloodjazman@gmail.com>

add reload and kill TERM without HUP ;)

fb2e214

Signed-off-by: Slach <bloodjazman@gmail.com>

extract const SeedTransferPort to configurable parameter

16f920c

Signed-off-by: Slach <bloodjazman@gmail.com>

add sudoCmd to send \ receive seed data commands

0772999

Signed-off-by: Slach <bloodjazman@gmail.com>

add backward compatibility to postCopy

b2d10ed

Signed-off-by: Slach <bloodjazman@gmail.com>

add backward compatibility to postCopy

46468cd

Signed-off-by: Slach <bloodjazman@gmail.com>

Slach mentioned this pull request Mar 21, 2018

actualize Vagrant and fix problems on orchestrator-agent api with sqlite backend openark/orchestrator#445

Merged

4 tasks

add /etc/profile.d/orchestrator-agent usage

9450b7d

Signed-off-by: Slach <bloodjazman@gmail.com>

shlomi-noach reviewed Mar 29, 2018

View reviewed changes

fixed Docker entrypoint after code review

66cb2a2

and also fixed some mistypes in init.d script Signed-off-by: Slach <bloodjazman@gmail.com>

Slach changed the title ~~Improve orchestrator-agent for use xtrabackup command for data seeding between nodes~~ Improve and actualize orchestrator-agent , extract many LVM commands into configuration file Mar 29, 2018

Slach added 2 commits March 29, 2018 15:33

return rsync folder mounting mode back

5c0dddb

Signed-off-by: Slach <bloodjazman@gmail.com>

return blocking behavior

8febb27

Signed-off-by: Slach <bloodjazman@gmail.com>

Slach added 2 commits March 31, 2018 13:54

get sourceHost via query string

17700a7

Signed-off-by: Slach <bloodjazman@gmail.com>

parse sourceHost over http.Request.URL.Query

4056d58

Signed-off-by: Slach <bloodjazman@gmail.com>

shlomi-noach reviewed Apr 8, 2018

View reviewed changes

		rm -f $PIDFILE
		# Wait for orchestrator to stop otherwise restart may fail.

Improve and actualize orchestrator-agent , extract many LVM commands into configuration file #20

Are you sure you want to change the base?

Improve and actualize orchestrator-agent , extract many LVM commands into configuration file #20

Uh oh!

Conversation

Slach commented Mar 21, 2018

Uh oh!

shlomi-noach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Slach commented Mar 29, 2018

Uh oh!

shlomi-noach commented Mar 29, 2018

Uh oh!

Slach commented Mar 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shlomi-noach commented Apr 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxFedotov commented Apr 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Slach commented Apr 12, 2018

Uh oh!

shlomi-noach commented Apr 12, 2018

Uh oh!

MaxFedotov commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shlomi-noach commented Apr 12, 2018

Uh oh!

timhughes commented Sep 3, 2018

Uh oh!

Uh oh!

shlomi-noach commented Apr 8, 2018 •

edited

Loading

MaxFedotov commented Apr 11, 2018 •

edited

Loading

MaxFedotov commented Apr 12, 2018 •

edited

Loading