search

spotahome / redis-operator

Redis Operator creates/configures/manages high availability redis with sentinel automatic failover atop Kubernetes.
Apache License 2.0
1.51k stars 359 forks source link

fix: fixed issue mentioned in https://github.com/spotahome/redis-oper… #685

Open EsDmitrii opened 8 months ago

EsDmitrii commented 8 months ago

Fixed issue mentioned in https://github.com/spotahome/redis-operator/pull/631

tpmccallum commented 8 months ago

Also getting the error, identified by @liupeng0518 at https://github.com/spotahome/redis-operator/pull/631#issuecomment-1833309651 Re: this open issue https://github.com/spotahome/redis-operator/issues/679

helm install redis-operator redis-operator/redis-operator
Error: INSTALLATION FAILED: failed to install CRD crds/databases.spotahome.com_redisfailovers.yaml: error parsing : error converting YAML to JSON: yaml: line 4: did not find expected node content

Believe this PR will fix the above error. Any update on this being reviewed/merged? cc: @EsDmitrii @ese

Thanks so much.

EsDmitrii commented 8 months ago

Also getting the error, identified by @liupeng0518 at #631 (comment) Re: this open issue #679

helm install redis-operator redis-operator/redis-operator
Error: INSTALLATION FAILED: failed to install CRD crds/databases.spotahome.com_redisfailovers.yaml: error parsing : error converting YAML to JSON: yaml: line 4: did not find expected node content

Believe this PR will fix the above error. Any update on this being reviewed/merged? cc: @EsDmitrii @ese

Thanks so much.

Hi! Yep I used helm tempting for CRD and it was outside folder for templates so when you try to deploy helm says “dude I don’t know what is it, it’s not a crd” :) It should fix the issue

winterrobert commented 7 months ago

@EsDmitrii is this ready to merge? I would really like to use your CRD changes for disablePodDisruptionBudget

EsDmitrii commented 7 months ago

@winterrobert Hi! I have no write access to this repo to merge chages

EsDmitrii commented 7 months ago

@ese Hi! Can you review changes and merge this PR please? It's blocking users

anantha1999 commented 6 months ago

@EsDmitrii I am facing the same issue in argocd, but will this fix the issue? Because you are using template language in yaml which is in crds folder, helm template would not be able to process it.

EsDmitrii commented 6 months ago

@EsDmitrii I am facing the same issue in argocd, but will this fix the issue? Because you are using template language in yaml which is in crds folder, helm template would not be able to process it.

Yes, it will. Folder with crd located in template folder. You can pull changes and test it locally

gecube commented 1 month ago

I am kindly asking to rewrite PR and then accept it.

I don't like unnecessary ArgoCD annotations: not everybody are ArgoCD users. They should not be in defaults. The default annotations should be {}. For ArgoCD users they can change the values.yaml and pass additional annotations.