actualize Vagrant and fix problems on orchestrator-agent api with sqlite backend by Slach · Pull Request #445 · openark/orchestrator

Slach · 2018-03-17T10:11:46Z

Related issue: #438

Description

This PR actualize Vagrant and fix problems on orchestrator-agent api with sqlite backend
i hope this help use innodbbackupex for data seeding between two mysql nodes

In case this PR introduced Go code changes:

contributed code is using same conventions as original code
code is formatted via gofmt (please avoid goimports)
code is built via ./build.sh
code is tested via go test ./go/...

…api with sqlite backend Signed-off-by: Slach <bloodjazman@gmail.com>

shlomi-noach · 2018-03-18T06:19:42Z

go/agent/agent_dao.go

 // PostCopy will request an agent to invoke post-copy commands
-func PostCopy(hostname string) (Agent, error) {
-	return executeAgentCommand(hostname, "post-copy", nil)
+func PostCopy(hostname, sourceHostname string) (Agent, error) {


Is this change going to break behavior for existing users?

could you help me figure out howto add optional parameter to martini routes?

Sure. Consider, for example,

https://github.com/github/orchestrator/blob/f465c53809370697566ca84b418fd43f7fe41ca9/go/http/api.go#L3291-L3292

and then

https://github.com/github/orchestrator/blob/f465c53809370697566ca84b418fd43f7fe41ca9/go/http/api.go#L2696-L2726

@shlomi-noach ok.
i add backwards compatibility on orchestrator-agent here
https://github.com/github/orchestrator-agent/pull/20/files#diff-fbdc8cde28a71961e53c2e83e2483c9aL619

and add some other improvments and tested it with percona-xtrabackup
currenly in my setup i have successfull data seed between nodes over netcat + xtrabackup + xbstream

…e_agent

… daemon running

shlomi-noach · 2018-03-21T14:32:08Z

etc/init.d/orchestrator.bash

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


Please explain why /etc/default/orchestrator? IF anything, I'd add /etc/profile.d/orchestrator which is what I should have done in the first place.

/etc/default it's a default behavion on debian based distributives

ok, i add usage /etc/profile.d/orchestrator and /etc/default/orchestrator

/etc/profile.d/orchestrator will have major priority

shlomi-noach · 2018-03-21T14:36:18Z

Thank you! This PR is growing bigger and deals with different problems, so it's becoming difficult for me to review it as a whole. Can you please elaborate on "i add backwards compatibility on orchestrator-agent here" ? Where is the backwards compatibility found?

Slach · 2018-03-21T14:48:41Z

go/agent/agent_dao.go

 			host_agent
 		where
-			IFNULL(last_checked < now() - interval ? minute, true)
+			IFNULL(last_checked < now() - interval ? minute, 1)


this change need for SQLlite compatibility cause SQLite desn't have BOOLEAN literal

Slach · 2018-03-21T14:49:35Z

go/agent/agent_dao.go

-func PostCopy(hostname string) (Agent, error) {
-	return executeAgentCommand(hostname, "post-copy", nil)
+func PostCopy(hostname, sourceHostname string) (Agent, error) {
+	return executeAgentCommand(hostname, fmt.Sprintf("post-copy/?sourceHost=%s", sourceHostname), nil)


there is backwards compatibility

Slach · 2018-03-21T14:49:56Z

Can you please elaborate on "i add backwards compatibility on orchestrator-agent here" ?
backwards behavior it's here

https://github.com/github/orchestrator-agent/pull/20/files#diff-fbdc8cde28a71961e53c2e83e2483c9aL619
and here
https://github.com/github/orchestrator/pull/445/files#diff-17473f4f3d0788c153babef6ddbba1faR551

shlomi-noach · 2018-03-22T07:59:04Z

I will get back on this early next week.

Slach · 2018-03-22T12:25:13Z

ok. i hope my PR will be merged ;)

shlomi-noach

Thank you for the PR! and I'm sorry for the slow response; I have had some busy days and will have yet more busy days ahead of me.
One critical change, and some questions.

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

.gitignore

+.idea/
+*.deb
+*.pcap
+*.log


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

Vagrantfile

-  config.vm.synced_folder '.', '/orchestrator', type: 'rsync',
-    rsync__auto: true
+  config.vm.synced_folder '.', '/orchestrator'
+  #, type: 'rsync', rsync__auto: true


Can you please explain this change?

rsync__auto want work on my Vagrant environment ;(
when i change some sources on host OS, it's not rsynced into guest maching
ok. i rollback this change

Sorry, please elaborate. Did you introduce rsync__auto in the first place?

i try use following variants
1)

config.vm.synced_folder '.', '/orchestrator', type: 'rsync'

it's require run vagrant reload when i change any file on my host machine
2)

config.vm.synced_folder '.', '/orchestrator', type: 'rsync', rsync__auto: true

it not worked, and i don't investigate why
then i just commented this options and default sharing mode over vboxfs works as i expected

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

build.sh

  gofmt -s -w  go/
  rsync -qa ./resources $builddir/orchestrator${prefix}/orchestrator/
-  rsync -qa ./conf/orchestrator-sample.* $builddir/orchestrator${prefix}/orchestrator/
+  rsync -qa ./conf/orchestrator-sample*.conf.json $builddir/orchestrator${prefix}/orchestrator/


shlomi-noach · 2018-03-29T09:18:54Z

build.sh

-    # n CentOD 6 box: we only want the rpms for CentOS6
-    # Add "-centos6" to the file name.
-    ls ${TOPDIR:-?}/*.rpm | while read f; do centos_file=$(echo $f | sed -r -e "s/^(.*)-${RELEASE_VERSION}(.*)/\1-centos6-${RELEASE_VERSION}\2/g") ; mv $f $centos_file ; done
+  if [[ -e /etc/centos-release ]]; then


better: -f

shlomi-noach · 2018-03-29T09:19:07Z

conf/orchestrator-sample-sqlite.conf.json

+  "GraphiteAddr": "",
+  "GraphitePath": "",
+  "GraphiteConvertHostnameDotsToUnderscores": true
+}


shlomi-noach · 2018-03-29T09:21:01Z

go/agent/agent_dao.go


 	seedFromLogicalVolume := sourceAgent.LogicalVolumes[0]
-	seedStateId, _ = submitSeedStateEntry(seedId, fmt.Sprintf("Mounting logical volume: %s", seedFromLogicalVolume.Path), "")
+	seedStateId, _ = submitSeedStateEntry(seedId, fmt.Sprintf("%s Mounting logical volume: %s", sourceHostname, seedFromLogicalVolume.Path), "")


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

go/agent/agent_dao.go

-	seedStateId, _ = submitSeedStateEntry(seedId, fmt.Sprintf("Waiting some time for %s to start listening for incoming data", targetHostname), "")
-	time.Sleep(2 * time.Second)
+	seedStateId, _ = submitSeedStateEntry(seedId, fmt.Sprintf("Waiting %d seconds for %s to start listening for incoming data", config.Config.SeedWaitSecondsBeforeSend, targetHostname), "")
+	time.Sleep(time.Duration(config.Config.SeedWaitSecondsBeforeSend) * time.Second)


shlomi-noach · 2018-03-29T09:23:19Z

go/db/generate_base.go

 		  last_seen timestamp NULL DEFAULT NULL,
 		  mysql_port smallint(5) unsigned DEFAULT NULL,
-		  count_mysql_snapshots smallint(5) unsigned NOT NULL,
+		  count_mysql_snapshots smallint(5) unsigned NOT NULL DEFAULT 0,


🚫 Must change: existing SQL must never ever change. The most we can do is add a modification in generate_patches.go. However, modifying a column is not supported by sqlite so this change must unfortunately go away.

The value is implicitly 0 anyway, what is the concern?

oh. ok. sorry i will rollback this change

yes concern is
count_mysql_snapshots not used when try insert into host_agent table
and sql query is failed when agent submitting

rollback generate_base.go after openark#445 (comment)

…e_agent

shlomi-noach · 2018-04-08T10:31:42Z

build.sh

+        # n CentOD 6 box: we only want the rpms for CentOS6
+        # Add "-centos6" to the file name.
+        ls ${TOPDIR:-?}/*.rpm | while read f; do centos_file=$(echo $f | sed -r -e "s/^(.*)-${RELEASE_VERSION}(.*)/\1-centos6-${RELEASE_VERSION}\2/g") ; mv $f $centos_file ; done
+      fi


shlomi-noach · 2018-04-08T10:34:04Z

vagrant/admin-build.sh


 # Discover instances
-/usr/bin/orchestrator -c discover -i localhost
+/usr/bin/orchestrator --verbose --debug --stack -c redeploy-internal-db


What happened that the above became necessary? Current logic should make it safe to run orchestrator without specifying redeploy-internal-db.

ohh, sorry, i multiple run vagrant up and add redeploy-internal-db to try fix errors when using sqlite backend
i revert this changes right now

Slach · 2018-04-13T11:36:37Z

@shlomi-noach would this PR ready for merge? i think i resolve any recomendation

…e_agent

…reanimate_agent

Slach · 2018-04-25T15:08:11Z

thanks!

shlomi-noach · 2018-04-25T15:16:09Z

Thank you!

Slach added 2 commits March 17, 2018 15:08

actualize Vagrant environment and fix problems on orchestrator-agent …

680e355

…api with sqlite backend Signed-off-by: Slach <bloodjazman@gmail.com>

add sourceHost parameter into agent /api/post-copy/

8bb825b

shlomi-noach reviewed Mar 18, 2018

View reviewed changes

Slach added 5 commits March 18, 2018 17:14

Merge branch 'master' of github.com:github/orchestrator into reanimat…

57a5f28

…e_agent

sourceHostname to logging

17f5a32

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

9d9dde5

… daemon running

more clarified log message

2d1b5d9

add SeedWaitSecondsBeforeSend config parameter

c96b2fb

shlomi-noach reviewed Mar 21, 2018

View reviewed changes

backwards compatibility for api/post-copy agent command

3c4a64f

Slach commented Mar 21, 2018

View reviewed changes

add /etc/profile.d/orchestrator usage

fe728b3

shlomi-noach self-assigned this Mar 22, 2018

Slach mentioned this pull request Mar 24, 2018

howto attach one mysql server to another with seed\receive data over orchestrator-agent? #438

Closed

shlomi-noach suggested changes Mar 29, 2018

View reviewed changes

Slach added 3 commits March 29, 2018 15:17

return rsync shared folder back

989017b

rollback generate_base.go after openark#445 (comment)

Merge branch 'master' of github.com:github/orchestrator into reanimat…

212e76f

…e_agent

fix agent

9fbf9f7

shlomi-noach reviewed Apr 8, 2018

View reviewed changes

remove redeploy-internal-db

0c9a7d0

Slach added 3 commits April 13, 2018 16:37

Merge branch 'master' of github.com:github/orchestrator into reanimat…

b2e2de0

…e_agent

Merge branch 'master' into reanimate_agent

9c21c33

Merge branch 'reanimate_agent' of github.com:Slach/orchestrator into …

d55d468

…reanimate_agent

shlomi-noach approved these changes Apr 17, 2018

View reviewed changes

Shlomi Noach added 2 commits April 17, 2018 12:24

Merge branch 'master' into reanimate_agent

3920ea3

Merge branch 'master' into reanimate_agent

43e0184

shlomi-noach merged commit b3cb488 into openark:master Apr 25, 2018

Conversation

Slach commented Mar 17, 2018

Description

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 Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

shlomi-noach commented Mar 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Slach commented Mar 21, 2018

Uh oh!

shlomi-noach commented Mar 22, 2018

Uh oh!

Slach commented Mar 22, 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!

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

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 Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Slach commented Apr 25, 2018

Uh oh!

shlomi-noach commented Apr 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Slach Mar 21, 2018 •

edited

Loading

shlomi-noach Mar 29, 2018 •

edited

Loading

Slach commented Apr 13, 2018 •

edited

Loading