Feature: increase/decrease logging level without restarting #748

HuKeping · 2016-05-14T12:31:21Z

Part of #633

As discussed with the team of notary, it might not make sense to completely reload the configuration yet.

With this patch set, we can send SIGUSR1/SIGUSR2 to increase/decrease the logging level of notary-server.

Signed-off-by: Hu Keping hukeping@huawei.com

docker-jenkins · 2016-05-14T12:31:22Z

Can one of the admins verify this patch?

HuKeping · 2016-05-14T14:52:56Z

cmd/notary-server/config.go

+		switch sig {
+		case syscall.SIGUSR1:
+			if err := utils.AdjustLogLevel(true); err != nil {
+				fmt.Println("Increase log level failed, will use the old one, error: ", err)


Why using fmt.Print not logrus here and below is because it will not log out anything if the log level is too low, Panic for example and we can't just use logrus.Panicf() for it will terminate the whole process.

Sounds good, could we also print the log level we're reverting back to? For example: Attempt to increase log level failed, will remain at %s level, error: (with the old level filling that string). logrus has a GetLevel function we can use

cyli · 2016-05-20T16:21:56Z

jenkins, test this please

NathanMcCauley · 2016-05-20T17:29:12Z

This functionality should be covered somewhere in the docs, so folks can know about it when reading through documentation.

NathanMcCauley · 2016-05-20T17:44:36Z

cmd/notary-server/config.go

+// reloadConfig was designed to do all the dirty work that needed to reload
+// the config for notary-server and make the entrance-ReloadConfiguration()
+// for hot configuration reload do some generic things.
+func reloadConfig(sig os.Signal, configFile string, oldConfig *viper.Viper, envPrefix string) {


This function doesn't seem to have any test coverage. @HuKeping could you add some tests for this?

HuKeping · 2016-05-21T18:17:36Z

@NathanMcCauley Will add the docs later

cyli · 2016-05-23T18:38:52Z

jenkins, test this please

HuKeping · 2016-05-30T11:34:35Z

docs and test ready @NathanMcCauley

HuKeping · 2016-06-03T16:29:45Z

ping ...

riyazdf · 2016-06-06T16:30:15Z

jenkins, test this please.

riyazdf · 2016-06-07T01:08:00Z

cmd/notary-server/main_test.go

+
+	v := viper.New()
+	utils.SetupViper(v, "envPrefix")
+	//v.SetConfigType("json")


can we delete this line?

HuKeping · 2016-06-11T05:48:36Z

Drop the code for reloading the whole config file, this patch set plans only to support increase/decrease logging level.

riyazdf · 2016-06-13T16:12:23Z

docs/reference/server-config.md

@@ -359,6 +361,29 @@ Example:
 	</tr>
 </table>

+## Hot logging level reload
+We don't support to completely reload the whole configuration yet at present. What we support for now is:


Can we change this first sentence to: "We don't support completely reloading notary configuration files yet at present."

riyazdf · 2016-06-13T16:17:03Z

jenkins, test this please

riyazdf · 2016-06-13T16:18:20Z

cmd/notary-server/config.go

@@ -225,6 +228,7 @@ func parseServerConfig(configFilePath string, hRegister healthRegister) (context
 	}

 	ctx := context.Background()
+	ctx = context.WithValue(ctx, "config", config)


I don't think we need this anymore for the current functionality

Oh I missed this one, will clean it up.

HuKeping · 2016-06-14T13:23:30Z

updated.

HuKeping · 2016-06-15T00:03:57Z

Some error on CI system

HuKeping · 2016-06-15T02:54:42Z

rebased

cyli · 2016-06-15T02:55:10Z

jenkins, test this please

Signed-off-by: Hu Keping <hukeping@huawei.com>

…o handle Signed-off-by: Hu Keping <hukeping@huawei.com>

… server This patch will allow people to: - use SIGUSR1 to increment the logging level - use SIGUSR2 to decrement the logging level And it will allow the logging level to be turned up or down as much as desired by sending the appropriate signal multiple times. Signed-off-by: Hu Keping <hukeping@huawei.com>

Signed-off-by: Hu Keping <hukeping@huawei.com>

This patch will allow people to use SIGHUP to tell notary server to reload the configuration file. For now it only supports to reload the logging level. Signed-off-by: Hu Keping <hukeping@huawei.com>

From NathanMcCauley's review Signed-off-by: Hu Keping <hukeping@huawei.com>

Signed-off-by: Hu Keping <hukeping@huawei.com>

From riyazdf's comments. Signed-off-by: Hu Keping <hukeping@huawei.com>

After disscussing with the team, we all agreed that it might not make sense to completely reload the whole config file yet at present. Instead, a graceful shutdown would be a better choice. Signed-off-by: Hu Keping <hukeping@huawei.com>

Fix typo , update docs and remove some unused code. From riyazdf's comment. Signed-off-by: Hu Keping <hukeping@huawei.com>

Otherwise it will not print anything until the flag for flush is coming. Signed-off-by: Hu Keping <hukeping@huawei.com>

About how to increase/decrease logging level via system signal. Signed-off-by: Hu Keping <hukeping@huawei.com>

HuKeping · 2016-06-16T00:28:27Z

ping ...

cyli · 2016-06-16T17:25:12Z

Sorry for the lack of response @HuKeping. We are all prepping for DockerCon. :) This LGTM, thank you so much for your incredible patience and all your hard work on this feature!

riyazdf · 2016-06-16T17:28:11Z

@HuKeping thank you for reworking this PR (code and docs) and your patience! LGTM 👍

GordonTheTurtle added status/0-triage labels May 14, 2016

HuKeping reviewed May 14, 2016
View reviewed changes

HuKeping changed the title ~~[WIP] Feature: hot configuration reload for notary server~~ Feature: hot configuration reload for notary server May 19, 2016

NathanMcCauley reviewed May 20, 2016
View reviewed changes

riyazdf reviewed Jun 7, 2016
View reviewed changes

HuKeping changed the title ~~Feature: hot configuration reload for notary server~~ Feature: increase/decrease logging level without restarting Jun 11, 2016

riyazdf reviewed Jun 13, 2016
View reviewed changes

HuKeping added 13 commits June 15, 2016 18:42

[PATCH 1/8] Add a helper function to increase/decrease log level

9337c34

Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 2/8] Add test for the helper function

9b096b7

Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 3/8] Add two constants to describe the signals we would like t…

1110a5e

…o handle Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 5/8] Introduce a signal constant to reload the configuration.

d2149d8

Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 6/8] Feature: hot configuration reload for notary server

229cf08

This patch will allow people to use SIGHUP to tell notary server to reload the configuration file. For now it only supports to reload the logging level. Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 7/8] Add test for reloadConfig

68cb2f3

From NathanMcCauley's review Signed-off-by: Hu Keping <hukeping@huawei.com>

[PATCH 8/8] Add docs for the hot configuration reload

eb8b53b

Signed-off-by: Hu Keping <hukeping@huawei.com>

Fix error message when failed to reload logging level

784eb37

From riyazdf's comments. Signed-off-by: Hu Keping <hukeping@huawei.com>

Clean up

4d1a985

Fix typo , update docs and remove some unused code. From riyazdf's comment. Signed-off-by: Hu Keping <hukeping@huawei.com>

Add newline to flush the stdout

783a7e2

Otherwise it will not print anything until the flag for flush is coming. Signed-off-by: Hu Keping <hukeping@huawei.com>

Add some user guide for notary-server

0119f5b

About how to increase/decrease logging level via system signal. Signed-off-by: Hu Keping <hukeping@huawei.com>

riyazdf merged commit d1e910b into notaryproject:master Jun 16, 2016

HuKeping deleted the feature-hot-reload branch June 17, 2016 00:51

HuKeping mentioned this pull request Jun 17, 2016

Increase/decrease logging level on notary-signer #791

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: increase/decrease logging level without restarting #748

Feature: increase/decrease logging level without restarting #748

HuKeping commented May 14, 2016 •

edited

Loading

docker-jenkins commented May 14, 2016

HuKeping May 14, 2016 •

edited

Loading

riyazdf Jun 6, 2016

HuKeping Jun 9, 2016

cyli commented May 20, 2016

NathanMcCauley commented May 20, 2016

NathanMcCauley May 20, 2016

HuKeping May 21, 2016

HuKeping commented May 21, 2016

cyli commented May 23, 2016

HuKeping commented May 30, 2016

HuKeping commented Jun 3, 2016

riyazdf commented Jun 6, 2016

riyazdf Jun 7, 2016

HuKeping commented Jun 11, 2016

riyazdf Jun 13, 2016 •

edited

Loading

riyazdf commented Jun 13, 2016

riyazdf Jun 13, 2016

HuKeping Jun 13, 2016

HuKeping commented Jun 14, 2016

HuKeping commented Jun 15, 2016

HuKeping commented Jun 15, 2016

cyli commented Jun 15, 2016

HuKeping commented Jun 16, 2016

cyli commented Jun 16, 2016

riyazdf commented Jun 16, 2016 •

edited

Loading

Feature: increase/decrease logging level without restarting #748

Feature: increase/decrease logging level without restarting #748

Conversation

HuKeping commented May 14, 2016 • edited Loading

docker-jenkins commented May 14, 2016

HuKeping May 14, 2016 • edited Loading

Choose a reason for hiding this comment

riyazdf Jun 6, 2016

Choose a reason for hiding this comment

HuKeping Jun 9, 2016

Choose a reason for hiding this comment

cyli commented May 20, 2016

NathanMcCauley commented May 20, 2016

NathanMcCauley May 20, 2016

Choose a reason for hiding this comment

HuKeping May 21, 2016

Choose a reason for hiding this comment

HuKeping commented May 21, 2016

cyli commented May 23, 2016

HuKeping commented May 30, 2016

HuKeping commented Jun 3, 2016

riyazdf commented Jun 6, 2016

riyazdf Jun 7, 2016

Choose a reason for hiding this comment

HuKeping commented Jun 11, 2016

riyazdf Jun 13, 2016 • edited Loading

Choose a reason for hiding this comment

riyazdf commented Jun 13, 2016

riyazdf Jun 13, 2016

Choose a reason for hiding this comment

HuKeping Jun 13, 2016

Choose a reason for hiding this comment

HuKeping commented Jun 14, 2016

HuKeping commented Jun 15, 2016

HuKeping commented Jun 15, 2016

cyli commented Jun 15, 2016

HuKeping commented Jun 16, 2016

cyli commented Jun 16, 2016

riyazdf commented Jun 16, 2016 • edited Loading

HuKeping commented May 14, 2016 •

edited

Loading

HuKeping May 14, 2016 •

edited

Loading

riyazdf Jun 13, 2016 •

edited

Loading

riyazdf commented Jun 16, 2016 •

edited

Loading