CLOUDSTACK-9039: Fix paths for logging Ubuntu Management#1039
Conversation
7ae487f to
c745427
Compare
There was a problem hiding this comment.
Perhaps use only instead of 2 lines to create the tree
mkdir -p /var/log/cloudstack/management/
There was a problem hiding this comment.
I agree with @milamberspace .
btw, what about the /var/log/cloudstack/usage for usage server?
|
@ustcweizhou This is only for the management server! The usage server will create it's own paths when installed. @milamberspace I'll push that change, thanks! |
c745427 to
e141287
Compare
There was a problem hiding this comment.
you could remove line 110
There was a problem hiding this comment.
Both lines can be removed. The packaging already creates them.
|
@borisroman LGTM if remove line 110. |
|
FYI: Started tests on this branch. |
e141287 to
478a0bd
Compare
|
@remibergsma @ustcweizhou As @wido pointed out, they are already created through the packager. https://github.com/apache/cloudstack/blob/master/debian/rules#L61 |
|
@wido @ustcweizhou Could you review again please? |
There was a problem hiding this comment.
Does this need to be hard-coded? I had a quick look into the python logging and am hoping we can retrieve the path from there. just a thought
There was a problem hiding this comment.
@DaanHoogland You have a valid point, but this is been done throughout all this Python code. Let's fix it later in one go.
|
@wido @ustcweizhou Can you look again here please, see @borisroman's comments. |
There was a problem hiding this comment.
@borisroman Did you see this remark in the code? Is that about the line you remove here? I have no clue what sl 6.1 is.. Seems like a RHEL like release? Scientific Linux? I dunno! Does it hurt to have the dir? We better remove it later when master is unfrozen IMHO as we do not have a change to test this properly otherwise.
There was a problem hiding this comment.
It looks like a xen release!
There was a problem hiding this comment.
@DaanHoogland No, this should be an OS capable of running the management server.
There was a problem hiding this comment.
@remibergsma Could you point me to a couple lines of code where /var/log/cloudstack-management/ is used in current master?
There was a problem hiding this comment.
@remibergsma Grepping yields:
grep "/var/log/cloudstack-management/" . -r
./python/lib/cloudutils/serviceConfigServer.py: bash("mkdir /var/log/cloudstack-management/")
There was a problem hiding this comment.
ok, @remibergsma either slackware or suse linux then. sl instead of s1 (which I read)
478a0bd to
acd49d8
Compare
|
OK, now these became a textual change in Python. LGTM. |
CLOUDSTACK-9039: Fix paths for logging Ubuntu ManagementFix paths for logging Ubuntu Management. How to test: Install via DEB packages... * pr/1039: CLOUDSTACK-9039: Fix paths for logging Ubuntu Management. Signed-off-by: Remi Bergsma <github@remi.nl>
Fix paths for logging Ubuntu Management.
How to test:
Install via DEB packages...