Skip to content
This repository was archived by the owner on Oct 23, 2024. It is now read-only.

Extend Functionality of Grafana Package - #2098

Open
cbakersdl wants to merge 17 commits into
d2iq-archive:version-3.xfrom
cbakersdl:version-3.x
Open

Extend Functionality of Grafana Package#2098
cbakersdl wants to merge 17 commits into
d2iq-archive:version-3.xfrom
cbakersdl:version-3.x

Conversation

@cbakersdl

Copy link
Copy Markdown

The Grafana package was missing the ability to configure the service at installation/start time. Nor was there any way to change the configuration afterward. It did not contain any volume option so any dashboards/datasources created would not persist across restarts.

My objective was to enable full configuration and provisioning of the service at installation time in addition to persistent volume option. I have tested it to work with the DC/OS Prometheus package and DC/OS version 1.11.

I have also added a full example: https://github.com/cbakersdl/examples/tree/master/grafana which will depend on this merge.

One thing I am not sure of is the maintenance of my svc.yml changes going forward. My org is hosting it here for now: https://s3-us-west-2.amazonaws.com/dcos-objects/grafana/7/svc.yml but I'm curious to know if Mesosphere will take over hosting it or how that works. It would be really cool to be able to commit these files to the repo as well.

@cbakersdl

Copy link
Copy Markdown
Author

I thought an upstream merge might work but it didn't. May I have view perms on TC to see why the checks are failing?

@mattj-io
mattj-io requested a review from ryadav88 December 4, 2018 22:33
@vishnu2kmohan

Copy link
Copy Markdown
Contributor

@cbakersdl All the TeamCity tests have passed.

I'll leave it to @ryadav88 and @mattj-io to review and merge.

@mattj-io

mattj-io commented Dec 5, 2018

Copy link
Copy Markdown
Contributor

@ryadav88 - I don't know enough about packages to review this code, although happy to test. Can you review please ?

@cbakersdl

Copy link
Copy Markdown
Author

Aww bummer. Looks like a test is failing again. Anything I need to do?

Comment thread repo/packages/G/grafana/7/marathon.json.mustache Outdated
Comment thread repo/packages/G/grafana/7/resource.json Outdated
"assets": {
"uris": {
"jre-tar-gz": "https://downloads.mesosphere.com/java/server-jre-8u162-linux-x64.tar.gz",
"libmesos-bundle-tar-gz": "https://downloads.mesosphere.com/libmesos-bundle/libmesos-bundle-1.11.0.tar.gz",

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.

Please bump this to libmesos-bundle-1.12.0.tar.gz

{
"packagingVersion": "4.0",
"upgradesFrom": [
"*"

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.

Shouldn't this be the previous package version?

"*"
],
"downgradesTo": [
"*"

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.

Shouldn't this be the previous package version?

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.

I just copied this from 6/package.json. Should I change it to 3.0?

Comment thread repo/packages/G/grafana/7/resource.json Outdated
"BOOTSTRAP_URI": "{{resource.assets.uris.bootstrap-zip}}",
"GRAFANA_URI": "{{resource.assets.uris.grafana-tar-gz}}",
{{#service.service_account_secret}}
"DCOS_SERVICE_ACCOUNT_CREDENTIAL": { "secret": "serviceCredential" },

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.

Please change this so that we inject the Service Account as a File-Based Secret so that we don't accidentally leak its RSA Private Key:

"DCOS_SERVICE_ACCOUNT_CREDENTIAL": "secrets/service-account.json",

Comment thread repo/packages/G/grafana/7/marathon.json.mustache Outdated
@d2iq-mergebot d2iq-mergebot added the awaiting-testing-approval The PR is waiting for approval to start tests. label Jan 7, 2019
@vishnu2kmohan
vishnu2kmohan requested a review from realmbgl January 8, 2019 13:54
@vishnu2kmohan

Copy link
Copy Markdown
Contributor

Once @ryadav88 and/or @realmbgl approve this you'll have my approval @cbakersdl

Thank you for your patience.

@ryadav88 ryadav88 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.

LGTM. Thanks for the contribution @cbakersdl.
dcos/examples#384 to be merged along with this package update.

@d2iq-mergebot
d2iq-mergebot dismissed vishnu2kmohan’s stale review October 7, 2019 15:56

Review dismissed as PR branch got new commits. Please re-review new commits.

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

Labels

awaiting-testing-approval The PR is waiting for approval to start tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants