{"action":"created","comment":{"url":"https://api.github.com/repos/coreos/fleet/pulls/comments/59887074","id":59887074,"diff_hunk":"@@ -41,34 +41,60 @@ func TestReconfigureServer(t *testing.T) {\n \t\tt.Fatal(err)\n \t}\n \n-\t// NOTE: we need to sleep once here to get reliable results.\n-\t// Without this sleep, the entire fleetd test always ends up succeeding\n-\t// no matter whether SIGHUP came or not.\n-\t_, _ = cluster.MemberCommand(m0, \"sh\", \"-c\", `'sleep 2'`)\n-\n \terr = waitForFleetdSocket(cluster, m0)\n \tif err != nil {\n \t\tt.Fatalf(\"Failed to get a list of fleetd sockets: %v\", err)\n \t}\n \n-\t// send a SIGHUP to fleetd, and periodically checks if a message\n+\tunit := fmt.Sprintf(\"fixtures/units/hello.service\")\n+\tstdout, stderr, err := cluster.Fleetctl(m0, \"start\", unit)\n+\tif err != nil {\n+\t\tt.Fatalf(\"Failed starting unit: \\nstdout: %s\\nstderr: %s\\nerr: %v\", stdout, stderr, err)\n+\t}\n+\n+\t_, err = cluster.WaitForNActiveUnits(m0, 1)\n+\tif err != nil {\n+\t\tt.Fatal(err)\n+\t}\n+\n+\t// Trigger AgentReconciler just here\n+\tstdout, stderr, err = cluster.Fleetctl(m0, \"unload\", unit)\n+\tif err != nil {\n+\t\tt.Fatalf(\"Failed unloading unit: \\nstdout: %s\\nstderr: %s\\nerr: %v\", stdout, stderr, err)\n+\t}\n+\n+\t// Send a SIGHUP to fleetd, and periodically checks if a message\n \t// \"Reloading configuration\" appears in fleet's journal, up to timeout (15) seconds.\n-\tstdout, _ := cluster.MemberCommand(m0, \"sudo\", \"systemctl\", \"kill\", \"-s\", \"SIGHUP\", \"fleet\")\n+\tstdout, _ = cluster.MemberCommand(m0, \"sudo\", \"systemctl\", \"kill\", \"-s\", \"SIGHUP\", \"fleet\")\n \tif strings.TrimSpace(stdout) != \"\" {\n \t\tt.Fatalf(\"Sending SIGHUP to fleetd returned: %s\", stdout)\n \t}\n \n-\terr = waitForReloadConfig(cluster, m0)\n-\tif err != nil {\n-\t\tt.Fatalf(\"Failed to get log about reconfiguration: %v\", err)\n+\t// Watch the logs if fleet was correctly reloaded\n+\terrSigHup := waitForReloadConfig(cluster, m0)\n+\tif errSigHup != nil {\n+\t\tt.Logf(\"Failed to ensure that fleet was correctly reloaded: %v\", errSigHup)\n \t}\n \n \t// check if fleetd is still running correctly, by running fleetctl status\n-\t_, _, err = cluster.Fleetctl(m0, \"list-units\")\n+\t// Even if the log message do not show up this test may catch the error.\n+\tstdout, _, err = cluster.Fleetctl(m0, \"list-units\")\n \tif err != nil {\n \t\tt.Fatalf(\"Unable to check list-units. Please check for fleetd socket. err:%v\", err)\n \t}\n \n+\t// Ensure that fleet received SIGHUP, if not then just skip this test\n+\t// probably due to journald and or other delays.\n+\tif errSigHup != nil {\n+\t\terr = waitForReloadConfig(cluster, m0)","path":"functional/server_test.go","position":60,"original_position":60,"commit_id":"9b173e1fbf3a1bf46495849080a534c5e0c05464","original_commit_id":"9b173e1fbf3a1bf46495849080a534c5e0c05464","user":{"login":"antrik","id":19555,"avatar_url":"https://avatars.githubusercontent.com/u/19555?v=3","gravatar_id":"","url":"https://api.github.com/users/antrik","html_url":"https://github.com/antrik","followers_url":"https://api.github.com/users/antrik/followers","following_url":"https://api.github.com/users/antrik/following{/other_user}","gists_url":"https://api.github.com/users/antrik/gists{/gist_id}","starred_url":"https://api.github.com/users/antrik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antrik/subscriptions","organizations_url":"https://api.github.com/users/antrik/orgs","repos_url":"https://api.github.com/users/antrik/repos","events_url":"https://api.github.com/users/antrik/events{/privacy}","received_events_url":"https://api.github.com/users/antrik/received_events","type":"User","site_admin":false},"body":"I don't understand what the point is in checking this again here? If it failed earlier, this means the test wouldn't produce any useful results anyway I think?...","created_at":"2016-04-15T14:56:01Z","updated_at":"2016-04-15T14:56:01Z","html_url":"https://github.com/coreos/fleet/pull/1554#discussion_r59887074","pull_request_url":"https://api.github.com/repos/coreos/fleet/pulls/1554","_links":{"self":{"href":"https://api.github.com/repos/coreos/fleet/pulls/comments/59887074"},"html":{"href":"https://github.com/coreos/fleet/pull/1554#discussion_r59887074"},"pull_request":{"href":"https://api.github.com/repos/coreos/fleet/pulls/1554"}}},"pull_request":{"url":"https://api.github.com/repos/coreos/fleet/pulls/1554","id":66644519,"html_url":"https://github.com/coreos/fleet/pull/1554","diff_url":"https://github.com/coreos/fleet/pull/1554.diff","patch_url":"https://github.com/coreos/fleet/pull/1554.patch","issue_url":"https://api.github.com/repos/coreos/fleet/issues/1554","number":1554,"state":"closed","locked":false,"title":"functional: improve TestReconfigureServer test","user":{"login":"tixxdz","id":1549291,"avatar_url":"https://avatars.githubusercontent.com/u/1549291?v=3","gravatar_id":"","url":"https://api.github.com/users/tixxdz","html_url":"https://github.com/tixxdz","followers_url":"https://api.github.com/users/tixxdz/followers","following_url":"https://api.github.com/users/tixxdz/following{/other_user}","gists_url":"https://api.github.com/users/tixxdz/gists{/gist_id}","starred_url":"https://api.github.com/users/tixxdz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tixxdz/subscriptions","organizations_url":"https://api.github.com/users/tixxdz/orgs","repos_url":"https://api.github.com/users/tixxdz/repos","events_url":"https://api.github.com/users/tixxdz/events{/privacy}","received_events_url":"https://api.github.com/users/tixxdz/received_events","type":"User","site_admin":false},"body":"Remove the sleep call that allowed us to produce reliable tests, it\r\nturns out it may fail in rare cases. I suspect this due to journald\r\ndelays and how things are serialized and started when running the test.\r\n\r\nInstead, start a normal unit then unload it to reproduce a real behaviour\r\nthen follow up with the SIGHUG on fleetd.\r\n\r\nIf the \"Reloading configuration\" message do not show up on the logs, then\r\nwe do not fail since the signal was received and processed, instead\r\ncontinue with the \"list-units\" check, if it succeed then the test should have\r\npassed at this point. However, we continue and check the logs that we ignored\r\npreviously since the test also checks for the\r\n\"Failed serving HTTP on listener\" which informs us that fleetd failed.\r\nSo we check the logs again if we find them we proceed with that message\r\nand check it, otherwise we skip the test at this point. We do not want\r\ndevelopers to have failing functional tests results due to obscure\r\ndelays. However the test succeed now and is more reliable. I have been running\r\nit for more than 1 hour now (more than 100 times), it did not fail and it was\r\nnever skipped.","created_at":"2016-04-15T13:44:30Z","updated_at":"2016-04-15T14:56:01Z","closed_at":"2016-04-15T14:09:49Z","merged_at":"2016-04-15T14:09:49Z","merge_commit_sha":"071230a3a93a415896f06468b9dc3f29a5c3fa06","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/coreos/fleet/pulls/1554/commits","review_comments_url":"https://api.github.com/repos/coreos/fleet/pulls/1554/comments","review_comment_url":"https://api.github.com/repos/coreos/fleet/pulls/comments{/number}","comments_url":"https://api.github.com/repos/coreos/fleet/issues/1554/comments","statuses_url":"https://api.github.com/repos/coreos/fleet/statuses/9b173e1fbf3a1bf46495849080a534c5e0c05464","head":{"label":"endocode:tixxdz/server-reconfigure-improvements","ref":"tixxdz/server-reconfigure-improvements","sha":"9b173e1fbf3a1bf46495849080a534c5e0c05464","user":{"login":"endocode","id":4068543,"avatar_url":"https://avatars.githubusercontent.com/u/4068543?v=3","gravatar_id":"","url":"https://api.github.com/users/endocode","html_url":"https://github.com/endocode","followers_url":"https://api.github.com/users/endocode/followers","following_url":"https://api.github.com/users/endocode/following{/other_user}","gists_url":"https://api.github.com/users/endocode/gists{/gist_id}","starred_url":"https://api.github.com/users/endocode/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/endocode/subscriptions","organizations_url":"https://api.github.com/users/endocode/orgs","repos_url":"https://api.github.com/users/endocode/repos","events_url":"https://api.github.com/users/endocode/events{/privacy}","received_events_url":"https://api.github.com/users/endocode/received_events","type":"Organization","site_admin":false},"repo":{"id":34001513,"name":"fleet","full_name":"endocode/fleet","owner":{"login":"endocode","id":4068543,"avatar_url":"https://avatars.githubusercontent.com/u/4068543?v=3","gravatar_id":"","url":"https://api.github.com/users/endocode","html_url":"https://github.com/endocode","followers_url":"https://api.github.com/users/endocode/followers","following_url":"https://api.github.com/users/endocode/following{/other_user}","gists_url":"https://api.github.com/users/endocode/gists{/gist_id}","starred_url":"https://api.github.com/users/endocode/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/endocode/subscriptions","organizations_url":"https://api.github.com/users/endocode/orgs","repos_url":"https://api.github.com/users/endocode/repos","events_url":"https://api.github.com/users/endocode/events{/privacy}","received_events_url":"https://api.github.com/users/endocode/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/endocode/fleet","description":"A Distributed init System","fork":true,"url":"https://api.github.com/repos/endocode/fleet","forks_url":"https://api.github.com/repos/endocode/fleet/forks","keys_url":"https://api.github.com/repos/endocode/fleet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/endocode/fleet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/endocode/fleet/teams","hooks_url":"https://api.github.com/repos/endocode/fleet/hooks","issue_events_url":"https://api.github.com/repos/endocode/fleet/issues/events{/number}","events_url":"https://api.github.com/repos/endocode/fleet/events","assignees_url":"https://api.github.com/repos/endocode/fleet/assignees{/user}","branches_url":"https://api.github.com/repos/endocode/fleet/branches{/branch}","tags_url":"https://api.github.com/repos/endocode/fleet/tags","blobs_url":"https://api.github.com/repos/endocode/fleet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/endocode/fleet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/endocode/fleet/git/refs{/sha}","trees_url":"https://api.github.com/repos/endocode/fleet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/endocode/fleet/statuses/{sha}","languages_url":"https://api.github.com/repos/endocode/fleet/languages","stargazers_url":"https://api.github.com/repos/endocode/fleet/stargazers","contributors_url":"https://api.github.com/repos/endocode/fleet/contributors","subscribers_url":"https://api.github.com/repos/endocode/fleet/subscribers","subscription_url":"https://api.github.com/repos/endocode/fleet/subscription","commits_url":"https://api.github.com/repos/endocode/fleet/commits{/sha}","git_commits_url":"https://api.github.com/repos/endocode/fleet/git/commits{/sha}","comments_url":"https://api.github.com/repos/endocode/fleet/comments{/number}","issue_comment_url":"https://api.github.com/repos/endocode/fleet/issues/comments{/number}","contents_url":"https://api.github.com/repos/endocode/fleet/contents/{+path}","compare_url":"https://api.github.com/repos/endocode/fleet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/endocode/fleet/merges","archive_url":"https://api.github.com/repos/endocode/fleet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/endocode/fleet/downloads","issues_url":"https://api.github.com/repos/endocode/fleet/issues{/number}","pulls_url":"https://api.github.com/repos/endocode/fleet/pulls{/number}","milestones_url":"https://api.github.com/repos/endocode/fleet/milestones{/number}","notifications_url":"https://api.github.com/repos/endocode/fleet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/endocode/fleet/labels{/name}","releases_url":"https://api.github.com/repos/endocode/fleet/releases{/id}","deployments_url":"https://api.github.com/repos/endocode/fleet/deployments","created_at":"2015-04-15T15:16:33Z","updated_at":"2016-01-27T08:29:06Z","pushed_at":"2016-04-15T14:00:18Z","git_url":"git://github.com/endocode/fleet.git","ssh_url":"git@github.com:endocode/fleet.git","clone_url":"https://github.com/endocode/fleet.git","svn_url":"https://github.com/endocode/fleet","homepage":"","size":7763,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"coreos:master","ref":"master","sha":"01d6766209e0f3911a02d66c7972062ebd971940","user":{"login":"coreos","id":3730757,"avatar_url":"https://avatars.githubusercontent.com/u/3730757?v=3","gravatar_id":"","url":"https://api.github.com/users/coreos","html_url":"https://github.com/coreos","followers_url":"https://api.github.com/users/coreos/followers","following_url":"https://api.github.com/users/coreos/following{/other_user}","gists_url":"https://api.github.com/users/coreos/gists{/gist_id}","starred_url":"https://api.github.com/users/coreos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coreos/subscriptions","organizations_url":"https://api.github.com/users/coreos/orgs","repos_url":"https://api.github.com/users/coreos/repos","events_url":"https://api.github.com/users/coreos/events{/privacy}","received_events_url":"https://api.github.com/users/coreos/received_events","type":"Organization","site_admin":false},"repo":{"id":13512328,"name":"fleet","full_name":"coreos/fleet","owner":{"login":"coreos","id":3730757,"avatar_url":"https://avatars.githubusercontent.com/u/3730757?v=3","gravatar_id":"","url":"https://api.github.com/users/coreos","html_url":"https://github.com/coreos","followers_url":"https://api.github.com/users/coreos/followers","following_url":"https://api.github.com/users/coreos/following{/other_user}","gists_url":"https://api.github.com/users/coreos/gists{/gist_id}","starred_url":"https://api.github.com/users/coreos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coreos/subscriptions","organizations_url":"https://api.github.com/users/coreos/orgs","repos_url":"https://api.github.com/users/coreos/repos","events_url":"https://api.github.com/users/coreos/events{/privacy}","received_events_url":"https://api.github.com/users/coreos/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/coreos/fleet","description":"A Distributed init System","fork":false,"url":"https://api.github.com/repos/coreos/fleet","forks_url":"https://api.github.com/repos/coreos/fleet/forks","keys_url":"https://api.github.com/repos/coreos/fleet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/coreos/fleet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/coreos/fleet/teams","hooks_url":"https://api.github.com/repos/coreos/fleet/hooks","issue_events_url":"https://api.github.com/repos/coreos/fleet/issues/events{/number}","events_url":"https://api.github.com/repos/coreos/fleet/events","assignees_url":"https://api.github.com/repos/coreos/fleet/assignees{/user}","branches_url":"https://api.github.com/repos/coreos/fleet/branches{/branch}","tags_url":"https://api.github.com/repos/coreos/fleet/tags","blobs_url":"https://api.github.com/repos/coreos/fleet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/coreos/fleet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/coreos/fleet/git/refs{/sha}","trees_url":"https://api.github.com/repos/coreos/fleet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/coreos/fleet/statuses/{sha}","languages_url":"https://api.github.com/repos/coreos/fleet/languages","stargazers_url":"https://api.github.com/repos/coreos/fleet/stargazers","contributors_url":"https://api.github.com/repos/coreos/fleet/contributors","subscribers_url":"https://api.github.com/repos/coreos/fleet/subscribers","subscription_url":"https://api.github.com/repos/coreos/fleet/subscription","commits_url":"https://api.github.com/repos/coreos/fleet/commits{/sha}","git_commits_url":"https://api.github.com/repos/coreos/fleet/git/commits{/sha}","comments_url":"https://api.github.com/repos/coreos/fleet/comments{/number}","issue_comment_url":"https://api.github.com/repos/coreos/fleet/issues/comments{/number}","contents_url":"https://api.github.com/repos/coreos/fleet/contents/{+path}","compare_url":"https://api.github.com/repos/coreos/fleet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/coreos/fleet/merges","archive_url":"https://api.github.com/repos/coreos/fleet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/coreos/fleet/downloads","issues_url":"https://api.github.com/repos/coreos/fleet/issues{/number}","pulls_url":"https://api.github.com/repos/coreos/fleet/pulls{/number}","milestones_url":"https://api.github.com/repos/coreos/fleet/milestones{/number}","notifications_url":"https://api.github.com/repos/coreos/fleet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/coreos/fleet/labels{/name}","releases_url":"https://api.github.com/repos/coreos/fleet/releases{/id}","deployments_url":"https://api.github.com/repos/coreos/fleet/deployments","created_at":"2013-10-11T23:56:38Z","updated_at":"2016-04-15T14:43:46Z","pushed_at":"2016-04-15T14:09:50Z","git_url":"git://github.com/coreos/fleet.git","ssh_url":"git@github.com:coreos/fleet.git","clone_url":"https://github.com/coreos/fleet.git","svn_url":"https://github.com/coreos/fleet","homepage":"","size":8481,"stargazers_count":2152,"watchers_count":2152,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":265,"mirror_url":null,"open_issues_count":163,"forks":265,"open_issues":163,"watchers":2152,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/coreos/fleet/pulls/1554"},"html":{"href":"https://github.com/coreos/fleet/pull/1554"},"issue":{"href":"https://api.github.com/repos/coreos/fleet/issues/1554"},"comments":{"href":"https://api.github.com/repos/coreos/fleet/issues/1554/comments"},"review_comments":{"href":"https://api.github.com/repos/coreos/fleet/pulls/1554/comments"},"review_comment":{"href":"https://api.github.com/repos/coreos/fleet/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/coreos/fleet/pulls/1554/commits"},"statuses":{"href":"https://api.github.com/repos/coreos/fleet/statuses/9b173e1fbf3a1bf46495849080a534c5e0c05464"}}}}