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

addon for prometheus in openshift 3.7 #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

durandom
Copy link
Contributor

fixes #50 prometheus for openshift 3.7

@durandom
Copy link
Contributor Author

cc @jorgemoralespou as its based on your work
ideally this could be in the prometheus addon, but that does conflict with prometheus on 3.6

@jorgemoralespou
Copy link
Contributor

@durandom there's now the ability to remove an add-on easily by creating an xxx.addon.remove file with instructions to delete the add-on. I still need to update the ones I created.

With regards to the differences to the 3.6 version, are they all in the yaml files or is it also in the install process?

@durandom
Copy link
Contributor Author

durandom commented Nov 26, 2017

With regards to the differences to the 3.6 version, are they all in the yaml files or is it also in the install process?

I also need to install the node-exporter.

oc create -f node-exporter.yaml -n #{prometheus_namespace}
oc adm policy add-scc-to-user -z prometheus-node-exporter -n #{prometheus_namespace} hostaccess

@durandom
Copy link
Contributor Author

I've added the .remove file

@praveenkumar
Copy link

@jorgemoralespou does this look good to you now?

@durandom
Copy link
Contributor Author

durandom commented Dec 7, 2017

I'm still not so sure on wether a new addon is actually needed.
But I only see a way to move this into the original addon, by moving some logic out of the .addon file and requiring the user to run a script after installation, which is a convenience degradation?!

@praveenkumar are there discussions on executing shell scripts in the .addon file? This way we could move some conditional logic into a shell script

@praveenkumar
Copy link

are there discussions on executing shell scripts in the .addon file?

Check minishift/minishift#1783 but we are not going to have bash script execution support in the addon.

@jorgemoralespou
Copy link
Contributor

Should we need to keep the addon working for 3.6 when minishift has already moved to 3.7?

@durandom
Copy link
Contributor Author

maybe not. I'll create a new PR to update your original addon to be working with 3.7 then

@coolbrg
Copy link
Contributor

coolbrg commented Dec 12, 2017

Should we need to keep the addon working for 3.6 when minishift has already moved to 3.7?

@jorgemoralespou

We have not moved to 3.7 yet. PR is there minishift/minishift#1808 but getting some issue.

Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

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

The addon name is "prometheus-3.7", shouldn't it be just "prometheus"? I saw that we already have a "prometheus" add-on, so why is this required as a separate add-on i.e. why we can update the existing "prometheus" add-on?

Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

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

I'm still not so sure on wether a new addon is actually needed.
But I only see a way to move this into the original addon, by moving some logic out of the .addon file and requiring the user to run a script after installation, which is a convenience degradation?!

I see. @jorgemoralespou are you ok with this?

To deploy prometheus do:

```
minishift addon apply prometheus --addon-env prometheus_namespace=kube-system
Copy link
Member

Choose a reason for hiding this comment

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

Add-on names are decided from the directory names of the add-on, so the name of the add-on for this PR is prometheus-3.7. But the readme says that the name is prometheus. So I am little confused.

@gbraad
Copy link
Member

gbraad commented Jan 25, 2018

any updates?

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.

addon for prometheus in openshift 3.7
6 participants