-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CLOUDSTACK-10311 Agent Log Rotate variable replace bug #2471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLOUDSTACK-10311 Agent Log Rotate variable replace bug #2471
Conversation
rafaelweingartner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I restarted the Travis build to see if the errors were due to code or some other environment problem.
yadvr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly like #2429
borisstoyanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@blueorangutan package |
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1757 |
|
@Slair1 can you take a look into travis failures? |
|
Since this is the same as #2429, should we just back-merge that? |
|
Can you check the build failures @Slair1 ? (See travis for details, or confirm you're able to build without failures?). @blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1779 |
|
Travis errors look unrelated? |
|
Can you rebase? I have fixed a code that was causing travis errors. |
48d09e1 to
2d0dea4
Compare
|
@rafaelweingartner sure, just did |
2d0dea4 to
e7a73f0
Compare
|
@rafaelweingartner still seeing some unrelated errors in the travis |
|
Hmmm, may I ask you why did you open the PR against 4.9? |
|
@rafaelweingartner as said in the other comments, there is already a PR that addresses this issue in future versions. We are running 4.9.3 right now in our environment. |
|
@Slair1 the changes are already in 4.11, master branches. By future version, do you expect the fix to be in a future 4.9.x version (likely 4.9.4.0)? |
|
Sorry for asking again, I forgot. |
|
@rafaelweingartner yep, i was hoping it would be included in 4.9.4.0 |
|
@rafaelweingartner how do i increase the travis timeouts? |
|
Timeouts can be changed in |
|
@rafaelweingartner thanks for info on increasing the travis timeout. I guess i thought there would be a 4.9.4.0, there have been other PRs merged in 4.9 and the POMs for it have been updated to 4.9.4.0 it looks like at least. |
|
@rafaelweingartner sorry for the newbie question, but i updated the .travis.yml file and it now shows up in my commit, is that correct? |
|
Same as referenced PR in one of the comments, I'll merge this. However, on 4.9 and olders branches we no longer support Travis. |
PR2094 made the log path a variable. The cloudstack-agent was modified to use the @agentlog@ variable entry, but the pom.xml file was not correct to do the replacement of the @agentlog@.
Line 62 of the pom.xml handles the replacement with the rename in place