Closed cchance27 closed 6 years ago
E-t-z
commented
6 years ago Just threwing in my 2 cents.
For some deployments I would rather have "fixed" release without any daily.sh running, it would assure that I know exactly which version runs where and sometimes I would just not like to run "latest and greatest".
cchance27
commented
6 years ago thats already supported just set daily_enabled to false and you’ll stick to the release cadence from the docker release 1.43 now
jarischaefer
commented
6 years ago What do you mean by enabling daily updates? Did you set $config['update'] = 1;?. As far as I know, daily.sh does not update the LibreNMS version (e.g. 1.43 to master) by default.
cchance27
commented
6 years ago $config['update_channel'] = 'master' will have daily.sh switch to master
so technically ENV DAILY_ENABLE = true just keeps the branch up to date which is fine, by default, but i still think it should also the daily.sh should be run if ENV DAILY_ENABLE = true on startup to make sure the container joining is on the latest (in this case 1.43) version
This will also allow for those that want to switch to master branch and set our update channel in config file, that DAILY_ENABLE will allow daily.sh to do it's nightly updates, but also with the change I'm recommending will make sure newly deployed containers are also updated.
jarischaefer
commented
6 years ago to make sure the container joining is on the latest (in this case 1.43) version
If you want to guarantee the container is running 1.43, you should pick an appropriate tag (e.g. jarischaefer/docker-librenms:1.43-02).
stock 1.43
What is stock 1.43? There is only 1.43, which is tied to this commit.
will update to the latest version
If you want to run development versions, you should build your own version using docker build -t librenms:{hash} --build-arg LIBRENMS_VERSION=dev-master and deploy it on all the servers.
cchance27
commented
6 years ago well in that case i'm not sure what DAILY_ENABLE does in the environmental variables? lol, but instead of rebuilding for a specific version, right now i can tell you DAILY_ENABLE=true + manually setting the build to master in config works fine, as it should, its just a request here that daily_enable instead of just enabling the cron, to also run once on startup.
jarischaefer
commented
6 years ago well in that case i'm not sure what DAILY_ENABLE does in the environmental variables
It merely executes the current daily.sh, which is an implementation detail that is of no concern.
The implementation in 1.43 does the following:
Updating SQL-Schema OK
Cleaning up DB OK
This mimics librenms.nonroot.cron, where the script is always executed, but certain actions may be disabled by configuration (e.g. $config['update']).
right now i can tell you DAILY_ENABLE=true + manually setting the build to master in config works fine
No, it does not really work, since it breaks immutability and makes the container unpredictable, which means it would become impossible to guarantee what version will be run. Another concern is the possibility of newly required dependencies that would be required by the latest code but missing in the underlying baseimage. Ultimately, it is just a gamble.
Furthermore, a container should never update itself. This complexity belongs outside the container. There are many reasons for this and the benefits are somewhat similar to what can be observed in programming languages that encourage immutable data structures.
Here are some resources explaining this philosophy (I did not read them entirely):
Perhaps the most relevant paragraph:
The best way to work with Application Services in docker is to treat them more as an immutable disk image; there’s less need for Java code inside the process to mutate itself at runtime. Then the rolling upgrade of new versions of containers is done outside of the application server itself (e.g. via rolling upgrades in kubernetes and using a load balancer in front of the containers).
cchance27
commented
6 years ago immutability is a feature of docker it's not a requirement or else stateful containers wouldn't exist, to each their own, and in all honesty, it's a philosophical debate plenty of people use lxc and docker as testbed environments for testing versions and such, myself included. I've seen entire libraries even of people who have written docker containers with ansible inside that actually builds an entire app install off the latest versions of apps when deployed, is it what the primary usecase for docker is, no, is it a valid usecase that actually works well, yes surprisingly. Hell i know a few people in corporate environments that use docker as a pseudo VMs replacements.
I've found setting the librenms to master and enabling daily runs fine and we get an environment running on bleeding edge, and like i said it does in fact work pretty well on your current container, my only recommendation was to improve what already exists. (in fact main gripes are the previous tickets i opened which found a bug in master chain, and a bug in the docker container since 1.43, the logs directory, beyond that though the upgrade works smoothly, for me)
That said even without consideration for cutting over to master, running daily.sh on startup is still good as that way even if your on the 1.43 branch, the first run will ensure that the database schema is up to date and the other things daily.sh checks instead of at the cronjob run, that way when people switch to 1.44 build of your container the schema will get updated immediately instead of at 12am or whenever the daily.sh is set to.
jarischaefer
commented
6 years ago I usually update by calling the build-base.php script via setup_database (source) or waiting for daily.sh.
Automatic updates would be great, but I am concerned about concurrency. I do not know whether the concurrency-safe line (file locks are unsafe) is used in all multi-container setups. Once daily.sh is confirmed to be concurrency-safe, it could be added on startup by default.
In the meantime, I will add the requested environment variable, but you should be aware of the implications (that breaking immutability is not intended/supported) if you use it for LibreNMS upgrades.
cchance27
commented
6 years ago what additional environment variable, this issue was opened just as a recommendation to run daily.sh on boot if the daily run is enabled in the existing env variable
cchance27
commented
6 years ago the branch can be left as part of a custom config people would have to do like i did since its unsupported feature
jarischaefer
commented
6 years ago what additional environment variable, this issue was opened just as a recommendation to run daily.sh on boot if the daily run is enabled in the existing env variable
As I explained, enabling it by default could be dangerous. Therefore, another environment variable is required.
Once daily.sh is confirmed to be concurrency-safe, it could be added on startup by default.
There are two steps in daily.sh:
jarischaefer
commented
6 years ago The changes are available in the latest build.
cchance27
commented
6 years ago cool will give it a go when i get some free time.
For consistency in multi-container setups, I believe it would be a good change, that if on initial spawn with daily updates enabled after the initial spinup process of the container, to run a daily.sh to pull the latest update.
The reason for this is if I've got an instance of 1.43 running with daily updates enabled via env, then i'll have a nice up to date version running, but if I add additional instances to the cluster, those will start up with the stock 1.43, and at midnight or whatever cron time, will update to the latest version.
With the adjustment, I'm suggesting as soon as the pod spawns it would update to the latest version, i realize this doesn't 100% solve the irregularity as technically the new pod could be newer than the older pod, but at least then killing them all and respawning them all would see them operating on the latest version and not falling back to an old version (had this issue when i had health checks set in k8s, and it killed some instances and when it came back up was on an older version than it had been originally been running)