Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Adding InfluxDB Helm chart #44

Merged
merged 9 commits into from
Nov 22, 2021
Merged

Adding InfluxDB Helm chart #44

merged 9 commits into from
Nov 22, 2021

Conversation

zakaria2905
Copy link
Contributor

@zakaria2905 zakaria2905 commented Nov 2, 2021 •
edited
Loading

Adding InfluxDB in Chart.yaml file
Adding enable configuration in values.yaml file
Adding doc link in README.md file

We chose the influxdb chart of bitnami because it release activity is higher:
https://artifacthub.io/packages/search?ts_query_web=influxdb&sort=relevance&page=1

@zakaria2905 zakaria2905 added the enhancement New feature or request label Nov 2, 2021
@zakaria2905 zakaria2905 assigned banzo and unassigned banzo Nov 2, 2021
@banzo
Copy link
Contributor

banzo commented Nov 3, 2021

Seems there is a linting issue:

>> Building chart...
>>> helm lint /root/project/fadi
coalesce.go:196: warning: cannot overwrite table with non table for service (map[annotations:map[] clusterIP: loadBalancerIP: loadBalancerSourceRanges:[] nodePorts:map[http: rpc:] port:8086 rpcPort:8088 sessionAffinity: sessionAffinityConfig:map[] type:ClusterIP])
coalesce.go:196: warning: cannot overwrite table with non table for service (map[annotations:map[] clusterIP: loadBalancerIP: loadBalancerSourceRanges:[] nodePorts:map[http: rpc:] port:8086 rpcPort:8088 sessionAffinity: sessionAffinityConfig:map[] type:ClusterIP])
==> Linting /root/project/fadi
[ERROR] templates/: template: fadi/charts/influxdb/templates/influxdb/service.yaml:11:19: executing "fadi/charts/influxdb/templates/influxdb/service.yaml" at <.Values.influxdb.service.annotations>: can't evaluate field annotations in type interface {}

Error: 1 chart(s) linted, 1 chart(s) failed

@banzo
Copy link
Contributor

banzo commented Nov 3, 2021

Some preliminary review comments:

  • keep the enabled flags as they were before in the values.yaml file
  • influxdb should be disabled by default
  • PR description should include a justification of the Bitnami chart selection (why not https://github.com/influxdata/helm-charts/tree/master/charts for example)
  • add influxdb to the README services table (mention this is including influxdb and not influxdbv2)
  • create issue so we keep track that we will probably want to add support for influxdb v2

@banzo banzo merged commit abc29d2 into master Nov 22, 2021
@banzo banzo linked an issue Dec 14, 2021 that may be closed by this pull request
[cetic/fadi] InfluxDB V2 support #45
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@banzo banzo banzo approved these changes

Assignees

@zakaria2905 zakaria2905

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[cetic/fadi] InfluxDB V2 support
2 participants
@zakaria2905 @banzo