Improve and actualize orchestrator-agent , extract many LVM commands into configuration file#20
Improve and actualize orchestrator-agent , extract many LVM commands into configuration file#20Slach wants to merge 21 commits intogithub:masterfrom
Conversation
Slach
commented
Mar 21, 2018
- С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>
Signed-off-by: Slach <bloodjazman@gmail.com>
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>
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
shlomi-noach
left a comment
There was a problem hiding this comment.
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.
docker/entrypoint.sh
Outdated
| "AvailableSnapshotHostsCommand": "echo localhost\n127.0.0.1", | ||
| "SnapshotVolumesFilter": "-my-snapshot-", | ||
| "MySQLDatadirCommand": "echo '~/tmp'", | ||
| "SnapshotMountPoint": "${SnapshotMountPoint:-/mysql-data}", |
There was a problem hiding this comment.
I'd prefer using conventional shell naming, e.g. ${SNAPSHOT_MOUNT_POINT}. This applies to all of the rest of the variables.
|
|
||
| # 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 |
There was a problem hiding this comment.
Very good. If we're down this rabbit hole, let's also add /etc/profile.d/orchestrator-agent to the party.
There was a problem hiding this comment.
/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
| if [ -f $PIDFILE ]; then | ||
| kill -HUP $PID | ||
| printf "%s\n" "Ok" | ||
| kill -TERM $PID |
etc/init.d/orchestrator-agent.bash
Outdated
| printf "%s\n" "Ok" | ||
| kill -TERM $PID | ||
| rm -f $PIDFILE | ||
| # Wait for orchestrator to stop otherwise restart may fail. |
There was a problem hiding this comment.
Change to "Wait for orchestrator-agent to stop..."
go/cmd/orchestrator-agent/main.go
Outdated
| sig := <-c | ||
| log.Fatalf("Got signal: %+v", sig) | ||
| signal.Notify(c, syscall.SIGHUP, syscall.SIGKILL, syscall.SIGTERM, syscall.SIGTERM) | ||
| go func() { |
There was a problem hiding this comment.
This doesn't block. Can you please explain the reasoning for changing from blocking to non-blocking?
There was a problem hiding this comment.
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
go/cmd/orchestrator-agent/main.go
Outdated
| 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 ... |
There was a problem hiding this comment.
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>
|
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 |
|
Then please hold off and let's discuss. IMO all the new 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>
Signed-off-by: Slach <bloodjazman@gmail.com>
|
hm, but i'm not broke current functionality |
Signed-off-by: Slach <bloodjazman@gmail.com>
Signed-off-by: Slach <bloodjazman@gmail.com>
| return | ||
| } | ||
| qs := req.URL.Query() | ||
| if sourceHost, exists := params["sourceHost"]; !exists || sourceHost=="" { |
There was a problem hiding this comment.
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 == "" {
True! (and I'm returned from vacation, sorry for the late response) But still I do not feel comfortable that a command named This obviously does not reflect on your excellent job (!) in making Let's try and think what would be an elegant solution to this. Do we:
|
|
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. |
|
@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 |
|
@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 Thank you! |
|
@shlomi-noach No problem, vacations are a good time to relax from everything :) |
|
Issues now enabled. |
|
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 |