Log flag and logs refactoring

New Pull Request

Conversation 0 Commits 26 Files Changed 34

There is no content yet.

jonathan requested changes 4 years ago

Overall looks very good. Just a few suggestions and one big question -- how do the supervisord changes take effect?

.gitignore

												
			@@ -1,7 +1,6 @@
		
			*.iml
		
			.idea
		
			tmp
		
			logs

jonathan commented 4 years ago

why remove this from .gitignore?

kris commented 4 years ago

This shouldn’t be created any more - the only files written in here are 2 bubble and 2 nodemanager logs. Removing it here would be another confirmation to me that I fully removed the usage of this folder.

bubble-server/src/main/java/bubble/ApiConstants.java

												
			@@ -201,6 +201,7 @@ public class ApiConstants {
		
			    public static final String EP_FORK = "/fork";
		
			    public static final String EP_NODE_MANAGER = "/nodeman";
		
			    public static final String EP_UPGRADE = "/upgrade";
		
			    public static final String EP_LOG_FLAG = "/logFlag";

jonathan commented 4 years ago

I’d prefer the endpoint to simple by called /logs (and the constant named EP_LOGS) -- because in the long run, we may offer more fine-grained configuration than just on/off. We may also offer GET endpoints to read log data.

bubble-server/src/main/java/bubble/resources/cloud/NetworksResource.java

												
			@@ -220,0 +232,4 @@
		
			        selfNodeService.setLogFlag(flag);
		
			        return ok();
		
			    }

jonathan commented 4 years ago

I’d prefer to add a LogsResource class, and reference it here via @Path(EP_LOGS) to delegate everything under /logs to LogsResource.

kris commented 4 years ago

Added, but also moved to a most proper place - NodesResource - as these calls doesn’t actually require network UUID. Then don’t require node UUID either, so the final URL doesn’t contain any UUID. But that seems like more proper place as this log flag is node’s property.

bubble-server/src/main/resources/packer/roles/bubble/files/log_manager.sh

												
			@@ -0,0 +66,4 @@
		
			    setLoggingForSupervisorConfig cfgFile REDIS_LOG_FLAG
		
			  done
		
			fi

jonathan commented 4 years ago

Do we need to supervisorctl reload to make sure the new log locations are used by the various processes that supervisord starts? Where is that happening?

kris commented 4 years ago

👍 Forgot to add that, sorry. And that is the answer to the question above `how do the supervisord changes take effect?

kris changed title from ~~WIP: (testing) Log flag and logs refactoring~~ to Log flag and logs refactoring 4 years ago

kris changed title from ~~Log flag and logs refactoring~~ to WIP: (re-implementing) Log flag and logs refactoring 4 years ago

kris changed title from ~~WIP: (re-implementing) Log flag and logs refactoring~~ to Log flag and logs refactoring 4 years ago

jonathan referenced this issue from a commit 4 years ago

Log flag and logs refactoring (#33) Use get_... and set_plaintext for log flag in redis Use links to /dev/null when logs are turned off Merge branch 'master' into kris/log_flag # Conflicts: # bubble-server/src/main/resources/packer/roles/nginx/files/init_dhparams.sh Add missing GET annotation on log flag status API call Remove encryption of log flag in redis Simplify geter and setter of log flag Fix log manager's tests and text replacements Fix log manager script Fix log manager script Move log manager cron to root user Move one more log into correct bubble folder Reload supervisor after changes in logs setup Remove another host prefix Add new subresource class for logs Create bubble user, bubble-log group and log folder earlier Set log flag initial value Fix not implemented error Add root user to the new group Fix user for newly created cron Move all log output to /var/log/bubble/ Merge branch 'master' into kris/log_flag Create a cron for log_manager in packer's ansible Add script for setting up logs as per log flag Add API calls to read and set log flag Remove not used file Use same log files' naming for tmp folder's logs Co-authored-by: Kristijan Mitrovic <kmitrovic@itekako.com> Reviewed-on: https://git.bubblev.org/bubblev/bubble/pulls/33

jonathan closed this pull request 4 years ago

jonathan deleted branch kris/log_flag 4 years ago

Reviewers

jonathan requested changes 4 years ago

The pull request has been merged as d5d2bb508a.

Sign in to join this conversation.

jonathan

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

#33 Log flag and logs refactoring

Reviewers