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

Install fails on non-posthog namespace #75

Closed
3 tasks done
taobojlen opened this issue Jul 28, 2021 · 6 comments · Fixed by #79
Closed
3 tasks done

Install fails on non-posthog namespace #75

taobojlen opened this issue Jul 28, 2021 · 6 comments · Fixed by #79
Labels
bug Something isn't working

Comments

@taobojlen
Copy link

Bug description

I saw that your Clickhouse charts were now public, so I thought I'd have a go at installing them in our staging namespace to see how it works. I'm running into an issue, though, where the chart seems to expect the namespace to be posthog.

Expected behavior

I can install the chart in any namespace.

How to reproduce

  1. helm upgrade -i --timeout 20m --namespace main-tools-staging -f values-staging.yaml staging-posthog-clickhouse path/to/my/chart
  2. Get the error Error: UPGRADE FAILED: failed to create resource: namespaces "posthog" not found

My chart

apiVersion: v2
name: posthog-clickhouse
type: application
version: 0.1.0
appVersion: "1.26.0"
dependencies:
  - name: posthog
    version: 3.0.0
    repository: https://posthog.github.io/charts-clickhouse

Values

posthog-clickhouse:
  cloud: "gcp"
  certManager:
    enabled: false
  ingress:
    nginx:
      enabled: false
    hostname: posthog-clickhouse.staging.example.com
  hooks:
    migrate:
      hookAnnotation: "pre-install,pre-upgrade"
  clickhouseOperator:
    namespace: "main-tools-staging"

Environment

  • Deployment platform (gcp/aws/...): GCP
  • Chart version/commit: 3.0.0
  • Posthog version: 1.26
@taobojlen taobojlen added the bug Something isn't working label Jul 28, 2021
@macobo
Copy link
Contributor

macobo commented Jul 28, 2021

To fix, change clickhouseOperator.namespace: "main-tools-staging" in values.yaml

cc @fuziontech @tiina303 - having this option still seems slightly weird. Perhaps it should just default to the same namespace?

@taobojlen
Copy link
Author

The error still appears without setting clickhouseOperator.namespace (I actually only changed that in an attempt to work around the bug). Sorry, could've been clearer about that!

@tiina303
Copy link
Contributor

The error still appears without setting clickhouseOperator.namespace (I actually only changed that in an attempt to work around the bug). Sorry, could've been clearer about that!

Sorry this is inconvenient. Currently you need to set that value or have "posthog" namespace. I agree that this is confusing and annoying that we need to define it, I'll look into removing that.

Why you're running into the error with your initial values that you provided: I'm not sure how your values are used as normally they would be at the top level, i.e.
instead of

posthog-clickhouse:
  cloud: "gcp"
  ...

you'd have

cloud: "gcp"
...

@tiina303
Copy link
Contributor

Looked into the clickhouseOperator.namespace and by default it would otherwise install into kube-system namespace (https://github.com/Altinity/clickhouse-operator/blob/master/docs/operator_installation_details.md), however I think it's better if we keep all Posthog things in the same namespace and I couldn't find a way to use the current namespace.

@taobojlen
Copy link
Author

Thanks @tiina303 -- we'll leave in clickhouseOperator.namespace for now then!

Re: the wrapped config values, it's an internal convention to "wrap" charts as dependencies in custom charts of our own. It turns out thaat using posthog instead of posthog-clickhouse for this actually fixes the problems (so our Chart.yaml has name: posthog and all values are under the posthog key).

Closing this; thanks for your help!

@fuziontech
Copy link
Member

cc @fuziontech @tiina303 - having this option still seems slightly weird. Perhaps it should just default to the same namespace?

Yeah this is for legacy compatibility and can be nuked now. #79 addresses it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants