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

Implementation of APCUPSD Addon #372

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

Conversation

danimbrogno
Copy link

This PR contains changes to the smartnode to implement the APCUPSD addon proposed in GA022303.

When a user has metrics enabled, this addon allows a node operator to:

  1. Optionally run apcupsd within a container and allow it to connect to your ups
  2. Run an apcupsd-prometheus exporter container and connect it to apcupsd (either running in a container as mentioned above, or connected to your host machine or network)

This PR must be accepted alongside the PR to the smartnode-install repository.

)

// Constants
const (
Copy link
Author

Choose a reason for hiding this comment

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

These docker images are provided by third parties.
Please let me know if in your estimation there is a security risk here and if these images need to be audited.


// Get the parameters
enabledParam := configPage.addon.GetEnabledParameter()
// TODO: Don't like how I reference these by index here. Is there a better way?
Copy link
Author

Choose a reason for hiding this comment

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

Any suggestion for a cleaner way to fetch these parameters? It seems I can only access functions as defined by the generic addon interface.

configPage.handleDraw()
}

func (configPage *AddonApcupsdPage) handleDraw() {
Copy link
Author

Choose a reason for hiding this comment

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

Please review my understanding of how form items should be drawn.

addonSubpages := []settingsPage{
addonsPage.gwwPage,
}

// TODO: Make this respond to uncommitted config changes
Copy link
Author

Choose a reason for hiding this comment

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

Currently the addon only appears/disappears once you have save your changes to the "MetricsEnabled" setting. Is there a way to fetch the current uncommitted state of the config?


var containerContents []byte

if apcupsdMode.Value == apcupsd.Mode_Network {
Copy link
Author

Choose a reason for hiding this comment

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

This is perhaps a bit novel as I conditionally choose which compose file to use based on the users preference on how to run APCUPSD. I've tested and the smartnode seems to create and destroy the services correctly when toggling between the two different setups.


// Write config file
if apcupsdMode.Value == apcupsd.Mode_Container {
// TODO: Confirm this is a good location for a generic config file (i.e. not a container file)
Copy link
Author

Choose a reason for hiding this comment

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

I arbitrarily chose the location to write the apcupsd.conf file. Please let me know if there is a preferred location for templates and configuration files.

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.

1 participant