Restructured daily.sh #2487

Merged
merged 2 commits into from Dec 5, 2015

Projects

None yet

5 participants

@f0o
Member
f0o commented Nov 22, 2015

Added staged update and check for distributed poller setups
Added release update channel

Fixes #2485 #2484

f0o added some commits Nov 22, 2015
@f0o f0o Restructured daily.sh
Added staged update and check for distributed poller setups
45b6a4c
@f0o f0o Added master and release update-channels
b2034cb
@f0o f0o added the Core label Nov 22, 2015
@laf
Member
laf commented Nov 24, 2015

👍 Tested (limited as no schema updates or logs to clear) but the code looks like it's ok.

@librenms/reviewers Can we get some more testing done please as if this isn't right then it will break auto updates which we've seen before :(

@f0o
Member
f0o commented Nov 25, 2015

Test test test :)
Also test the new config option to update between tags!
And check the new log's output!

I did test it all before submitting but you never know if some setup causes a bug or not ;)

@Rosiak
Contributor
Rosiak commented Nov 26, 2015

Issues could appear when messing this new daily.log and .git/ directory (Permissions).


bash-4.1$ ./daily.sh 
Updating to latest codebase                       ./daily.sh: line 25: logs/daily.log: Permission denied
./daily.sh: line 26: logs/daily.log: Permission denied

error: cannot open .git/FETCH_HEAD: Permission denied

Updating to latest codebase
You are not currently on a branch, so I cannot use any
'branch.<branchname>.merge' in your configuration file.
Please specify which remote branch you want to use on the command
line and try again (e.g. 'git pull <repository> <refspec>').
See git-pull(1) for details.
Returned: 1

I'm very well aware that these issues are caused by myself, but we still do need to be aware of the possibility. I'm a bit worried about this one, we need to be very careful as this could break installs.

Other than that I didn't have any issues after fixing permissions and git stuff! 👍

@f0o
Member
f0o commented Nov 27, 2015

The .git perms would've always caused an issue fyi

@f0o f0o referenced this pull request Nov 28, 2015
Closed

Mib split #1831

@laf
Member
laf commented Nov 28, 2015

👍 proposing to merge.

@f0o
Member
f0o commented Dec 2, 2015
@SaaldjorMike
Member

As mentioned on IRC I've tested this on my installs, looks good. As for the .git permissions, I guess we could add extra checks in another PR to check if permissions are correct.

@f0o
Member
f0o commented Dec 2, 2015

@SaaldjorMike that's likely best done in the validate.php

@SaaldjorMike
Member

@f0o Might be. I was just thinking of a way to notify people if they mess up permissions after installing, as this would be output from cron running.

@f0o
Member
f0o commented Dec 2, 2015

@SaaldjorMike still this perms issue is not part of the PR and isnt related to this PR either :S

@SaaldjorMike
Member

@f0o Agreed. As for this PR 👍

@laf
Member
laf commented Dec 2, 2015

./validate already takes care of permission checks if the user has provided the user they run cron as (default for a while now).

We've had 3 +1's for merges so I'll propose we merge in the next 12 hours

@f0o
Member
f0o commented Dec 3, 2015

Bump :)

@laf
Member
laf commented Dec 3, 2015

Sorry fella. Will merge tomorrow when I'm around in case of issues

@laf laf merged commit 2728bca into librenms:master Dec 5, 2015

3 checks passed

Auto-Deploy Build finished. No test results found.
Details
Scrutinizer Analysis No new issues
Details
Scrutinizer Tests Tests are not configured
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment