Skip to content

new script ct/icinga - #1343

Closed
chrnie wants to merge 17 commits into
community-scripts:mainfrom
chrnie:feature/icinga_pr
Closed

new script ct/icinga#1343
chrnie wants to merge 17 commits into
community-scripts:mainfrom
chrnie:feature/icinga_pr

Conversation

@chrnie

@chrnie chrnie commented Jan 27, 2026

Copy link
Copy Markdown

✍️ Description

New Script for icinga

This will install a LXC container (debian) with

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected.
  • No breaking changes – Existing functionality remains intact.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to website-related JSON files or metadata.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.

🔍 Code & Security Review (X in brackets)

  • Follows Code_Audit.md & CONTRIBUTING.md guidelines
  • Uses correct script structure (AppName.sh, AppName-install.sh, AppName.json)
  • No hardcoded credentials

📋 Additional Information (optional)


📦 Application Requirements (for new scripts)

Required for 🆕 New script submissions.
Pull requests that do not meet these requirements may be closed without review.

  • The application is at least 6 months old
  • The application is actively maintained
  • The application has 600+ GitHub stars
  • Official release tarballs are published

https://github.com/icinga/icinga2

@greptile-apps

greptile-apps Bot commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

This PR adds a new Icinga2 monitoring system container setup with comprehensive installation including Icinga2 Core, IcingaDB, Icinga Web 2, and numerous pre-configured modules (Director, x509, Reporting, Notifications, Businessprocess). The implementation installs MariaDB and Apache locally, sets up multiple databases with generated passwords, and includes Linuxfabrik monitoring plugins for advanced monitoring scenarios.

Key Changes:

  • Added container setup script ct/icinga.sh with privileged container requirement for ping checks
  • Created comprehensive installation script install/icinga-install.sh with multi-stage setup for Icinga ecosystem
  • Added JSON metadata file with proper schema compliance
  • Includes optional InfluxDB integration for performance data and x509 certificate scanning

Issues Found:

  • Multiple style violations: uses git clone instead of the recommended fetch_and_deploy_gh_release function for stability
  • Uses deprecated apt-get instead of apt command
  • Incorrect license URL path (/ProxmoxVE/ should be /ProxmoxVED/)
  • Contains non-header comments in ct/icinga.sh which violate project standards
  • Unreachable code at line 310 in the installation script

Confidence Score: 3/5

  • This PR is functional but requires addressing multiple style violations and one logic issue before merging
  • The implementation is comprehensive and functionally sound, but has multiple violations of project coding standards (git clone usage, apt-get vs apt, improper comments, incorrect URLs) and contains unreachable code that should be fixed. These issues don't prevent the script from working but violate established best practices
  • Pay close attention to install/icinga-install.sh for the multiple git clone usages, unreachable break statement, and service enable pattern; and ct/icinga.sh for the non-header comments and license URL

Important Files Changed

Filename Overview
ct/icinga.sh Container setup script with minor style issues - incorrect license URL path and comments that should be removed
install/icinga-install.sh Complex installation script with multiple style violations - uses git clone instead of fetch_and_deploy_gh_release, incorrect license URL, uses apt-get instead of apt, has unreachable code in line 310
frontend/public/json/icinga.json Well-structured JSON metadata following all required schema fields with correct logo URL from selfh.st/icons CDN

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

10 files reviewed, 10 comments

Edit Code Review Agent Settings | Greptile

Comment thread ct/icinga.sh Outdated
Comment thread ct/icinga.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
Comment thread install/icinga-install.sh Outdated
chrnie and others added 5 commits January 27, 2026 17:16
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

@chrnie chrnie left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed all known problems from greptile.

A working fork can be used from here:
bash -c "$(curl -fsSL https://raw.githubusercontent.com/chrnie/ProxmoxVED/refs/heads/icingawrk/ct/icinga.sh)"

Comment thread install/icinga-install.sh
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh Outdated
Comment thread ct/headers/icinga Outdated
Comment thread ct/icinga.sh Outdated
Comment thread ct/icinga.sh Outdated
Comment thread install/icinga-install.sh
Comment on lines +28 to +41
msg_info "Adding Netways extras and plugins repository"
setup_deb822_repo \
"netways-extras" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/extras/debian/" \
"${VERSION_CODENAME}" \
"main"
setup_deb822_repo \
"netways-plugins" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/plugins/debian/" \
"${VERSION_CODENAME}" \
"main"
msg_ok "Set up Netways Repositories"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
msg_info "Adding Netways extras and plugins repository"
setup_deb822_repo \
"netways-extras" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/extras/debian/" \
"${VERSION_CODENAME}" \
"main"
setup_deb822_repo \
"netways-plugins" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/plugins/debian/" \
"${VERSION_CODENAME}" \
"main"
msg_ok "Set up Netways Repositories"
setup_deb822_repo \
"netways-extras" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/extras/debian/" \
"${VERSION_CODENAME}" \
"main"
setup_deb822_repo \
"netways-plugins" \
"https://packages.netways.de/netways-repo.asc" \
"https://packages.netways.de/plugins/debian/" \
"${VERSION_CODENAME}" \
"main"

I think we have some code in tools.func for handling things like version codename etc, so no need to source them, right @MickLesk ?

Comment thread install/icinga-install.sh
Comment on lines +19 to +26
msg_info "Setting up Icinga Repository"
setup_deb822_repo \
"icinga-stable" \
"https://packages.icinga.com/icinga.key" \
"https://packages.icinga.com/debian/" \
"icinga-${VERSION_CODENAME}" \
"main"
msg_ok "Set up Icinga Repository"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
msg_info "Setting up Icinga Repository"
setup_deb822_repo \
"icinga-stable" \
"https://packages.icinga.com/icinga.key" \
"https://packages.icinga.com/debian/" \
"icinga-${VERSION_CODENAME}" \
"main"
msg_ok "Set up Icinga Repository"
setup_deb822_repo \
"icinga-stable" \
"https://packages.icinga.com/icinga.key" \
"https://packages.icinga.com/debian/" \
"icinga-${VERSION_CODENAME}" \
"main"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

get_os

Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
Comment on lines +434 to +435
fetch_and_deploy_gh_release "pve" "nbuchwitz/icingaweb2-module-pve" "tarball" "latest" "/usr/share/icingaweb2/modules/pve"
fetch_and_deploy_gh_release "check_pve" "nbuchwitz/check_pve"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

those not inside an msg block

Comment thread install/icinga-install.sh Outdated
Comment thread install/icinga-install.sh
MARIADB_DB_NAME="reporting" MARIADB_DB_USER="reporting" setup_mariadb_db
REPORTING_DB_PW=$MARIADB_DB_PASS
cat <<EOF | mariadb || { msg_error "Failed to alter databases"; exit 1; }
ALTER DATABASE director CHARACTER SET utf8 COLLATE utf8_unicode_ci;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@github-actions

github-actions Bot commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

This PR has been marked as stale. It will be closed if no new commits are added in 7 days.

@github-actions github-actions Bot closed this Feb 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@chrnie Closing stale PR due to inactivity (no commits for 7 days after stale label).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants