-
Notifications
You must be signed in to change notification settings - Fork 226
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
chore: update chart with latest add-on changes #376
Conversation
Signed-off-by: Jorge Turrado <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update Chart.yaml please?
I'll do another PR with the release changes (chart.yaml, package and index), this PR is only to add missing changes |
Oh I have misread the title; sorry. |
no worries xD |
{{- include "keda-addons-http.labels" . | indent 4 }} | ||
name: {{ .Chart.Name }}-manager-role | ||
name: {{ .Chart.Name }}-role | ||
rules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this introduced a spec error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this produces error 😢
Let me review it and check to cover this by test for the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we get a chart v0.4.1? I could provide a PR for a release if you are not already up to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on it, but I want to solve the CI before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question @holygrolli ,
How are you deploying the chart? I'm just trying with helm and even added again the extra line, I can't reproduce it (due to IDK how to validate it in the CI, which also uses helm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using the GitOps approach and actually FluxCD. I first found the explanation here where I recognized the Helm chart is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okey... it's complicated to reproduce during CI :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The chart is already available @holygrolli , tomorrow we will announce it, but it's already available
Signed-off-by: Jorge Turrado [email protected]
I have executed the e2e test we have in the http-add-on repo and it works.
Basically I have updated the RBAC files to organize them as we autogenerate them in the add-on repo in order to be easier to update. I have also created a serviceAccount to use for the add-on and solve some small issues with some labels/namespaces
CRD is also updated
Checklist