pravega / zookeeper-operator

Kubernetes Operator for Zookeeper
Apache License 2.0
362 stars 201 forks source link

Zookeeper operator fails to install when used as a Helm dependency #285

Open junglie85 opened 3 years ago

junglie85 commented 3 years ago

Description

When specified as a dependency of a Helm chart, the Zookeeper Operator fails to install due to CRD's not being installed.

apiVersion: v2
name: my-name
description: My description
type: application
version: 0.1.0
appVersion: 0.1.0
dependencies:
  - name: zookeeper
    version: "0.2.9"
    repository: "https://charts.pravega.io"
  - name: zookeeper-operator
    version: "0.2.9"
    repository: "https://charts.pravega.io"
...

Helm cannot find the relevant CRD's because they have not yet been installed, so fails with the following error:

Error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "BookkeeperCluster" in version "bookkeeper.pravega.io/v1alpha1", unable to recognize "": no matches for kind "PravegaCluster" in version "pravega.pravega.io/v1beta1", unable to recognize "": no matches for kind "ZookeeperCluster" in version "zookeeper.pravega.io/v1beta1"]

The CRD template file has the conditional statement which prevents kubectl apply -f https://raw.githubusercontent.com/pravega/zookeeper-operator/master/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yamll:

error: error parsing https://raw.githubusercontent.com/pravega/zookeeper-operator/master/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml: json: line 0: invalid character '{' looking for beginning of object key string

This is equally applicable to the Pravega and Bookkeeper CRD's - I'll raise separate issues for those.

Importance

This is a must have for packaging Pravega as part of a larger application.

Location

charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml

Suggestions for an improvement

CRD's can be included in a crd directory and Helm will apply these first. The CRD's could also be provided in a separate Helm chart but I wouldn't find this particularly useful due to not being able to specify a dependency application order. https://helm.sh/docs/chart_best_practices/custom_resource_definitions/

Alternatively, the CRD could be provided as a release artefact that can be installed before running Helm with crd.create=false:

kubectl apply -f https://github.com/pravega/zookeeper-operator/releases/download/0.2.9/zookeeper-operator.crds.yaml
SrishT commented 3 years ago

@ambye85 having the crds folder was not required in our use-case since the crd installation is done in the zookeeper-operator charts and required in the zookeeper charts. Hence if you can ensure that the dependencies are installed in a particular order, i.e. the zookeeper-operator installation happens before attempting to install the zookeeper charts, that should fix the issue you are facing.

junglie85 commented 3 years ago

@SrishT I initially attempted to enforce an order but soft dependencies between charts is not supported by Helm:

Soft dependency: A chart may simply not function without another chart being installed in a cluster. Helm does not provide tooling for this case. In this case, dependencies may be managed separately.

There is a post from 2018 on Stackoverflow which suggests a possible workaround - if it works for Helm 3 the charts would need updating to allow custom annotations to be added. I think a CRD's folder would be a neater solution.

SrishT commented 3 years ago

Switching to using the crds folder does not seem to fit our requirement because of the removal of templating support from CRDs, and as there is no support for upgrading or deleting CRDs at this time, which is essential to our use-case.

AnujAroshA commented 2 years ago

I also had the same issue and what I did was overwrite the following values in my values.yaml file where the relevant helm chart takes zookeeper-operator as a dependency.

zookeeper-operator:
  crd:
    create: true

I don't know why we should do this since in the original helm chart the default value given is true.

bderrly commented 2 years ago

@AnujAroshA that doesn't seem to be working for me. Was there anything else you did?

josmare commented 3 months ago

I created a Flux Helm release and after some attempts I found @AnujAroshA's configuration worked:

---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: solr-cloud
  namespace: test-solr
spec:
  dependsOn:
    - name: solr-operator
  interval: 10m
  chart:
    spec:
      chart: solr
      version: '0.8.x'
      sourceRef:
        kind: HelmRepository
        name: apache-solr
      interval: 10m
  values:
    image:
      tag: 8.11.2
    addressability:
    dataStorage:
      capacity: 100Gi
      persistent:
        pvc:
          name: data
          storageClassName: default
      type: persistent
    fullnameOverride: test
    nameOverride: test
    replicaCount: 3
    solrOptions:
      javaMemory: -Xms10g -Xmx10g
      security:
        authenticationType: Basic
---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: solr-operator
  namespace: test-solr
spec:
  interval: 10m
  chart:
    spec:
      chart: solr-operator
      version: '0.8.x'
      sourceRef:
        kind: HelmRepository
        name: apache-solr
      interval: 10m
  values:
    zookeeper-operator:
      crd:
        create: true
    image:
      pullPolicy: IfNotPresent
      repository: apache/solr-operator
      tag: v0.8.0