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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

EsDmitrii
Copy link
Contributor

Fixed issue mentioned in #631

@EsDmitrii EsDmitrii requested a review from a team as a code owner January 19, 2024 11:31
@tpmccallum
Copy link

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.

@EsDmitrii
Copy link
Contributor Author

EsDmitrii commented Jan 25, 2024

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
Copy link

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

@EsDmitrii
Copy link
Contributor Author

EsDmitrii commented Feb 19, 2024

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

@EsDmitrii
Copy link
Contributor Author

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

@anantha1999
Copy link

anantha1999 commented Mar 7, 2024

@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
Copy link
Contributor Author

@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

@oxyii oxyii mentioned this pull request May 13, 2024
@gecube
Copy link

gecube commented Aug 27, 2024

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.

additional annotations optional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants