Skip to content
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

Update build_and_push.sh #5

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

uhlhosting
Copy link

Bump netdata

@uhlhosting uhlhosting closed this Sep 2, 2023
@uhlhosting uhlhosting reopened this Sep 2, 2023
Copy link
Author

@uhlhosting uhlhosting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made all updates and fixes

@githubsaturn
Copy link
Contributor

githubsaturn commented Sep 3, 2023

Thanks for the PR! Have you tested the environment variables to see if they still work in this new version? If I recall correctly, netdata has added some extra environment variables for notifications that were not present in the older versions.

PS: I'm referring to the environment variables used for netdata alert notifications. e.g. TELEGRAM_BOT_TOKEN and TELEGRAM_CHAT_ID

@uhlhosting
Copy link
Author

@githubsaturn yes the last push should be passing the builds see here: https://github.com/uhlhosting/netdata-docker/actions/runs/6063839503

@uhlhosting
Copy link
Author

@githubsaturn let me check the env variables changes, thats one thing I had not managed to look into, I just managed to finish the fixes and have a complete build.

@uhlhosting
Copy link
Author

uhlhosting commented Sep 3, 2023

@githubsaturn I think is best you take a look, and consider what else would like to add, https://github.com/netdata/netdata/tree/master/health/notifications there are so many options.

IE:

#------------------------------------------------------------------------------
# telegram (telegram.org) global notification options

SEND_TELEGRAM="YES"
TELEGRAM_BOT_TOKEN="111122223:7OpFlFFRzRBbrUUmIjj5HF9Ox2pYJZy5"
DEFAULT_RECIPIENT_TELEGRAM="-100233335555"

@uhlhosting
Copy link
Author

image

@uhlhosting
Copy link
Author

I don't get it, how it got 46 pulls in 30 mins 🗡️

@uhlhosting
Copy link
Author

need to look into solving these:

root@pac:/home/pac# docker logs d0bdd9e2665c
cp: cannot stat '/etc/netdata/override/*': No such file or directory
/usr/sbin/netdata: error while loading shared libraries: libelf.so.1: cannot open shared object file: No such file or directory

@githubsaturn
Copy link
Contributor

Here is the full list of env variables:
https://github.com/caprover/caprover/blob/80cb3a3a90ba5840c020a7e3ddaf00cc19b239b4/src/user/system/CaptainManager.ts#L564-L651

By just having a glance, I think your update breaks these notifications. For example SEND_TELEGRAM wasn't previously needed for telegram notifications.

@uhlhosting
Copy link
Author

@githubsaturn yes seems they added some new variables, anyhow, this last push allows to build the docker images without errors and they start, Ideal would be if you can also check the above link provided to the new variables, and see how we can work around them. Also the initial image used for this repo, was archived in 2020, there are better building procedures available at the netdata repo, would just require you to do a quick look over them, and pick the one needed for caprover.

@uhlhosting
Copy link
Author

An example of a working configuration would be:

#------------------------------------------------------------------------------
# pushover (pushover.net) global notification options

SEND_PUSHOVER="YES"
PUSHOVER_APP_TOKEN="XXXXXXXXX"
DEFAULT_RECIPIENT_PUSHOVER="USERTOKEN"

Seems they added a switch for all notifications, i believe it makes sense, hence one can configure more and only the switch enable / disable .

SO this would require maybe some minor updates. Still the best would be to completely change the way images are build, since like I said they provide better ways here : https://github.com/netdata/netdata/tree/master/packaging

@githubsaturn
Copy link
Contributor

Exactly, please feel free to update the env vars on main repo to align with these new variables, and we can merge this pull request.

@uhlhosting
Copy link
Author

Made caprover/caprover#1868 . Just need to check now on the build variables. I also introduced Pushover support.

@uhlhosting
Copy link
Author

root@pac:/home/pac# docker logs 225ff104957f
cp: cannot stat '/etc/netdata/override/*': No such file or directory
2023-09-03 20:21:58: netdata ERROR : MAIN : close_range() failed, will try to close fds one by one (errno 1, Operation not permitted)
2023-09-03 20:21:58: netdata INFO  : MAIN : CONFIG: cannot load cloud config '/var/lib/netdata/cloud.d/cloud.conf'. Running with internal defaults.
2023-09-03 20:21:58: netdata INFO  : MAIN : Found 0 legacy dbengines, setting multidb diskspace to 256MB
2023-09-03 20:21:58: netdata INFO  : MAIN : Created file '/var/lib/netdata/dbengine_multihost_size' to store the computed value
2023-09-03 20:21:58: netdata ERROR : MAIN : Ignoring host prefix '/host': path '/host' failed to stat() (errno 2, No such file or directory)

We need to find how to fix these. I believe is best to use the build procedures from netdata. Since the template for this build is too old.

I tried my best to addapt, try to merge this branch, we can see if the build works, if not we will have to find workaround.

All last builds pass fine on my side.

@githubsaturn
Copy link
Contributor

I believe is best to use the build procedures from netdata

I agree! I'm totally up for using their official image, the on difference is to make sure that we can translate the env vars to the config files that they have. This should be a fairly easy thing to change.

I had previously asked them to add this functionality to the official image, but they rejected the proposal.
netdata/netdata#5782

@uhlhosting
Copy link
Author

Well the person who closed the issues closed it with these remarks.

Closing this feature request. We will re-evaluate the request internally

I dont see it fully as rejected maybe we need to knock more on that door.

@drmrbrewer
Copy link

Would be great if the latest version of NetData could be used in CapRover... apparently the dashboard and charts have been given a complete refresh even since v1.34.1.

@uhlhosting
Copy link
Author

Thats what we try to do here. Just that my time for this is limited. If more people would get involved could move faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants